Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: byte-range request body size #190

Merged
merged 2 commits into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions packages/verified-fetch/src/utils/byte-range-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,35 @@
return body
}

// TODO: we should be able to use this.offset and this.length to slice the body
private getSlicedBody <T extends SliceableBody>(body: T): SliceableBody {
const offset = this.byteStart ?? 0
const length = this.byteEnd == null ? undefined : this.byteEnd + 1
this.log.trace('returning body with offset %o and length %o', offset, length)
return body.slice(offset, length) satisfies SliceableBody

// Calculate the correct number of bytes to return
// For a range like bytes=1000-2000, we want exactly 1001 bytes
let length: number | undefined

if (this.byteEnd != null && this.byteStart != null) {
// Exact number of bytes is (end - start + 1) due to inclusive ranges
length = this.byteEnd - this.byteStart + 1
} else {
length = undefined
}

Check warning on line 132 in packages/verified-fetch/src/utils/byte-range-context.ts

View check run for this annotation

Codecov / codecov/patch

packages/verified-fetch/src/utils/byte-range-context.ts#L131-L132

Added lines #L131 - L132 were not covered by tests

this.log.trace('slicing body with offset=%o and length=%o', offset, length)

if (typeof body === 'string') {
// String slicing works with start and end indices
return body.slice(offset, length !== undefined ? offset + length : undefined) satisfies SliceableBody

Check warning on line 138 in packages/verified-fetch/src/utils/byte-range-context.ts

View check run for this annotation

Codecov / codecov/patch

packages/verified-fetch/src/utils/byte-range-context.ts#L137-L138

Added lines #L137 - L138 were not covered by tests
} else if (body instanceof Blob) {
// Blob.slice takes start and end positions
return body.slice(offset, length !== undefined ? offset + length : undefined) satisfies SliceableBody
} else if (body instanceof ArrayBuffer || body instanceof Uint8Array) {
// ArrayBuffer.slice and Uint8Array.slice take start and end positions
return body.slice(offset, length !== undefined ? offset + length : undefined) satisfies SliceableBody
} else {
// This should never happen due to type constraints
return body as SliceableBody
}

Check warning on line 148 in packages/verified-fetch/src/utils/byte-range-context.ts

View check run for this annotation

Codecov / codecov/patch

packages/verified-fetch/src/utils/byte-range-context.ts#L146-L148

Added lines #L146 - L148 were not covered by tests
}

/**
Expand Down Expand Up @@ -228,14 +251,15 @@
* 2. slicing the body
*/
public get length (): number | undefined {
if (this.byteEnd != null && this.byteStart != null && this.byteStart === this.byteEnd) {
return 1
if (this.byteEnd != null && this.byteStart != null) {
// For a range like bytes=1000-2000, we want a length of 1001 bytes
return this.byteEnd - this.byteStart + 1
}
if (this.byteEnd != null) {
return this.byteEnd + 1
}

return this.byteSize != null ? this.byteSize - 1 : undefined
return this.byteSize
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('ByteRangeContext', () => {
})

const array = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]
const largeArray = Array.from({ length: 3000 }, (_, i) => i + 1)
const uint8arrayRangeTests = [
// full ranges:
{ type: 'Uint8Array', range: 'bytes=0-10', contentRange: 'bytes 0-10/11', body: new Uint8Array(array), expected: new Uint8Array(array) },
Expand All @@ -63,12 +64,14 @@ describe('ByteRangeContext', () => {
{ type: 'Uint8Array', range: 'bytes=1-', contentRange: 'bytes 1-10/11', body: new Uint8Array(array), expected: new Uint8Array([2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) },
{ type: 'Uint8Array', range: 'bytes=2-', contentRange: 'bytes 2-10/11', body: new Uint8Array(array), expected: new Uint8Array([3, 4, 5, 6, 7, 8, 9, 10, 11]) },
{ type: 'Uint8Array', range: 'bytes=-2', contentRange: 'bytes 9-10/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-2)) },
{ type: 'Uint8Array', range: 'bytes=1000-2000', contentRange: 'bytes 1000-2000/3000', body: new Uint8Array(largeArray), expected: new Uint8Array(largeArray.slice(1000, 2001)) }, // https://github.com/ipfs/helia-verified-fetch/issues/184

// single byte ranges:
{ type: 'Uint8Array', range: 'bytes=1-1', contentRange: 'bytes 1-1/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(1, 2)) },
{ type: 'Uint8Array', range: 'bytes=-1', contentRange: 'bytes 10-10/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-1)) }

]

const validRanges = [
...uint8arrayRangeTests,
...uint8arrayRangeTests.map(({ range, contentRange, body, expected }) => ({
Expand Down
Loading