From 1c48290eeb19537e6917a6486b4385aa8df3e734 Mon Sep 17 00:00:00 2001 From: "otoneko." Date: Sat, 11 Jul 2026 01:52:52 +0900 Subject: [PATCH 1/3] fix(serve-static): correct Range header parsing edge cases - `bytes=-N` (suffix range) now returns the last N bytes instead of the first N+1 bytes. - `bytes=0-0` now returns 1 byte instead of the whole file (the old `parseInt(...) || size - 1` treated a valid 0 as falsy). - An unsatisfiable range (start beyond EOF, or start > end) now returns 416 instead of crashing createReadStream with a RangeError. --- src/serve-static.ts | 66 ++++++++++++++++++++++++++++++++------- test/serve-static.test.ts | 47 ++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 12 deletions(-) diff --git a/src/serve-static.ts b/src/serve-static.ts index 3e1b053..06672fb 100644 --- a/src/serve-static.ts +++ b/src/serve-static.ts @@ -35,6 +35,48 @@ const getStats = (path: string) => { return stats } +// Parses a `Range` header value against a resource of the given `size`. +// Returns 416 when the range is syntactically valid but unsatisfiable (e.g. the +// start is beyond the end of the file). A malformed header (e.g. not matching +// `-` or `-` at all) is treated as "no range" and the +// whole file is served, matching the existing behavior for invalid ranges. +const parseRange = (range: string, size: number): { start: number; end: number } | 416 => { + const parts = range.replace(/^bytes=/, '').split('-', 2) + + let start: number + let end: number + let malformed = false + + if (parts[0] === '') { + // suffix-range: `bytes=-N` requests the last N bytes. + const suffixLength = parseInt(parts[1], 10) + if (Number.isNaN(suffixLength)) { + malformed = true + start = 0 + end = size - 1 + } else { + start = Math.max(size - suffixLength, 0) + end = size - 1 + } + } else { + start = parseInt(parts[0], 10) + if (Number.isNaN(start)) { + malformed = true + start = 0 + } + end = parts[1] === undefined || parts[1] === '' ? size - 1 : parseInt(parts[1], 10) + if (Number.isNaN(end)) { + end = size - 1 + } + } + + if (!malformed && (start >= size || start > end)) { + return 416 + } + + return { start, end: Math.min(end, size - 1) } +} + type Decoder = (str: string) => string const tryDecode = (str: string, decoder: Decoder): string => { @@ -151,20 +193,20 @@ export const serveStatic = ( } else { c.header('Accept-Ranges', 'bytes') - const parts = range.replace(/bytes=/, '').split('-', 2) - const start = parseInt(parts[0], 10) || 0 - let end = parseInt(parts[1], 10) || size - 1 - if (size < end - start + 1) { - end = size - 1 - } - - const chunksize = end - start + 1 - const stream = createReadStream(path, { start, end }) + const parsedRange = parseRange(range, size) + if (parsedRange === 416) { + c.header('Content-Range', `bytes */${size}`) + result = c.body(null, 416) + } else { + const { start, end } = parsedRange + const chunksize = end - start + 1 + const stream = createReadStream(path, { start, end }) - c.header('Content-Length', chunksize.toString()) - c.header('Content-Range', `bytes ${start}-${end}/${stats.size}`) + c.header('Content-Length', chunksize.toString()) + c.header('Content-Range', `bytes ${start}-${end}/${size}`) - result = c.body(createStreamBody(stream), 206) + result = c.body(createStreamBody(stream), 206) + } } await options.onFound?.(path, c) diff --git a/test/serve-static.test.ts b/test/serve-static.test.ts index b0e4bb7..770af5e 100644 --- a/test/serve-static.test.ts +++ b/test/serve-static.test.ts @@ -185,6 +185,53 @@ describe('Serve Static Middleware', () => { expect(res.headers['content-range']).toBe('bytes 0-16/17') }) + it('Should return the last N bytes for a suffix range', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=-5') + expect(res.status).toBe(206) + expect(res.headers['content-length']).toBe('5') + expect(res.headers['content-range']).toBe('bytes 12-16/17') + expect(res.text).toBe('n.txt') + }) + + it('Should return the whole file for a suffix range exceeding the file size', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=-100') + expect(res.status).toBe(206) + expect(res.headers['content-range']).toBe('bytes 0-16/17') + expect(res.text).toBe('This is plain.txt') + }) + + it('Should return exactly 1 byte for range bytes=0-0', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=0-0') + expect(res.status).toBe(206) + expect(res.headers['content-length']).toBe('1') + expect(res.headers['content-range']).toBe('bytes 0-0/17') + expect(res.text).toBe('T') + }) + + it('Should return 416 when the range start is beyond the end of the file', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=100-200') + expect(res.status).toBe(416) + expect(res.headers['content-range']).toBe('bytes */17') + }) + + it('Should return 416 when the range start is beyond the file size, even if the window is small', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=20-25') + expect(res.status).toBe(416) + expect(res.headers['content-range']).toBe('bytes */17') + }) + + it('Should return 416 when the range start is after the range end', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=10-5') + expect(res.status).toBe(416) + expect(res.headers['content-range']).toBe('bytes */17') + }) + + it('Should return 416 for a zero-length suffix range', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=-0') + expect(res.status).toBe(416) + expect(res.headers['content-range']).toBe('bytes */17') + }) + it('Should handle the `onNotFound` option', async () => { const res = await request(server).get('/on-not-found/foo.txt') expect(res.status).toBe(404) From 52ccd9d8f2efd47339dd92f77e46a11af85dc455 Mon Sep 17 00:00:00 2001 From: "otoneko." Date: Sun, 12 Jul 2026 00:04:43 +0900 Subject: [PATCH 2/3] fix(serve-static): reject non-digit range values and fix empty-file crash - Range segments with trailing garbage (e.g. `bytes=0-1x`) were silently truncated by parseInt into `0-1` instead of being treated as malformed. first-pos/last-pos/suffix-length are now validated as `1*DIGIT`. - A Range request on an empty file fell through to createReadStream with end=-1 and crashed with 500 instead of returning 416. Addresses review feedback from @usualoma on #372. --- src/serve-static.ts | 45 +++++++++++++++++++-------------------- test/serve-static.test.ts | 26 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/serve-static.ts b/src/serve-static.ts index 06672fb..ef892cf 100644 --- a/src/serve-static.ts +++ b/src/serve-static.ts @@ -35,42 +35,41 @@ const getStats = (path: string) => { return stats } +// first-pos / last-pos / suffix-length are all `1*DIGIT` per RFC 9110 §14.1.2 - +// reject anything with non-digit characters instead of letting `parseInt` silently +// truncate trailing garbage (e.g. `1x` must not be accepted as `1`). +const isDigits = (str: string): boolean => /^\d+$/.test(str) + // Parses a `Range` header value against a resource of the given `size`. // Returns 416 when the range is syntactically valid but unsatisfiable (e.g. the -// start is beyond the end of the file). A malformed header (e.g. not matching -// `-` or `-` at all) is treated as "no range" and the -// whole file is served, matching the existing behavior for invalid ranges. +// start is beyond the end of the file, or the file is empty). A malformed header +// (e.g. not matching `-` or `-` at all) is treated as +// "no range" and the whole file is served, matching the existing behavior for +// invalid ranges. const parseRange = (range: string, size: number): { start: number; end: number } | 416 => { const parts = range.replace(/^bytes=/, '').split('-', 2) let start: number let end: number - let malformed = false - if (parts[0] === '') { + if (parts[0] === '' && isDigits(parts[1] ?? '')) { // suffix-range: `bytes=-N` requests the last N bytes. - const suffixLength = parseInt(parts[1], 10) - if (Number.isNaN(suffixLength)) { - malformed = true - start = 0 - end = size - 1 - } else { - start = Math.max(size - suffixLength, 0) - end = size - 1 - } - } else { + start = Math.max(size - parseInt(parts[1], 10), 0) + end = size - 1 + } else if ( + isDigits(parts[0]) && + (parts[1] === undefined || parts[1] === '' || isDigits(parts[1])) + ) { start = parseInt(parts[0], 10) - if (Number.isNaN(start)) { - malformed = true - start = 0 - } end = parts[1] === undefined || parts[1] === '' ? size - 1 : parseInt(parts[1], 10) - if (Number.isNaN(end)) { - end = size - 1 - } + } else { + // Malformed range: ignore it and serve the whole representation. + start = 0 + end = size - 1 } - if (!malformed && (start >= size || start > end)) { + // Also covers empty files: `size - 1` is -1, which is always < start (0). + if (start >= size || start > end) { return 416 } diff --git a/test/serve-static.test.ts b/test/serve-static.test.ts index 770af5e..62eb2a6 100644 --- a/test/serve-static.test.ts +++ b/test/serve-static.test.ts @@ -232,6 +232,32 @@ describe('Serve Static Middleware', () => { expect(res.headers['content-range']).toBe('bytes */17') }) + it('Should treat a range with trailing garbage as malformed and serve the whole file', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=0-1x') + expect(res.status).toBe(206) + expect(res.headers['content-range']).toBe('bytes 0-16/17') + expect(res.text).toBe('This is plain.txt') + }) + + it('Should treat a range with a non-numeric start as malformed and serve the whole file', async () => { + const res = await request(server).get('/static/plain.txt').set('range', 'bytes=x-1') + expect(res.status).toBe(206) + expect(res.headers['content-range']).toBe('bytes 0-16/17') + expect(res.text).toBe('This is plain.txt') + }) + + it('Should return 416 instead of crashing for a range request on an empty file', async () => { + const res = await request(server).get('/static/foo..bar.txt').set('range', 'bytes=0-0') + expect(res.status).toBe(416) + expect(res.headers['content-range']).toBe('bytes */0') + }) + + it('Should return 416 instead of crashing for a malformed range on an empty file', async () => { + const res = await request(server).get('/static/foo..bar.txt').set('range', 'hello') + expect(res.status).toBe(416) + expect(res.headers['content-range']).toBe('bytes */0') + }) + it('Should handle the `onNotFound` option', async () => { const res = await request(server).get('/on-not-found/foo.txt') expect(res.status).toBe(404) From 8009f73ea3db96fd26e99d59ba912f8706dde7a3 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sun, 12 Jul 2026 22:06:26 +0900 Subject: [PATCH 3/3] refactor(serve-static): separate range parsing and resolution --- src/serve-static.ts | 95 ++++++++++++++++++++++----------------- test/serve-static.test.ts | 22 ++++----- 2 files changed, 64 insertions(+), 53 deletions(-) diff --git a/src/serve-static.ts b/src/serve-static.ts index ef892cf..6cf59ee 100644 --- a/src/serve-static.ts +++ b/src/serve-static.ts @@ -35,45 +35,53 @@ const getStats = (path: string) => { return stats } -// first-pos / last-pos / suffix-length are all `1*DIGIT` per RFC 9110 §14.1.2 - -// reject anything with non-digit characters instead of letting `parseInt` silently -// truncate trailing garbage (e.g. `1x` must not be accepted as `1`). -const isDigits = (str: string): boolean => /^\d+$/.test(str) - -// Parses a `Range` header value against a resource of the given `size`. -// Returns 416 when the range is syntactically valid but unsatisfiable (e.g. the -// start is beyond the end of the file, or the file is empty). A malformed header -// (e.g. not matching `-` or `-` at all) is treated as -// "no range" and the whole file is served, matching the existing behavior for -// invalid ranges. -const parseRange = (range: string, size: number): { start: number; end: number } | 416 => { - const parts = range.replace(/^bytes=/, '').split('-', 2) - - let start: number - let end: number - - if (parts[0] === '' && isDigits(parts[1] ?? '')) { - // suffix-range: `bytes=-N` requests the last N bytes. - start = Math.max(size - parseInt(parts[1], 10), 0) - end = size - 1 - } else if ( - isDigits(parts[0]) && - (parts[1] === undefined || parts[1] === '' || isDigits(parts[1])) - ) { - start = parseInt(parts[0], 10) - end = parts[1] === undefined || parts[1] === '' ? size - 1 : parseInt(parts[1], 10) - } else { - // Malformed range: ignore it and serve the whole representation. - start = 0 - end = size - 1 +type ByteRangeSpec = + | { type: 'bounded'; start: number; end: number } + | { type: 'open-ended'; start: number } + | { type: 'suffix'; length: number } + +type ByteRange = { start: number; end: number } + +const BYTE_RANGE_PATTERN = /^(?:bytes=)?(?!-$)(\d*)-(\d*)$/ + +const parseByteRange = (range: string): ByteRangeSpec | undefined => { + const match = range.match(BYTE_RANGE_PATTERN) + if (!match) { + return undefined + } + + const [, start, end] = match + + if (start === '') { + return { type: 'suffix', length: Number(end) } + } + + if (end === '') { + return { type: 'open-ended', start: Number(start) } + } + + return { type: 'bounded', start: Number(start), end: Number(end) } +} + +const resolveByteRange = (spec: ByteRangeSpec, size: number): ByteRange | undefined => { + if (size === 0) { + return undefined } - // Also covers empty files: `size - 1` is -1, which is always < start (0). - if (start >= size || start > end) { - return 416 + if (spec.type === 'suffix') { + if (spec.length === 0) { + return undefined + } + + return { start: Math.max(size - spec.length, 0), end: size - 1 } } - return { start, end: Math.min(end, size - 1) } + const end = spec.type === 'bounded' ? Math.min(spec.end, size - 1) : size - 1 + if (spec.start >= size || spec.start > end) { + return undefined + } + + return { start: spec.start, end } } type Decoder = (str: string) => string @@ -192,16 +200,23 @@ export const serveStatic = ( } else { c.header('Accept-Ranges', 'bytes') - const parsedRange = parseRange(range, size) - if (parsedRange === 416) { + // Preserve the existing behavior of serving the whole representation for + // a malformed range. + const rangeSpec: ByteRangeSpec = parseByteRange(range) ?? { + type: 'open-ended', + start: 0, + } + const resolvedRange = resolveByteRange(rangeSpec, size) + + if (!resolvedRange) { c.header('Content-Range', `bytes */${size}`) result = c.body(null, 416) } else { - const { start, end } = parsedRange - const chunksize = end - start + 1 + const { start, end } = resolvedRange + const chunkSize = end - start + 1 const stream = createReadStream(path, { start, end }) - c.header('Content-Length', chunksize.toString()) + c.header('Content-Length', chunkSize.toString()) c.header('Content-Range', `bytes ${start}-${end}/${size}`) result = c.body(createStreamBody(stream), 206) diff --git a/test/serve-static.test.ts b/test/serve-static.test.ts index 62eb2a6..681347a 100644 --- a/test/serve-static.test.ts +++ b/test/serve-static.test.ts @@ -232,19 +232,15 @@ describe('Serve Static Middleware', () => { expect(res.headers['content-range']).toBe('bytes */17') }) - it('Should treat a range with trailing garbage as malformed and serve the whole file', async () => { - const res = await request(server).get('/static/plain.txt').set('range', 'bytes=0-1x') - expect(res.status).toBe(206) - expect(res.headers['content-range']).toBe('bytes 0-16/17') - expect(res.text).toBe('This is plain.txt') - }) - - it('Should treat a range with a non-numeric start as malformed and serve the whole file', async () => { - const res = await request(server).get('/static/plain.txt').set('range', 'bytes=x-1') - expect(res.status).toBe(206) - expect(res.headers['content-range']).toBe('bytes 0-16/17') - expect(res.text).toBe('This is plain.txt') - }) + it.each(['bytes=0-1x', 'bytes=x-1', 'bytes=0-1-2', 'bytes=-5-extra'])( + 'Should treat a malformed range as the whole file: %s', + async (range) => { + const res = await request(server).get('/static/plain.txt').set('range', range) + expect(res.status).toBe(206) + expect(res.headers['content-range']).toBe('bytes 0-16/17') + expect(res.text).toBe('This is plain.txt') + } + ) it('Should return 416 instead of crashing for a range request on an empty file', async () => { const res = await request(server).get('/static/foo..bar.txt').set('range', 'bytes=0-0')