From df8193d558c808137c596aeee321a683d5655d03 Mon Sep 17 00:00:00 2001 From: uzlopak Date: Mon, 19 Feb 2024 01:10:27 +0100 Subject: [PATCH] imrpove parseContentRangeHeader --- benchmarks/parseContentRangeHeader.mjs | 19 ++++++++ lib/core/util.js | 50 ++++++++++++++------ lib/handler/RetryHandler.js | 27 ++++++----- test/utils/parse-content-range-header.js | 58 ++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 25 deletions(-) create mode 100644 benchmarks/parseContentRangeHeader.mjs create mode 100644 test/utils/parse-content-range-header.js diff --git a/benchmarks/parseContentRangeHeader.mjs b/benchmarks/parseContentRangeHeader.mjs new file mode 100644 index 00000000000..532aa6db428 --- /dev/null +++ b/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() diff --git a/lib/core/util.js b/lib/core/util.js index 1ad0eab89fd..eee2e8f2c22 100644 --- a/lib/core/util.js +++ b/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), diff --git a/lib/handler/RetryHandler.js b/lib/handler/RetryHandler.js index 2a2f878c00f..7f48084b448 100644 --- a/lib/handler/RetryHandler.js +++ b/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' ) diff --git a/test/utils/parse-content-range-header.js b/test/utils/parse-content-range-header.js new file mode 100644 index 00000000000..f421da79b6e --- /dev/null +++ b/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 }) + }) +})