fix: improve parsing content-range header

benchmarks/parseContentRangeHeader.mjs

@@ -0,0 +1,19 @@
+import { bench, group, run } from 'mitata'
+import { parseContentRangeHeader } from '../lib/core/util.js'
+
+group('parseContentRangeHeader', () => {
+  bench('parseContentRangeHeader undefined', () => {
+    parseContentRangeHeader()
+  })
+  bench('parseContentRangeHeader empty', () => {
+    parseContentRangeHeader('')
+  })
+  bench('parseContentRangeHeader bytes 0-400/400', () => {
+    parseContentRangeHeader('bytes 0-400/400')
+  })
+  bench('parseContentRangeHeader bytes 0-400/*', () => {
+    parseContentRangeHeader('bytes 0-400/*')
+  })
+})
+
+await run()

lib/core/util.js

@@ -487,19 +487,41 @@ function isValidHTTPToken (characters) {
   return true
 }
 
-// Parsed accordingly to RFC 9110
-// https://www.rfc-editor.org/rfc/rfc9110#field.content-range
-function parseRangeHeader (range) {
-  if (range == null || range === '') return { start: 0, end: null, size: null }
-
-  const m = range ? range.match(/^bytes (\d+)-(\d+)\/(\d+)?$/) : null
-  return m
-    ? {
-        start: parseInt(m[1]),
-        end: m[2] ? parseInt(m[2]) : null,
-        size: m[3] ? parseInt(m[3]) : null
-      }
-    : null
+const emptyRange = Object.freeze({ start: 0, end: null, size: null })
+const rangeHeaderRE = /^bytes (?:(?:(?:(\d+)-(\d+)))\/(?:\*|(\d+))$|\*\/(\d+))$/
+
+/**
+ * @typedef {Object} SatisfiedContentRangeHeader
+ * @property {number} start
+ * @property {number} end
+ * @property {number | null} size
+ */
+
+/**
+ * @typedef {Object} UnsatisfiedContentRangeHeader
+ * @property {0} start
+ * @property {null} end
+ * @property {number} size
+ */
+
+/**
+ * Parsed accordingly to RFC 9110
+ *
+ * @param {null|string} range
+ * @returns {null|SatisfiedContentRangeHeader|UnsatisfiedContentRangeHeader}
+ * @see https://www.rfc-editor.org/rfc/rfc9110#field.content-range
+ */
+function parseContentRangeHeader (range) {
+  if (range == null || range === '') {
+    return emptyRange
+  }
+
+  const m = range.match(rangeHeaderRE)
+  return m && ((m[4] && { start: 0, end: null, size: +m[4] }) || {
+    start: +m[1],
+    end: (m[2] && +m[2]) ?? null,
+    size: (m[3] && +m[3]) ?? null
+  })
 }
 
 const kEnumerableProperty = Object.create(null)
@@ -538,7 +560,7 @@ module.exports = {
   addAbortListener,
   isValidHTTPToken,
   isTokenCharCode,
-  parseRangeHeader,
+  parseContentRangeHeader,
   nodeMajor,
   nodeMinor,
   nodeHasAutoSelectFamily: nodeMajor > 18 || (nodeMajor === 18 && nodeMinor >= 13),

lib/handler/RetryHandler.js

@@ -2,7 +2,7 @@ const assert = require('node:assert')
 
 const { kRetryHandlerDefaultRetry } = require('../core/symbols')
 const { RequestRetryError } = require('../core/errors')
-const { isDisturbed, parseHeaders, parseRangeHeader } = require('../core/util')
+const { isDisturbed, parseHeaders, parseContentRangeHeader } = require('../core/util')
 
 function calculateRetryAfterHeader (retryAfter) {
   const current = Date.now()
@@ -58,7 +58,13 @@ class RetryHandler {
     }
 
     this.retryCount = 0
+    /**
+     * @type {number}
+     */
     this.start = 0
+    /**
+     * @type {number | null}
+     */
     this.end = null
     this.etag = null
     this.resume = null
@@ -198,9 +204,9 @@ class RetryHandler {
         return true
       }
 
-      const contentRange = parseRangeHeader(headers['content-range'])
+      const contentRange = parseContentRangeHeader(headers['content-range'])
       // If no content range
-      if (!contentRange) {
+      if (contentRange === null) {
         this.abort(
           new RequestRetryError('Content-Range mismatch', statusCode, {
             headers,
@@ -224,7 +230,7 @@ class RetryHandler {
       const { start, size, end = size } = contentRange
 
       assert(this.start === start, 'content-range mismatch')
-      assert(this.end == null || this.end === end, 'content-range mismatch')
+      assert(this.end === null || this.end === end, 'content-range mismatch')
 
       this.resume = resume
       return true
@@ -233,9 +239,9 @@ class RetryHandler {
     if (this.end == null) {
       if (statusCode === 206) {
         // First time we receive 206
-        const range = parseRangeHeader(headers['content-range'])
+        const contentRange = parseContentRangeHeader(headers['content-range'])
 
-        if (range == null) {
+        if (contentRange === null) {
           return this.handler.onHeaders(
             statusCode,
             rawHeaders,
@@ -244,15 +250,14 @@ class RetryHandler {
           )
         }
 
-        const { start, size, end = size } = range
+        const { start, size, end = size } = contentRange
 
         assert(
-          start != null && Number.isFinite(start) && this.start !== start,
+          this.start !== start,
           'content-range mismatch'
         )
-        assert(Number.isFinite(start))
         assert(
-          end != null && Number.isFinite(end) && this.end !== end,
+          end !== null && this.end !== end,
           'invalid content-length'
         )
 
@@ -268,7 +273,7 @@ class RetryHandler {
 
       assert(Number.isFinite(this.start))
       assert(
-        this.end == null || Number.isFinite(this.end),
+        this.end === null || Number.isFinite(this.end),
         'invalid content-length'
       )
 

test/utils/parse-content-range-header.js

@@ -0,0 +1,58 @@
+'use strict'
+
+const { strictEqual, deepStrictEqual } = require('node:assert')
+const { test, describe } = require('node:test')
+const { parseContentRangeHeader } = require('../../lib/core/util')
+
+describe('parseContentRangeHeader', () => {
+  test('empty string', () => {
+    deepStrictEqual(parseContentRangeHeader(''), { start: 0, end: null, size: null })
+    strictEqual(Object.isFrozen(parseContentRangeHeader('')), true)
+  })
+
+  test('undefined', () => {
+    deepStrictEqual(parseContentRangeHeader(), { start: 0, end: null, size: null })
+    strictEqual(Object.isFrozen(parseContentRangeHeader()), true)
+  })
+
+  test('null', () => {
+    deepStrictEqual(parseContentRangeHeader(), { start: 0, end: null, size: null })
+    strictEqual(Object.isFrozen(parseContentRangeHeader()), true)
+  })
+
+  test('invalid', () => {
+    deepStrictEqual(parseContentRangeHeader('invalid'), null)
+  })
+
+  test('bytes */*', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes */*'), null)
+  })
+
+  test('bytes 0-2', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes 0-2'), null)
+  })
+
+  test('bytes 0-2/', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes 0-2/'), null)
+  })
+
+  test('bytes 0-400/400', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes 0-400/400'), { start: 0, end: 400, size: 400 })
+  })
+
+  test('bytes 1-400/400', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes 1-400/400'), { start: 1, end: 400, size: 400 })
+  })
+
+  test('bytes 1-400/*', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes 1-400/*'), { start: 1, end: 400, size: null })
+  })
+
+  test('bytes 1-400/0', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes 1-400/0'), { start: 1, end: 400, size: 0 })
+  })
+
+  test('bytes */400', () => {
+    deepStrictEqual(parseContentRangeHeader('bytes */400'), { start: 0, end: null, size: 400 })
+  })
+})