fix(serve-static): correct Range header parsing edge cases (#372)

* 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.

* 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.

* refactor(serve-static): separate range parsing and resolution

---------

Co-authored-by: Taku Amano <taku@taaas.jp>

Author: otnc

src/serve-static.ts

@@ -35,6 +35,55 @@ const getStats = (path: string) => {
   return stats
 }
 
+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
+  }
+
+  if (spec.type === 'suffix') {
+    if (spec.length === 0) {
+      return undefined
+    }
+
+    return { start: Math.max(size - spec.length, 0), 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
 
 const tryDecode = (str: string, decoder: Decoder): string => {
@@ -151,20 +200,27 @@ export const serveStatic = <E extends Env = any>(
     } 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
+      // 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)
 
-      const chunksize = end - start + 1
-      const stream = createReadStream(path, { start, end })
+      if (!resolvedRange) {
+        c.header('Content-Range', `bytes */${size}`)
+        result = c.body(null, 416)
+      } else {
+        const { start, end } = resolvedRange
+        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)

test/serve-static.test.ts

@@ -185,6 +185,75 @@ 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.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')
+    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)