Skip to content

Commit

Permalink
fix: byte range request end should never equal file size (#24)
Browse files Browse the repository at this point in the history 
* fix: request-headers byte range parsing

* fix: response-headers byte parsing

* fix: request-headers

* fix: byteRangeContext tests

* fix: range-requests tests
  • Loading branch information
@SgtPooki
SgtPooki authored Mar 18, 2024
1 parent 8bf9c9f commit aafc567
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 69 deletions.
68 changes: 33 additions & 35 deletions packages/verified-fetch/src/utils/byte-range-context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,30 +116,14 @@ export class ByteRangeContext {
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 {
if (this.isPrefixLengthRequest) {
this.log.trace('sliced body with byteStart %o', this.byteStart)
return body.slice(this.offset) satisfies SliceableBody
}
if (this.isSuffixLengthRequest && this.length != null) {
this.log.trace('sliced body with length %o', -this.length)
return body.slice(-this.length) satisfies 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
}

private get isSuffixLengthRequest (): boolean {
return this.requestRangeStart == null && this.requestRangeEnd != null
}

private get isPrefixLengthRequest (): boolean {
return this.requestRangeStart != null && this.requestRangeEnd == null
}

/**
* Sometimes, we need to set the fileSize explicitly because we can't calculate
* the size of the body (e.g. for unixfs content where we call .stat).
Expand All @@ -162,7 +146,10 @@ export class ByteRangeContext {
if (this.byteStart < 0) {
return false
}
if (this._fileSize != null && this.byteStart > this._fileSize) {
if (this._fileSize != null && this.byteStart >= this._fileSize) {
return false
}
if (this.byteEnd != null && this.byteStart > this.byteEnd) {
return false
}
}
Expand All @@ -174,7 +161,10 @@ export class ByteRangeContext {
if (this.byteEnd < 0) {
return false
}
if (this._fileSize != null && this.byteEnd > this._fileSize) {
if (this._fileSize != null && this.byteEnd >= this._fileSize) {
return false
}
if (this.byteStart != null && this.byteEnd < this.byteStart) {
return false
}
}
Expand Down Expand Up @@ -214,6 +204,10 @@ export class ByteRangeContext {
return false
}
}
if (this.byteEnd == null && this.byteStart == null && this.byteSize == null) {
this.log.trace('invalid range request, could not calculate byteStart, byteEnd, or byteSize')
return false
}

return true
}
Expand All @@ -224,16 +218,6 @@ export class ByteRangeContext {
* 2. slicing the body
*/
public get offset (): number {
if (this.byteStart === 0) {
return 0
}
if (this.isPrefixLengthRequest || this.isSuffixLengthRequest) {
if (this.byteStart != null) {
// we have to subtract by 1 because the offset is inclusive
return this.byteStart - 1
}
}

return this.byteStart ?? 0
}

Expand All @@ -243,7 +227,14 @@ export class ByteRangeContext {
* 2. slicing the body
*/
public get length (): number | undefined {
return this.byteSize ?? undefined
if (this.byteEnd != null && this.byteStart != null && this.byteStart === this.byteEnd) {
return 1
}
if (this.byteEnd != null) {
return this.byteEnd + 1
}

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

/**
Expand All @@ -269,11 +260,18 @@ export class ByteRangeContext {
return
}

const { start, end, byteSize } = calculateByteRangeIndexes(this.requestRangeStart ?? undefined, this.requestRangeEnd ?? undefined, this._fileSize ?? undefined)
this.log.trace('set byteStart to %o, byteEnd to %o, byteSize to %o', start, end, byteSize)
this.byteStart = start
this.byteEnd = end
this.byteSize = byteSize
try {
const { start, end, byteSize } = calculateByteRangeIndexes(this.requestRangeStart ?? undefined, this.requestRangeEnd ?? undefined, this._fileSize ?? undefined)
this.log.trace('set byteStart to %o, byteEnd to %o, byteSize to %o', start, end, byteSize)
this.byteStart = start
this.byteEnd = end
this.byteSize = byteSize
} catch (e) {
this.log.error('error setting offset details: %o', e)
this.byteStart = undefined
this.byteEnd = undefined
this.byteSize = undefined
}
}

/**
Expand Down
32 changes: 22 additions & 10 deletions packages/verified-fetch/src/utils/request-headers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,41 @@ export function getHeader (headers: HeadersInit | undefined, header: string): st
* 2. the start index of the range. // inclusive
* 3. the end index of the range. // inclusive
*/
// eslint-disable-next-line complexity
export function calculateByteRangeIndexes (start: number | undefined, end: number | undefined, fileSize?: number): { byteSize?: number, start?: number, end?: number } {
if (start != null && end != null) {
if (start > end) {
throw new Error('Invalid range')
}
if ((start ?? 0) > (end ?? Infinity)) {
throw new Error('Invalid range: Range-start index is greater than range-end index.')
}
if (start != null && (end ?? 0) >= (fileSize ?? Infinity)) {
throw new Error('Invalid range: Range-end index is greater than or equal to the size of the file.')
}
if (start == null && (end ?? 0) > (fileSize ?? Infinity)) {
throw new Error('Invalid range: Range-end index is greater than the size of the file.')
}
if (start != null && start < 0) {
throw new Error('Invalid range: Range-start index cannot be negative.')
}

if (start != null && end != null) {
return { byteSize: end - start + 1, start, end }
} else if (start == null && end != null) {
// suffix byte range requested
if (fileSize == null) {
return { end }
}
const result = { byteSize: end, start: fileSize - end + 1, end: fileSize }
return result
if (end === fileSize) {
return { byteSize: fileSize, start: 0, end: fileSize - 1 }
}
return { byteSize: end, start: fileSize - end, end: fileSize - 1 }
} else if (start != null && end == null) {
if (fileSize == null) {
// we only have the start index, and no fileSize, so we can't return a valid range.
return { start }
}
const byteSize = fileSize - start + 1
const end = fileSize
const end = fileSize - 1
const byteSize = fileSize - start
return { byteSize, start, end }
}

// both start and end are undefined
return { byteSize: fileSize }
return { byteSize: fileSize, start: 0, end: fileSize != null ? fileSize - 1 : 0 }
}
14 changes: 12 additions & 2 deletions packages/verified-fetch/src/utils/response-headers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,30 @@
export function getContentRangeHeader ({ byteStart, byteEnd, byteSize }: { byteStart: number | undefined, byteEnd: number | undefined, byteSize: number | undefined }): string {
const total = byteSize ?? '*' // if we don't know the total size, we should use *

if ((byteEnd ?? 0) >= (byteSize ?? Infinity)) {
throw new Error('Invalid range: Range-end index is greater than or equal to the size of the file.')
}
if ((byteStart ?? 0) >= (byteSize ?? Infinity)) {
throw new Error('Invalid range: Range-start index is greater than or equal to the size of the file.')
}

if (byteStart != null && byteEnd == null) {
// only byteStart in range
if (byteSize == null) {
return `bytes */${total}`
}
return `bytes ${byteStart}-${byteSize}/${byteSize}`
return `bytes ${byteStart}-${byteSize - 1}/${byteSize}`
}

if (byteStart == null && byteEnd != null) {
// only byteEnd in range
if (byteSize == null) {
return `bytes */${total}`
}
return `bytes ${byteSize - byteEnd + 1}-${byteSize}/${byteSize}`
const end = byteSize - 1
const start = end - byteEnd + 1

return `bytes ${start}-${end}/${byteSize}`
}

if (byteStart == null && byteEnd == null) {
Expand Down
6 changes: 3 additions & 3 deletions packages/verified-fetch/test/range-requests.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ describe('range requests', () => {
},
{
byteSize: 8,
contentRange: 'bytes 4-11/11',
contentRange: 'bytes 4-10/11',
rangeHeader: 'bytes=4-',
bytes: new Uint8Array([3, 4, 5, 6, 7, 8, 9, 10])
bytes: new Uint8Array([4, 5, 6, 7, 8, 9, 10])
},
{
byteSize: 9,
contentRange: 'bytes 3-11/11',
contentRange: 'bytes 2-10/11',
rangeHeader: 'bytes=-9',
bytes: new Uint8Array([2, 3, 4, 5, 6, 7, 8, 9, 10])
}
Expand Down
17 changes: 8 additions & 9 deletions packages/verified-fetch/test/utils/byte-range-context.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ describe('ByteRangeContext', () => {
const array = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]
const uint8arrayRangeTests = [
// full ranges:
{ type: 'Uint8Array', range: 'bytes=0-11', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) },
{ type: 'Uint8Array', range: 'bytes=-11', contentRange: 'bytes 1-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) },
{ type: 'Uint8Array', range: 'bytes=0-', contentRange: 'bytes 0-11/11', body: new Uint8Array(array), expected: new Uint8Array(array) },
{ type: 'Uint8Array', range: 'bytes=0-10', contentRange: 'bytes 0-10/11', body: new Uint8Array(array), expected: new Uint8Array(array) },
{ type: 'Uint8Array', range: 'bytes=-11', contentRange: 'bytes 0-10/11', body: new Uint8Array(array), expected: new Uint8Array(array) },
{ type: 'Uint8Array', range: 'bytes=0-', contentRange: 'bytes 0-10/11', body: new Uint8Array(array), expected: new Uint8Array(array) },

// partial ranges:
{ type: 'Uint8Array', range: 'bytes=0-1', contentRange: 'bytes 0-1/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2]) },
Expand All @@ -60,13 +60,13 @@ describe('ByteRangeContext', () => {
{ type: 'Uint8Array', range: 'bytes=0-8', contentRange: 'bytes 0-8/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9]) },
{ type: 'Uint8Array', range: 'bytes=0-9', contentRange: 'bytes 0-9/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) },
{ type: 'Uint8Array', range: 'bytes=0-10', contentRange: 'bytes 0-10/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) },
{ type: 'Uint8Array', range: 'bytes=1-', contentRange: 'bytes 1-11/11', body: new Uint8Array(array), expected: new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]) },
{ type: 'Uint8Array', range: 'bytes=2-', contentRange: 'bytes 2-11/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 10-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-2)) },
{ 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)) },

// 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 11-11/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-1)) }
{ type: 'Uint8Array', range: 'bytes=-1', contentRange: 'bytes 10-10/11', body: new Uint8Array(array), expected: new Uint8Array(array.slice(-1)) }

]
const validRanges = [
Expand Down Expand Up @@ -138,12 +138,11 @@ describe('ByteRangeContext', () => {
})
const stat = await fs.stat(cid)
context.setFileSize(stat.fileSize)

context.setBody(await getBodyStream(context.offset, context.length))
const response = new Response(context.getBody())
const bodyResult = await response.arrayBuffer()
expect(new Uint8Array(bodyResult)).to.deep.equal(expected)
expect(context.contentRangeHeaderValue).to.equal(contentRange)
expect(new Uint8Array(bodyResult)).to.deep.equal(expected)
})
})
})
Expand Down
23 changes: 15 additions & 8 deletions packages/verified-fetch/test/utils/request-headers.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ describe('request-headers', () => {
describe('calculateByteRangeIndexes', () => {
const testCases = [
// Range: bytes=5-
{ start: 5, end: undefined, fileSize: 10, expected: { byteSize: 6, start: 5, end: 10 } },
{ start: 5, end: undefined, fileSize: 10, expected: { byteSize: 5, start: 5, end: 9 } },
// Range: bytes=-5
{ start: undefined, end: 5, fileSize: 10, expected: { byteSize: 5, start: 6, end: 10 } },
{ start: undefined, end: 5, fileSize: 10, expected: { byteSize: 5, start: 5, end: 9 } },
// Range: bytes=0-0
{ start: 0, end: 0, fileSize: 10, expected: { byteSize: 1, start: 0, end: 0 } },
// Range: bytes=5- with unknown filesize
Expand All @@ -44,18 +44,25 @@ describe('request-headers', () => {
// Range: bytes=0-0 with unknown filesize
{ start: 0, end: 0, fileSize: undefined, expected: { byteSize: 1, start: 0, end: 0 } },
// Range: bytes=-9 & fileSize=11
{ start: undefined, end: 9, fileSize: 11, expected: { byteSize: 9, start: 3, end: 11 } },
// Range: bytes=0-11 & fileSize=11
{ start: 0, end: 11, fileSize: 11, expected: { byteSize: 12, start: 0, end: 11 } }
{ start: undefined, end: 9, fileSize: 11, expected: { byteSize: 9, start: 2, end: 10 } },
// Range: bytes=-11 & fileSize=11
{ start: undefined, end: 11, fileSize: 11, expected: { byteSize: 11, start: 0, end: 10 } },
// Range: bytes=-2 & fileSize=11
{ start: undefined, end: 2, fileSize: 11, expected: { byteSize: 2, start: 9, end: 10 } },
// Range request with no range (added for coverage)
{ start: undefined, end: undefined, fileSize: 10, expected: { byteSize: 10, start: 0, end: 9 } }

]
testCases.forEach(({ start, end, fileSize, expected }) => {
it(`should return expected result for bytes=${start ?? ''}-${end ?? ''} and fileSize=${fileSize}`, () => {
const result = calculateByteRangeIndexes(start, end, fileSize)
expect(result).to.deep.equal(expected)
expect(calculateByteRangeIndexes(start, end, fileSize)).to.deep.equal(expected)
})
})
it('throws error for invalid range', () => {
expect(() => calculateByteRangeIndexes(5, 4, 10)).to.throw('Invalid range')
expect(() => calculateByteRangeIndexes(5, 4, 10)).to.throw('Invalid range: Range-start index is greater than range-end index.')
expect(() => calculateByteRangeIndexes(5, 11, 11)).to.throw('Invalid range: Range-end index is greater than or equal to the size of the file.')
expect(() => calculateByteRangeIndexes(undefined, 12, 11)).to.throw('Invalid range: Range-end index is greater than the size of the file.')
expect(() => calculateByteRangeIndexes(-1, 5, 10)).to.throw('Invalid range: Range-start index cannot be negative.')
})
})
})
11 changes: 9 additions & 2 deletions packages/verified-fetch/test/utils/response-headers.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ describe('response-headers', () => {
})

it('should return correct content range header when only byteEnd and byteSize are provided', () => {
expect(getContentRangeHeader({ byteStart: undefined, byteEnd: 9, byteSize: 11 })).to.equal('bytes 3-11/11')
expect(getContentRangeHeader({ byteStart: undefined, byteEnd: 9, byteSize: 11 })).to.equal('bytes 2-10/11')
})

it('should return correct content range header when only byteStart and byteSize are provided', () => {
expect(getContentRangeHeader({ byteStart: 5, byteEnd: undefined, byteSize: 11 })).to.equal('bytes 5-11/11')
expect(getContentRangeHeader({ byteStart: 5, byteEnd: undefined, byteSize: 11 })).to.equal('bytes 5-10/11')
})

it('should return correct content range header when only byteStart is provided', () => {
Expand All @@ -29,5 +29,12 @@ describe('response-headers', () => {
it('should return content range header with when only byteSize is provided', () => {
expect(getContentRangeHeader({ byteStart: undefined, byteEnd: undefined, byteSize: 50 })).to.equal('bytes */50')
})

it('should not allow range-end to equal or exceed the size of the file', () => {
expect(() => getContentRangeHeader({ byteStart: 0, byteEnd: 11, byteSize: 11 })).to.throw('Invalid range') // byteEnd is equal to byteSize
expect(() => getContentRangeHeader({ byteStart: undefined, byteEnd: 11, byteSize: 11 })).to.throw('Invalid range') // byteEnd is equal to byteSize
expect(() => getContentRangeHeader({ byteStart: undefined, byteEnd: 12, byteSize: 11 })).to.throw('Invalid range') // byteEnd is greater than byteSize
expect(() => getContentRangeHeader({ byteStart: 11, byteEnd: undefined, byteSize: 11 })).to.throw('Invalid range') // byteEnd is greater than byteSize
})
})
})

0 comments on commit aafc567

Please sign in to comment.