fix: byte range request end should never equal file size

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

@@ -116,30 +116,14 @@
     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).
@@ -162,7 +146,10 @@
       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
       }
     }
@@ -174,7 +161,10 @@
       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
       }
     }
@@ -214,6 +204,10 @@
         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
   }
@@ -224,16 +218,6 @@
    * 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
   }
 
@@ -243,7 +227,14 @@
    * 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
   }
 
   /**
@@ -269,11 +260,18 @@
       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
+    }
   }
 
   /**

packages/verified-fetch/src/utils/request-headers.ts

@@ -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 }
 }

packages/verified-fetch/src/utils/response-headers.ts

@@ -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) {

packages/verified-fetch/test/range-requests.spec.ts

@@ -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])
         }

packages/verified-fetch/test/utils/byte-range-context.spec.ts

@@ -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]) },
@@ -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 = [
@@ -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)
       })
     })
   })

packages/verified-fetch/test/utils/request-headers.spec.ts

@@ -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
@@ -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.')
     })
   })
 })

packages/verified-fetch/test/utils/response-headers.spec.ts

@@ -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', () => {
@@ -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
+    })
   })
 })