mirror of
https://github.com/djdevin/recflare.git
synced 2026-09-08 14:41:28 -07:00
[cdn] fix some range issues where delivered images might be corrupt
This commit is contained in:
+49
-37
@@ -2,7 +2,13 @@ import { Hono } from 'hono'
|
||||
import { describeRoute, openAPIRouteHandler } from 'hono-openapi'
|
||||
import { useWorkersLogger } from 'workers-tagged-logger'
|
||||
|
||||
import { withCleanSpec, withNotFound, withOnError } from '@repo/hono-helpers'
|
||||
import {
|
||||
withCleanSpec,
|
||||
withDefaultCors,
|
||||
withNotFound,
|
||||
withOnError,
|
||||
writeContentRange,
|
||||
} from '@repo/hono-helpers'
|
||||
|
||||
import loadingScreenTipData from '../static/loading-screen-tip-data.json'
|
||||
import {
|
||||
@@ -23,38 +29,41 @@ import type { App, Env } from './context'
|
||||
* streamed out of the shared `recflare-cdn` R2 bucket, keyed by prefix.
|
||||
*/
|
||||
|
||||
/** Parse a single-range `Range: bytes=start-end` header into an R2 range. */
|
||||
function parseRange(header: string | undefined): R2Range | undefined {
|
||||
if (!header) return undefined
|
||||
const m = /^bytes=(\d*)-(\d*)$/.exec(header.trim())
|
||||
if (!m) return undefined
|
||||
const start = m[1]
|
||||
const end = m[2]
|
||||
if (start === '' && end !== '') return { suffix: Number(end) } // last N bytes
|
||||
if (start !== '') {
|
||||
return end !== ''
|
||||
? { offset: Number(start), length: Number(end) - Number(start) + 1 }
|
||||
: { offset: Number(start) }
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Stream a binary asset from the CDN R2 bucket as application/octet-stream,
|
||||
* honoring Range requests. 404s when the file is missing.
|
||||
* Supports conditional GET and byte-range requests (206) — large-file
|
||||
* downloaders fetch in ranges, and a 200 where a 206 is expected corrupts the
|
||||
* reassembled file (e.g. EAC "Signatures don't match").
|
||||
*
|
||||
* This is why `cache.enabled` is false in wrangler.jsonc: Workers Caching strips `Range`
|
||||
* before the worker is invoked and slices the 206 out of its own cache, which silently
|
||||
* degrades to a whole-object 200 whenever the response is not cacheable. The range
|
||||
* answer has to be ours to guarantee.
|
||||
*/
|
||||
async function serveAsset(c: Context<App>, key: string) {
|
||||
if (key.includes('..')) return c.body(null, 400)
|
||||
|
||||
const ifNoneMatch = c.req.header('if-none-match')?.replace(/"/g, '')
|
||||
const range = parseRange(c.req.header('range'))
|
||||
const object = await (c.env as Env).CDN_ASSETS.get(key, {
|
||||
...(ifNoneMatch ? { onlyIf: { etagDoesNotMatch: ifNoneMatch } } : {}),
|
||||
...(range ? { range } : {}),
|
||||
})
|
||||
// R2 parses the `Range` header itself when handed the request headers, so there is no
|
||||
// grammar to reimplement here. It resolves every form (`bytes=a-b`, `bytes=a-`,
|
||||
// `bytes=-n`) to a concrete offset/length, and anything it cannot parse or satisfy to
|
||||
// the whole object — see the 206 branch, which is what turns that back into a 200.
|
||||
// With no `Range` header present this is an ordinary whole-object read.
|
||||
let object
|
||||
try {
|
||||
object = await (c.env as Env).CDN_ASSETS.get(key, {
|
||||
...(ifNoneMatch ? { onlyIf: { etagDoesNotMatch: ifNoneMatch } } : {}),
|
||||
range: c.req.raw.headers,
|
||||
})
|
||||
} catch (e) {
|
||||
// Defensive: R2 documents InvalidRange (10039) for a range it can't satisfy, which
|
||||
// is a 416 rather than the 500 the error handler would otherwise turn it into.
|
||||
// Locally it never fires — workerd resolves an unsatisfiable range to the whole
|
||||
// object instead of throwing — so this covers the service behaving as documented.
|
||||
if (e instanceof Error && e.message.includes('(10039)')) return c.body(null, 416)
|
||||
throw e
|
||||
}
|
||||
if (!object) return c.notFound()
|
||||
|
||||
const headers = new Headers()
|
||||
@@ -67,20 +76,12 @@ async function serveAsset(c: Context<App>, key: string) {
|
||||
// Precondition matched (If-None-Match) → R2 returns no body.
|
||||
if (!('body' in object)) return new Response(null, { status: 304, headers })
|
||||
|
||||
// Range honored → 206 Partial Content with Content-Range.
|
||||
if (object.range && c.req.header('range')) {
|
||||
// R2 hands back the RESOLVED range, and the object it returns carries all three
|
||||
// keys with the inapplicable ones set to undefined — so `'suffix' in r` is true
|
||||
// even for an offset/length range and cannot discriminate between the two forms.
|
||||
// (It read as a suffix range every time, making offset/length NaN and the
|
||||
// Content-Range header garbage.) Read the values, not the keys. A `bytes=-N`
|
||||
// request already comes back resolved to a concrete offset/length; the suffix
|
||||
// fallback below is only there in case that ever stops being true.
|
||||
const r = object.range as { offset?: number; length?: number; suffix?: number }
|
||||
const length = r.length ?? r.suffix ?? object.size - (r.offset ?? 0)
|
||||
const offset = r.offset ?? object.size - length
|
||||
headers.set('content-length', String(length))
|
||||
headers.set('content-range', `bytes ${offset}-${offset + length - 1}/${object.size}`)
|
||||
// A `bytes=` request is ALWAYS answered 206 with a Content-Range naming the bytes
|
||||
// actually enclosed — never a bare 200 carrying the whole object. That is the one
|
||||
// answer a chunked downloader cannot survive: it asked for a slice, so it writes
|
||||
// whatever comes back at that offset, and a whole-object body silently corrupts the
|
||||
// reassembled file (EAC "Signatures don't match"). See writeContentRange().
|
||||
if (writeContentRange(headers, c.req.raw.headers, object)) {
|
||||
return new Response(object.body, { status: 206, headers })
|
||||
}
|
||||
|
||||
@@ -98,6 +99,15 @@ const app = new Hono<App>()
|
||||
})(c, next)
|
||||
)
|
||||
|
||||
// The website lets a room's owner download their own scene blobs (see the room page
|
||||
// in `www`), which means a browser reading these bytes from another origin — without
|
||||
// these headers it can fetch them but not touch the result. `origin: '*'` gives away
|
||||
// nothing: every route here is already unauthenticated and public to anyone holding
|
||||
// the key, and nothing on this worker reads a cookie or a token, so there is no
|
||||
// ambient credential for `*` to expose. The keys are unguessable UUIDs, and that is
|
||||
// unchanged by who may read a response they already had to name exactly.
|
||||
.use('*', withDefaultCors())
|
||||
|
||||
.onError(withOnError())
|
||||
.notFound(withNotFound())
|
||||
|
||||
@@ -247,7 +257,9 @@ app.get(
|
||||
'byte ranges (`Range` → 206). The ranges matter: large-file downloaders fetch in',
|
||||
'chunks, and answering 200 where a 206 is expected corrupts the reassembled file —',
|
||||
'which surfaces as an anti-cheat “Signatures don’t match” failure, not a download',
|
||||
'error.',
|
||||
'error. So a `bytes=` request is never answered with a whole-object 200: the 206',
|
||||
'always carries a `Content-Range` stating which bytes the body holds, even where',
|
||||
'that turns out to be all of them.',
|
||||
].join('\n'),
|
||||
},
|
||||
servers: [{ url: 'https://cdn.recflare.net', description: 'Production' }],
|
||||
|
||||
@@ -46,6 +46,7 @@ export function assetResponses(description: string): OpenAPIV3_1.ResponsesObject
|
||||
304: { description: '`If-None-Match` matched the stored etag (no body)' },
|
||||
400: { description: 'The key contains `..` (no body)' },
|
||||
404: { description: 'No such object in the bucket' },
|
||||
416: { description: 'The `Range` header could not be satisfied (no body)' },
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,7 +57,7 @@ export const CONDITIONAL_HEADERS: OpenAPIV3_1.ParameterObject[] = [
|
||||
in: 'header',
|
||||
required: false,
|
||||
description:
|
||||
'A single byte range (`bytes=start-end`, `bytes=start-`, `bytes=-suffix`). Honoured with a 206; a malformed or multi-range value is ignored and the whole object served.',
|
||||
'A single byte range (`bytes=start-end`, `bytes=start-`, `bytes=-suffix`), parsed by R2 itself. Any `bytes=` value is answered 206 with a `Content-Range` naming the bytes enclosed — never a bare 200 carrying the whole object, which a chunked downloader would write at the offset it asked for. A multi-range or unsatisfiable value yields the whole object, but says so in the `Content-Range`. A unit other than `bytes` is ignored (200).',
|
||||
schema: { type: 'string', example: 'bytes=0-1023' },
|
||||
},
|
||||
{
|
||||
|
||||
@@ -73,6 +73,44 @@ describe('cdn endpoints', () => {
|
||||
expect(new Uint8Array(await suffix.arrayBuffer())).toEqual(new Uint8Array([14, 15]))
|
||||
})
|
||||
|
||||
// The corrupting answer to a byte-range request is a bare 200 carrying the whole
|
||||
// object: the downloader asked for a slice, so it writes the body at that offset and
|
||||
// the reassembled file is wrong (EAC "Signatures don't match"). R2 resolves a value
|
||||
// it cannot parse or satisfy to the WHOLE object rather than failing, so these are
|
||||
// exactly the inputs that used to fall through to a 200 — every one of them must
|
||||
// still come back 206 with a Content-Range stating what the body actually holds.
|
||||
test('GET /sigs/:sigName never answers a bytes range with a whole-object 200', async () => {
|
||||
await env.CDN_ASSETS.put('sigs/ranged3', new Uint8Array([10, 11, 12, 13, 14, 15]))
|
||||
const fetchRange = (range: string) =>
|
||||
exports.default.fetch(`${ORIGIN}/sigs/ranged3`, { headers: { Range: range } })
|
||||
|
||||
for (const range of [
|
||||
'bytes=100-200', // wholly past the end of a 6-byte object
|
||||
'bytes=abc', // not the byte-range grammar
|
||||
'bytes=0-1,3-4', // multi-range, which R2 does not serve
|
||||
'bytes=0-5', // satisfiable, and covers everything
|
||||
]) {
|
||||
const res = await fetchRange(range)
|
||||
expect(res.status, range).toBe(206)
|
||||
expect(res.headers.get('content-range'), range).toBe('bytes 0-5/6')
|
||||
}
|
||||
|
||||
// A range that runs off the end but starts inside is a real partial read.
|
||||
const partial = await fetchRange('bytes=4-99')
|
||||
expect(partial.status).toBe(206)
|
||||
expect(partial.headers.get('content-range')).toBe('bytes 4-5/6')
|
||||
expect(new Uint8Array(await partial.arrayBuffer())).toEqual(new Uint8Array([14, 15]))
|
||||
|
||||
// A unit other than bytes must be ignored outright — RFC 9110 — not answered
|
||||
// with a byte-denominated Content-Range.
|
||||
const other = await fetchRange('items=0-1')
|
||||
expect(other.status).toBe(200)
|
||||
expect(other.headers.get('content-range')).toBeNull()
|
||||
expect(new Uint8Array(await other.arrayBuffer())).toEqual(
|
||||
new Uint8Array([10, 11, 12, 13, 14, 15])
|
||||
)
|
||||
})
|
||||
|
||||
test('GET /room/:dataBlob streams the room blob from R2', async () => {
|
||||
await env.CDN_ASSETS.put('room/94tp5zjtwz0gppp8xlv1j9l5b.room', new Uint8Array([9, 8, 7]))
|
||||
const res = await exports.default.fetch(`${ORIGIN}/room/94tp5zjtwz0gppp8xlv1j9l5b.room`)
|
||||
@@ -86,6 +124,20 @@ describe('cdn endpoints', () => {
|
||||
expect(res.status).toBe(404)
|
||||
})
|
||||
|
||||
// The website lets a room's owner download their own scene data (the room page in
|
||||
// `www`), which is a browser reading these bytes from another origin. Without the
|
||||
// header it can fetch them but not read the result — and the page can't tell that
|
||||
// apart from the blob being gone.
|
||||
test('answers CORS so a browser on another origin can read a blob', async () => {
|
||||
await env.CDN_ASSETS.put('room/2026-08-01/cors-check', new Uint8Array([4, 2]))
|
||||
const res = await exports.default.fetch(`${ORIGIN}/room/2026-08-01/cors-check`, {
|
||||
headers: { origin: 'https://www.example.net' },
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
expect(res.headers.get('access-control-allow-origin')).toBe('*')
|
||||
expect(new Uint8Array(await res.arrayBuffer())).toEqual(new Uint8Array([4, 2]))
|
||||
})
|
||||
|
||||
test('GET /invention/:dataBlob streams the invention blob from R2', async () => {
|
||||
// Date-foldered, `.inv`-suffixed — the name the storage worker generates and the
|
||||
// api worker hands back as the invention's BlobName.
|
||||
|
||||
+10
-1
@@ -4,8 +4,17 @@
|
||||
"main": "src/cdn.app.ts",
|
||||
"compatibility_date": "2026-06-16",
|
||||
"compatibility_flags": ["nodejs_compat"],
|
||||
// Workers Caching is OFF here, and must stay off: it STRIPS the `Range` header before
|
||||
// invoking the worker, asks for the whole body, and slices the 206 out of its own
|
||||
// cache. That works only while the response is actually cacheable — on any bypass
|
||||
// (see the automatic bypass rules) nothing slices, and the client that asked for a
|
||||
// byte range receives the whole object with a 200. A chunked downloader writes that
|
||||
// at the offset it asked for and the reassembled file is corrupt (EAC "Signatures
|
||||
// don't match"). With caching off the `Range` header reaches serveAsset, which
|
||||
// always answers a `bytes=` request with a 206 and a truthful Content-Range.
|
||||
// The cost is that every asset read hits R2; correctness on these blobs is worth it.
|
||||
"cache": {
|
||||
"enabled": true
|
||||
"enabled": false
|
||||
},
|
||||
// CDN binaries (signature blobs + room build data) are stored as R2 objects
|
||||
// and streamed back by key. Keys are prefixed `sigs/` and `room/`.
|
||||
|
||||
+39
-5
@@ -3,7 +3,7 @@ import { Hono } from 'hono'
|
||||
import { describeRoute, openAPIRouteHandler } from 'hono-openapi'
|
||||
import { useWorkersLogger } from 'workers-tagged-logger'
|
||||
|
||||
import { withCleanSpec, withNotFound, withOnError } from '@repo/hono-helpers'
|
||||
import { withCleanSpec, withNotFound, withOnError, writeContentRange } from '@repo/hono-helpers'
|
||||
|
||||
import { imageBytes, json, ServiceStatus } from './openapi'
|
||||
|
||||
@@ -373,6 +373,12 @@ app.get(
|
||||
'return JPEG with no `ETag` (the source etag no longer describes the body), and the',
|
||||
'`If-None-Match` precondition is skipped. An out-of-range or non-integer dimension is',
|
||||
'ignored and the original is served — never an error.',
|
||||
'',
|
||||
'A `Range` is honoured (206) only on the untouched stream, which is the only response',
|
||||
'that advertises `Accept-Ranges`. A transform decodes the whole image and a real',
|
||||
'signature covers the whole body, so those serve the entire result and ignore the',
|
||||
'header. Where a range does apply, a `bytes=` request is never answered with a bare',
|
||||
'200: the `Content-Range` always states which bytes the body holds.',
|
||||
].join('\n'),
|
||||
parameters: [
|
||||
{
|
||||
@@ -433,9 +439,22 @@ app.get(
|
||||
'Conditional request against the R2 object etag. Ignored when a transform is requested.',
|
||||
schema: { type: 'string' },
|
||||
},
|
||||
{
|
||||
name: 'Range',
|
||||
in: 'header',
|
||||
required: false,
|
||||
description: [
|
||||
'A single byte range, parsed by R2 itself. Honoured with a 206 on the untouched',
|
||||
'stream only — ignored when a transform or a real signature applies, since both',
|
||||
'need the whole image. A `bytes=` value never yields a bare 200: the',
|
||||
'`Content-Range` names the bytes enclosed even where that is all of them.',
|
||||
].join(' '),
|
||||
schema: { type: 'string', example: 'bytes=0-1023' },
|
||||
},
|
||||
],
|
||||
responses: {
|
||||
200: imageBytes('The image bytes (or the DefaultProfileImage.jpg fallback)'),
|
||||
206: imageBytes('A byte range of the stored image, when the request carried a `Range`'),
|
||||
304: { description: 'If-None-Match matched the stored object etag; no body' },
|
||||
400: { description: 'The key contained `..`; no body' },
|
||||
},
|
||||
@@ -468,10 +487,16 @@ app.get(
|
||||
// one. Skip the precondition when a transform is requested.
|
||||
const ifNoneMatch = transform ? undefined : c.req.header('if-none-match')?.replace(/"/g, '')
|
||||
const { bucket, objectKey } = resolveObject(c.env, key)
|
||||
const object = await bucket.get(
|
||||
objectKey,
|
||||
ifNoneMatch ? { onlyIf: { etagDoesNotMatch: ifNoneMatch } } : undefined
|
||||
)
|
||||
// A `Range` applies only to the untouched stream. Resizing decodes the whole image
|
||||
// and an RSA signature covers the whole body, so a ranged read there would produce
|
||||
// bytes that are not the range asked for — ask R2 for the range only when we are
|
||||
// going to hand its bytes straight back. R2 parses the header itself; see
|
||||
// writeContentRange() below for why it is never answered with a bare 200.
|
||||
const range = needsBody(transform, signing) ? undefined : c.req.raw.headers
|
||||
const object = await bucket.get(objectKey, {
|
||||
...(ifNoneMatch ? { onlyIf: { etagDoesNotMatch: ifNoneMatch } } : {}),
|
||||
...(range ? { range } : {}),
|
||||
})
|
||||
if (!object) {
|
||||
// Missing from both static and R2 → serve the bundled DefaultProfileImage.jpg
|
||||
// static asset so clients still get a valid image instead of a 404. Honour
|
||||
@@ -497,6 +522,15 @@ app.get(
|
||||
return finalizeImage(c.env, bytes, headers, transform, signing)
|
||||
}
|
||||
|
||||
// Only the untouched stream can honour a range, so only it advertises the fact.
|
||||
// The transformed and static-asset paths above serve the whole thing regardless,
|
||||
// which is the legal answer to a range you cannot honour — but claiming
|
||||
// `accept-ranges` there would invite a client to expect otherwise.
|
||||
headers.set('accept-ranges', 'bytes')
|
||||
if (writeContentRange(headers, c.req.raw.headers, object)) {
|
||||
return new Response(object.body, { status: 206, headers })
|
||||
}
|
||||
|
||||
return new Response(object.body, { headers })
|
||||
}
|
||||
)
|
||||
|
||||
@@ -140,6 +140,62 @@ describe('img endpoints', () => {
|
||||
expect(res.status).toBe(304)
|
||||
})
|
||||
|
||||
it('honors a Range request on the stored image with a 206', async () => {
|
||||
const res = await SELF.fetch(`${ORIGIN}/${R2_KEY}`, { headers: { Range: 'bytes=2-4' } })
|
||||
expect(res.status).toBe(206)
|
||||
expect(res.headers.get('content-range')).toBe('bytes 2-4/8')
|
||||
expect(res.headers.get('accept-ranges')).toBe('bytes')
|
||||
expect(new Uint8Array(await res.arrayBuffer())).toEqual(IMAGE_BYTES.slice(2, 5))
|
||||
})
|
||||
|
||||
// R2 resolves a range it cannot parse or satisfy to the WHOLE object rather than
|
||||
// failing. Handing that back as a bare 200 is the shape that corrupts a chunked
|
||||
// download — the client wrote a whole file where it expected a slice — so every one
|
||||
// of these still states what the body holds.
|
||||
it('never answers a bytes range with a whole-object 200', async () => {
|
||||
for (const range of ['bytes=100-200', 'bytes=abc', 'bytes=0-1,3-4', 'bytes=0-7']) {
|
||||
const res = await SELF.fetch(`${ORIGIN}/${R2_KEY}`, { headers: { Range: range } })
|
||||
expect(res.status, range).toBe(206)
|
||||
expect(res.headers.get('content-range'), range).toBe('bytes 0-7/8')
|
||||
}
|
||||
|
||||
// A unit other than bytes must be ignored outright (RFC 9110), not answered with
|
||||
// a byte-denominated Content-Range.
|
||||
const other = await SELF.fetch(`${ORIGIN}/${R2_KEY}`, { headers: { Range: 'items=0-1' } })
|
||||
expect(other.status).toBe(200)
|
||||
expect(other.headers.get('content-range')).toBeNull()
|
||||
})
|
||||
|
||||
// A resize decodes the whole image, so there is no meaningful slice of the source to
|
||||
// read — the range is ignored and the whole transformed result served, which is the
|
||||
// legal answer. What it must NOT do is claim a 206 over bytes it rebuilt. Runs against
|
||||
// the R2 path (a decodable JPEG borrowed from `static/`), since that is the one that
|
||||
// has a range to suppress; the static-asset path is never handed one at all.
|
||||
it('ignores a Range when a transform rebuilds the body', async () => {
|
||||
const real = await (await SELF.fetch(`${ORIGIN}/3DCharades.jpg`)).arrayBuffer()
|
||||
await env.IMAGES.put('ranged-transform.jpg', real, {
|
||||
httpMetadata: { contentType: 'image/jpeg' },
|
||||
})
|
||||
|
||||
const res = await SELF.fetch(`${ORIGIN}/ranged-transform.jpg?width=128`, {
|
||||
headers: { Range: 'bytes=0-9' },
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
expect(res.headers.get('content-range')).toBeNull()
|
||||
expect(res.headers.get('accept-ranges')).toBeNull()
|
||||
expect(jpegSize(new Uint8Array(await res.arrayBuffer())).width).toBe(128)
|
||||
|
||||
// Same for a real RSA signature, which covers the whole body (the test env binds
|
||||
// IMG_SIGNING_ENABLED on, so `?sig=p1` takes the signing path rather than the stub).
|
||||
const signed = await SELF.fetch(`${ORIGIN}/ranged-transform.jpg?sig=p1`, {
|
||||
headers: { Range: 'bytes=0-9' },
|
||||
})
|
||||
expect(signed.status).toBe(200)
|
||||
expect(signed.headers.get('content-range')).toBeNull()
|
||||
expect(signed.headers.get('content-signature')).toContain('key-id=KEY:RSA:p1.rec.net')
|
||||
expect(new Uint8Array(await signed.arrayBuffer()).byteLength).toBe(real.byteLength)
|
||||
})
|
||||
|
||||
it('serves the DefaultProfileImage.jpg fallback for a missing image', async () => {
|
||||
const res = await SELF.fetch(`${ORIGIN}/missing.png`)
|
||||
expect(res.status).toBe(200)
|
||||
|
||||
Reference in New Issue
Block a user