From 048cf92b75cdbf2685ba32deed738b63b9b7c015 Mon Sep 17 00:00:00 2001 From: ytkimirti Date: Tue, 29 Sep 2026 18:07:35 +0300 Subject: [PATCH 1/4] feat: version the upload protocol --- src/browser/task.ts | 6 +++++- src/react/constraints.ts | 18 +++++++++++++----- src/server/handle-upload.ts | 5 +++-- src/shared/protocol.ts | 31 +++++++++++++++++++++++++++++++ src/shared/types.ts | 10 ++++++++-- test/unit/browser.test.ts | 24 ++++++++++++++++++++++-- test/unit/handler.test.ts | 14 +++++++------- test/unit/react.test.ts | 15 +++++++++++++++ test/unit/server.test.ts | 20 +++++++++++++++++++- 9 files changed, 123 insertions(+), 20 deletions(-) create mode 100644 src/shared/protocol.ts diff --git a/src/browser/task.ts b/src/browser/task.ts index 8b2b30a..ac2095a 100644 --- a/src/browser/task.ts +++ b/src/browser/task.ts @@ -1,4 +1,5 @@ import { BlobError } from '../shared/errors.ts'; +import { protocolOf, UPLOAD_PROTOCOL } from '../shared/protocol.ts'; import type { CompletedBlob, UploadSnapshot, UploadTask, WireBeginResponse, WireEndResponse, WireLanded, WirePartsResponse } from '../shared/types.ts'; import { SNIFF_BYTES } from '../shared/units.ts'; import { abortError, clock } from './clock.ts'; @@ -78,6 +79,8 @@ class Task implements InternalTask { private token: string | undefined; private storeKey: string; private partSize = 0; + /** The protocol the route advertised at 'begin'. A resume after a reload never asks, so it stays 1. */ + private protocol = 1; // Whether the route cut this file into real multipart parts. False is a single PUT: one url, one // object write, and nothing on the other side to pause into or resume from. private multipart = false; @@ -321,6 +324,7 @@ class Task implements InternalTask { // Never retried: begin runs onBeforeUpload, which inserts the app's row. 1, )) as WireBeginResponse; + this.protocol = protocolOf(res.protocol); this.token = res.completionToken; this.partSize = res.upload.partSize; this.multipart = res.upload.multipart !== false; @@ -552,7 +556,7 @@ class Task implements InternalTask { res = await fetch(this.options.route, { method: 'POST', headers: { 'content-type': 'application/json', ...authored }, - body: JSON.stringify(body), + body: JSON.stringify({ ...body, protocol: UPLOAD_PROTOCOL }), signal, }); } catch (e) { diff --git a/src/react/constraints.ts b/src/react/constraints.ts index 5021981..b2cdb54 100644 --- a/src/react/constraints.ts +++ b/src/react/constraints.ts @@ -2,6 +2,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { clock } from '../browser/clock.ts'; import { resolveHeaders, type HeadersProvider } from '../browser/task.ts'; import { BlobError } from '../shared/errors.ts'; +import { protocolOf } from '../shared/protocol.ts'; import type { ServedConstraints, WireConstraintsResponse } from '../shared/types.ts'; import { formatBytes } from '../shared/units.ts'; @@ -12,6 +13,11 @@ const inFlight = new Map>(); export interface RouteFacts { constraints: ServedConstraints | undefined; + /** + * The route's UPLOAD_PROTOCOL: 1 when it sent none, or could not be asked. Only an upload route's + * means anything: a useServerUpload url is the app's own, and its GET may say whatever it likes. + */ + protocol: number; /** Why the route could not be asked. Failed responses are not cached. */ error?: BlobError; } @@ -42,16 +48,16 @@ async function fetchFacts(route: string, headers: HeadersProvider | undefined): try { authored = await resolveHeaders(headers); } catch (e) { - return { constraints: undefined, error: BlobError.is(e) ? e : new BlobError('request_failed', { message: e instanceof Error ? e.message : String(e), status: 400, cause: e }) }; + return { constraints: undefined, protocol: 1, error: BlobError.is(e) ? e : new BlobError('request_failed', { message: e instanceof Error ? e.message : String(e), status: 400, cause: e }) }; } let res: Response; try { res = await fetch(route, { headers: authored }); } catch (e) { - return { constraints: undefined, error: new BlobError('request_failed', { message: 'could not reach the route', status: 503, cause: e }) }; + return { constraints: undefined, protocol: 1, error: new BlobError('request_failed', { message: 'could not reach the route', status: 503, cause: e }) }; } // Ordinary server-upload URLs commonly expose POST only. There are no route constraints to cache. - if (res.status === 404 || res.status === 405) return remember(route, { constraints: undefined }); + if (res.status === 404 || res.status === 405) return remember(route, { constraints: undefined, protocol: 1 }); if (!res.ok) { let body: unknown; try { @@ -59,16 +65,18 @@ async function fetchFacts(route: string, headers: HeadersProvider | undefined): } catch { body = undefined; } - return { constraints: undefined, error: BlobError.fromJSON(body, res.status) ?? BlobError.fromStatus(res.status) }; + return { constraints: undefined, protocol: 1, error: BlobError.fromJSON(body, res.status) ?? BlobError.fromStatus(res.status) }; } let constraints: ServedConstraints | undefined; + let protocol = 1; try { const body = (await res.json()) as WireConstraintsResponse | undefined; if (body?.constraints && typeof body.constraints === 'object') constraints = body.constraints; + protocol = protocolOf(body?.protocol); } catch { // Not a constraints document. The upload itself remains authoritative. } - return remember(route, { constraints }); + return remember(route, { constraints, protocol }); } function remember(route: string, facts: RouteFacts): RouteFacts { diff --git a/src/server/handle-upload.ts b/src/server/handle-upload.ts index 5cc695f..4e4715e 100644 --- a/src/server/handle-upload.ts +++ b/src/server/handle-upload.ts @@ -1,4 +1,5 @@ import { BlobError } from '../shared/errors.ts'; +import { UPLOAD_PROTOCOL } from '../shared/protocol.ts'; import type { UploadFile, WireBeginResponse, WireEndResponse, WireLanded, ServedConstraints, WireConstraintsResponse, WirePart, WirePartsResponse } from '../shared/types.ts'; import { cacheControl, formatBytes, parseSize, type CacheOption, type Size } from '../shared/units.ts'; import { r2Of, type Bucket } from './bucket.ts'; @@ -206,7 +207,7 @@ export function handleUpload(options: InternalUploadOptions): InternalUploadHand } throw e; } - return { completionToken, path: decided.path, upload: { partSize, multipart, parts } }; + return { protocol: UPLOAD_PROTOCOL, completionToken, path: decided.path, upload: { partSize, multipart, parts } }; } async function parts(body: any, details: ErrorDetails): Promise { @@ -422,7 +423,7 @@ export function constraintsEndpoint(routeConstraints: ResolvedConstraints): (req const served: ServedConstraints = {}; if (routeConstraints.contentTypes) served.contentTypes = routeConstraints.contentTypes; if (routeConstraints.maxSize !== undefined) served.maxSize = routeConstraints.maxSize; - const body = JSON.stringify({ constraints: served } satisfies WireConstraintsResponse); + const body = JSON.stringify({ protocol: UPLOAD_PROTOCOL, constraints: served } satisfies WireConstraintsResponse); const etag = `"${hash(body)}"`; return async (request: Request): Promise => { const headers = { 'content-type': 'application/json', 'cache-control': `public, max-age=${LIMITS_MAX_AGE}`, etag }; diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts new file mode 100644 index 0000000..bbab54c --- /dev/null +++ b/src/shared/protocol.ts @@ -0,0 +1,31 @@ +/** + * The version of the upload route protocol: what `uploadHandler` and `@upstash/blob/browser` (and + * the React hooks on top of it) say to each other. It is not the package version: a browser bundle + * and the server it talks to ship on their own schedules, and blob-py's handler speaks the same + * protocol from a different package entirely. + * + * It crosses the wire as a top-level `protocol` key in the JSON body, beside `phase` or + * `constraints`, never a header, so it needs no CORS change: the server puts it on the GET + * constraints document and on the phase 'begin' answer, the client puts it on every POST. The other + * answers and the errors carry none. Anything missing, or not a JSON number with an integer value + * of at least 1 (a boolean is not one), is 1, which is what every release before the field existed + * spoke. Both sides ignore keys they do not know; the rest of this depends on it. + * + * Every change is additive. Neither side has a minimum and nothing is ever refused over the + * number: a newer client uses a feature only once the server has advertised a protocol that has it, + * and a newer server answers an older client the way that client expects. At 1 there is nothing + * to gate, so the server does not read the client's number yet; protocolOf() is how it will. + * + * The GET number is cached for up to two minutes, and a rolling deploy or a rollback can answer + * with an older server than the one that advertised it. So a feature used at 'begin' is gated on + * the GET number and survives an answer without it, and a bare upload(), which sends no GET, never + * uses one; anything after 'begin' is gated on its answer. + * + * 1: phases begin, parts, end and cancel; the GET constraints document. + */ +export const UPLOAD_PROTOCOL = 1; + +/** The protocol the other side sent. Missing, or anything but a positive integer, is 1. */ +export function protocolOf(raw: unknown): number { + return typeof raw === 'number' && Number.isInteger(raw) && raw >= 1 ? raw : 1; +} diff --git a/src/shared/types.ts b/src/shared/types.ts index 469ec38..beec17c 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -69,6 +69,8 @@ export interface WireUploadPlan { } export interface WireBeginResponse { + /** The server's UPLOAD_PROTOCOL. Missing is 1. */ + protocol?: number; completionToken: string; path: string; upload: WireUploadPlan; @@ -92,11 +94,13 @@ export interface WireEndResponse { data: TData; } -export type WireRequest = +/** `protocol` is the client's UPLOAD_PROTOCOL, on every phase. Missing is 1. */ +export type WireRequest = { protocol?: number } & ( | { phase: 'begin'; file: WireFile; head?: string; input?: unknown } | { phase: 'parts'; completionToken: string; from: number } | { phase: 'end'; completionToken: string; parts?: WireLanded[] } - | { phase: 'cancel'; completionToken: string }; + | { phase: 'cancel'; completionToken: string } +); /* -------------------------------------------------------------- browser -- */ @@ -161,5 +165,7 @@ export type UploadRoute(); const sleeps: number[] = []; let timers: { ms: number; cb: () => void }[] = []; let wakers: (() => void)[] = []; -let calls: { phase: string; body: any }[] = []; +let calls: { phase: string; body: any; protocol: unknown }[] = []; let onPhase: (body: any) => unknown | Response; const restore: (() => void)[] = []; @@ -86,7 +86,9 @@ beforeAll(() => { const url = typeof input === 'string' ? input : input.url; if (!url.startsWith('/api/')) return real(input, init); const body = JSON.parse(init!.body as string); - calls.push({ phase: body.phase, body }); + // Kept apart so the body assertions stay about the phase; afterEach checks it on every call. + const { protocol, ...rest } = body; + calls.push({ phase: body.phase, body: rest, protocol }); const out = onPhase(body); if (out instanceof Response) return out; return Response.json(out); @@ -112,6 +114,8 @@ beforeEach(() => { }); afterEach(() => { expect(poolState().active).toBe(0); + // Every phase of every test: the client's protocol rides each POST. + expect(calls.map((c) => c.protocol)).toEqual(calls.map(() => 1)); }); async function settle(rounds = 6) { @@ -517,6 +521,22 @@ describe('a single PUT', () => { } }); + test('a route advertising a newer protocol, or none, is uploaded to the same way', async () => { + for (const advertised of [undefined, 2, 'x']) { + const from = calls.length; + const route = defaultRoute(3); + onPhase = (body) => { + const out = route(body) as Record; + return body.phase === 'begin' && advertised !== undefined ? { ...out, protocol: advertised } : out; + }; + const task = upload(png(), { route: '/api/upload' }); + await settle(); + ManualXhr.pending[0]!.respond(200, { etag: '"x"' }); + await expect(task.done).resolves.toMatchObject({ path: 'p' }); + expect(calls.slice(from).map((c) => c.phase)).toEqual(['begin', 'end']); + } + }); + test('cancel while queued rejects done with AbortError and never PUTs', async () => { let release!: () => void; onPhase = () => new Response(new ReadableStream({ start: (c) => (release = () => c.close()) })); diff --git a/test/unit/handler.test.ts b/test/unit/handler.test.ts index bde3638..5787d43 100644 --- a/test/unit/handler.test.ts +++ b/test/unit/handler.test.ts @@ -110,7 +110,7 @@ describe('dispatch', () => { const uploads = handler(); const res = await uploads.GET(new Request(url('large'))); expect(res.status).toBe(200); - expect(await res.json()).toEqual({ constraints: { maxSize: 2_000_000_000 } }); + expect(await res.json()).toEqual({ protocol: 1, constraints: { maxSize: 2_000_000_000 } }); const begin = await post(uploads, 'attachment', { phase: 'begin', file: { name: 'a.png', type: 'image/png', size: 10 } }); expect(begin.status).toBe(200); @@ -148,7 +148,7 @@ describe('dispatch', () => { const uploads = uploadHandler({ bucket: bucket(), constraints: { maxSize: '1mb' }, onBeforeUpload: () => ({ path: 'only/1.png' }) }); const begin = await began(uploads, undefined, { name: 'a.png', type: 'image/png', size: 10 }); expect(begin.path).toBe('only/1.png'); - expect(await (await uploads.GET(new Request(url()))).json()).toEqual({ constraints: { maxSize: 1_000_000 } }); + expect(await (await uploads.GET(new Request(url()))).json()).toEqual({ protocol: 1, constraints: { maxSize: 1_000_000 } }); // A name on the query is a client bound to some other handler: it does not silently get this // route, for GET or for POST. for (const name of ['anything', '__proto__', 'toString']) { @@ -231,11 +231,11 @@ describe('dispatch', () => { describe('defaults', () => { test('a route replaces constraints per key, and null clears one', async () => { const uploads = handler(); - expect(await (await uploads.GET(new Request(url('attachment')))).json()).toEqual({ constraints: { contentTypes: ['image/png'], maxSize: 20_000_000 } }); + expect(await (await uploads.GET(new Request(url('attachment')))).json()).toEqual({ protocol: 1, constraints: { contentTypes: ['image/png'], maxSize: 20_000_000 } }); // maxSize replaced, contentTypes cleared by null. - expect(await (await uploads.GET(new Request(url('large')))).json()).toEqual({ constraints: { maxSize: 2_000_000_000 } }); + expect(await (await uploads.GET(new Request(url('large')))).json()).toEqual({ protocol: 1, constraints: { maxSize: 2_000_000_000 } }); // maxSize replaced, the handler's type list inherited. - expect(await (await uploads.GET(new Request(url('avatar')))).json()).toEqual({ constraints: { contentTypes: ['image/png'], maxSize: 2_000_000 } }); + expect(await (await uploads.GET(new Request(url('avatar')))).json()).toEqual({ protocol: 1, constraints: { contentTypes: ['image/png'], maxSize: 2_000_000 } }); }); test('the inherited constraints still refuse', async () => { @@ -339,8 +339,8 @@ describe('defaults', () => { routes: { omega: { constraints: { maxSize: '1kb' } }, alpha: {} }, }); for (const uploads of [first, second]) { - expect(await (await uploads.GET(new Request(url('alpha')))).json()).toEqual({ constraints: { maxSize: 20_000_000 } }); - expect(await (await uploads.GET(new Request(url('omega')))).json()).toEqual({ constraints: { maxSize: 1_000 } }); + expect(await (await uploads.GET(new Request(url('alpha')))).json()).toEqual({ protocol: 1, constraints: { maxSize: 20_000_000 } }); + expect(await (await uploads.GET(new Request(url('omega')))).json()).toEqual({ protocol: 1, constraints: { maxSize: 1_000 } }); } }); }); diff --git a/test/unit/react.test.ts b/test/unit/react.test.ts index be12704..efa188a 100644 --- a/test/unit/react.test.ts +++ b/test/unit/react.test.ts @@ -3,6 +3,7 @@ import { afterAll, afterEach, beforeAll, beforeEach, expect, test } from 'bun:te import { act, createElement, StrictMode } from 'react'; import { clock } from '../../src/browser/clock.ts'; import { poolState } from '../../src/browser/pool.ts'; +import { loadFacts } from '../../src/react/constraints.ts'; import { BlobError, uploadHooks, useServerUpload, useUpload } from '../../src/react/index.ts'; import type { UploadRoute } from '../../src/shared/types.ts'; import { installRouter, installXhr, ManualXhr } from '../helpers/xhr.ts'; @@ -866,6 +867,20 @@ test('a direct upload is finishing between the last byte and the end response', expect(upload.status === 'done' || upload.status === 'finishing').toBe(true); }); +test("the route's protocol is read off its constraints, and 1 when it sends none", async () => { + const unused = async () => jsonResponse({}, 405); + const cases: [unknown, number][] = [[2, 2], [undefined, 1], ['2', 1], [0, 1]]; + const urls = cases.map(() => `/api/protocol/${++routeId}`); + const missing = `/api/protocol/${++routeId}`; + const routes: Parameters[0] = { [missing]: { GET: async () => jsonResponse({}, 404), POST: unused } }; + cases.forEach(([served], i) => { + routes[urls[i]!] = { GET: async () => jsonResponse({ ...(served === undefined ? {} : { protocol: served }), constraints: {} }), POST: unused }; + }); + restore.push(installRouter(routes)); + for (const [i, [, expected]] of cases.entries()) expect((await loadFacts(urls[i]!, undefined)).protocol).toBe(expected); + expect(await loadFacts(missing, undefined)).toEqual({ constraints: undefined, protocol: 1 }); +}); + /* -------------------------------------------------------- uploadHooks -- */ test('a route with no contentTypes is not asked for the file head', async () => { diff --git a/test/unit/server.test.ts b/test/unit/server.test.ts index 35cfd03..5236c58 100644 --- a/test/unit/server.test.ts +++ b/test/unit/server.test.ts @@ -643,11 +643,29 @@ describe('uploadHandler: the direct transport', () => { expect(res.headers.get('cache-control')).toBe('public, max-age=60'); const etag = res.headers.get('etag')!; expect(etag).toMatch(/^"[a-z0-9]+"$/); - expect(await res.json()).toEqual({ constraints: { maxSize: 1_000_000 } }); + expect(await res.json()).toEqual({ protocol: 1, constraints: { maxSize: 1_000_000 } }); const again = await route.GET(new Request('https://app.test/api/upload', { headers: { 'if-none-match': etag } })); expect(again.status).toBe(304); }); + test('the protocol is advertised on GET and begin, and nothing is refused over it', async () => { + resetCredentialCaches(); + r2Handler = beginR2; + const route = uploadHandler({ bucket: bucket(), onBeforeUpload: () => ({ path: 'a.png' }) }); + expect((await (await route.GET(new Request('https://app.test/api/upload'))).json()).protocol).toBe(1); + const file = { name: 'a.png', type: 'image/png', size: 10 }; + // Missing is 1, a newer client is answered the way it would be answered at 1, and a value that + // is not a version at all is ignored rather than turned into a refusal. + for (const protocol of [undefined, 1, 2, 99, 'x', -1, null]) { + const res = await post(route, { phase: 'begin', file, ...(protocol === undefined ? {} : { protocol }) }); + expect(res.status).toBe(200); + const started = (await res.json()) as WireBeginResponse; + expect(started.protocol).toBe(1); + const parts = await post(route, { phase: 'parts', completionToken: started.completionToken, from: 1, protocol }); + expect(parts.status).toBe(200); + } + }); + test('a file under the threshold is one presigned object PUT, with nothing created behind it', async () => { resetCredentialCaches(); r2Handler = beginR2; From e20a20664376c8496cd98bcabc62094fee078086 Mon Sep 17 00:00:00 2001 From: ytkimirti Date: Tue, 29 Sep 2026 18:10:50 +0300 Subject: [PATCH 2/4] test: expect protocol on the live constraints document --- test/live/handle-upload.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/live/handle-upload.test.ts b/test/live/handle-upload.test.ts index ed688c5..f9de21b 100644 --- a/test/live/handle-upload.test.ts +++ b/test/live/handle-upload.test.ts @@ -93,6 +93,7 @@ describe('GET', () => { expect(etag).toMatch(/^"/); expect((await chat.GET(new Request('https://app.test/api/upload', { headers: { 'if-none-match': etag } }))).status).toBe(304); expect(await res.json()).toEqual({ + protocol: 1, constraints: { contentTypes: ['image/png', 'image/jpeg', 'image/webp', 'application/pdf'], maxSize: 20_000_000 }, }); const img = uploadHandler({ bucket: pub, constraints: { contentTypes: ['image/*'] }, onBeforeUpload: () => ({ path: 'x' }) }); From 2ef0e9f983bb5670c6ca7218c06de401a0e6c55d Mon Sep 17 00:00:00 2001 From: ytkimirti Date: Thu, 1 Oct 2026 09:11:25 +0300 Subject: [PATCH 3/4] feat: stamp the protocol on every route answer --- src/browser/task.ts | 13 +++++++++---- src/server/handle-upload.ts | 18 +++++++++++++----- src/shared/protocol.ts | 33 +++++++++++++++++++-------------- src/shared/types.ts | 17 +++++++++-------- test/unit/browser.test.ts | 8 +++++--- test/unit/server.test.ts | 10 +++++++++- 6 files changed, 64 insertions(+), 35 deletions(-) diff --git a/src/browser/task.ts b/src/browser/task.ts index ac2095a..d496c3e 100644 --- a/src/browser/task.ts +++ b/src/browser/task.ts @@ -1,6 +1,6 @@ import { BlobError } from '../shared/errors.ts'; import { protocolOf, UPLOAD_PROTOCOL } from '../shared/protocol.ts'; -import type { CompletedBlob, UploadSnapshot, UploadTask, WireBeginResponse, WireEndResponse, WireLanded, WirePartsResponse } from '../shared/types.ts'; +import type { CompletedBlob, UploadSnapshot, UploadTask, WireAnswer, WireBeginResponse, WireEndResponse, WireLanded, WirePartsResponse } from '../shared/types.ts'; import { SNIFF_BYTES } from '../shared/units.ts'; import { abortError, clock } from './clock.ts'; import { acquire } from './pool.ts'; @@ -79,7 +79,10 @@ class Task implements InternalTask { private token: string | undefined; private storeKey: string; private partSize = 0; - /** The protocol the route advertised at 'begin'. A resume after a reload never asks, so it stays 1. */ + /** + * The protocol on the last answer to arrive. Calls overlap, so this is not a gate: a feature reads + * protocolOf() off the answer it holds, which a deploy may have had a different release write. + */ private protocol = 1; // Whether the route cut this file into real multipart parts. False is a single PUT: one url, one // object write, and nothing on the other side to pause into or resume from. @@ -324,7 +327,6 @@ class Task implements InternalTask { // Never retried: begin runs onBeforeUpload, which inserts the app's row. 1, )) as WireBeginResponse; - this.protocol = protocolOf(res.protocol); this.token = res.completionToken; this.partSize = res.upload.partSize; this.multipart = res.upload.multipart !== false; @@ -573,7 +575,10 @@ class Task implements InternalTask { } catch { json = undefined; } - if (res.ok) return json; + if (res.ok) { + this.protocol = protocolOf((json as WireAnswer | undefined)?.protocol); + return json; + } const err = BlobError.fromJSON(json, res.status) ?? BlobError.fromStatus(res.status, { message: routeMessage(json, res) }); if (classify(res.status) !== 'retry') throw err; last = err; diff --git a/src/server/handle-upload.ts b/src/server/handle-upload.ts index 4e4715e..02dafe9 100644 --- a/src/server/handle-upload.ts +++ b/src/server/handle-upload.ts @@ -105,13 +105,13 @@ export function handleUpload(options: InternalUploadOptions): InternalUploadHand if (!body || typeof body !== 'object' || typeof body.phase !== 'string') throw new BlobError('invalid_input', { message: 'expected a JSON body with a phase' }); switch (body.phase) { case 'begin': - return Response.json(await begin(request, body, details)); + return answer(await begin(request, body, details)); case 'parts': - return Response.json(await parts(body, details)); + return answer(await parts(body, details)); case 'end': - return Response.json(await end(request, body, details)); + return answer(await end(request, body, details)); case 'cancel': - return Response.json(await cancel(body, details)); + return answer(await cancel(body, details)); default: throw new BlobError('invalid_input', { message: `unknown phase ${String(body.phase)}` }); } @@ -207,7 +207,7 @@ export function handleUpload(options: InternalUploadOptions): InternalUploadHand } throw e; } - return { protocol: UPLOAD_PROTOCOL, completionToken, path: decided.path, upload: { partSize, multipart, parts } }; + return { completionToken, path: decided.path, upload: { partSize, multipart, parts } }; } async function parts(body: any, details: ErrorDetails): Promise { @@ -392,6 +392,14 @@ export function handleUpload(options: InternalUploadOptions): InternalUploadHand return { GET, POST }; } +/** + * Every successful answer carries the protocol of the server that wrote it. On a deploy, instances of + * two releases answer one upload side by side, so the client reads each answer by its own number. + */ +function answer(body: object): Response { + return Response.json({ ...body, protocol: UPLOAD_PROTOCOL }); +} + /** * Every refusal leaves a route as BlobError.toJSON(), so the browser rebuilds it with its code * intact and callers switch on error.code instead of reading status numbers and message text. diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index bbab54c..97776b5 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -4,22 +4,27 @@ * and the server it talks to ship on their own schedules, and blob-py's handler speaks the same * protocol from a different package entirely. * - * It crosses the wire as a top-level `protocol` key in the JSON body, beside `phase` or - * `constraints`, never a header, so it needs no CORS change: the server puts it on the GET - * constraints document and on the phase 'begin' answer, the client puts it on every POST. The other - * answers and the errors carry none. Anything missing, or not a JSON number with an integer value - * of at least 1 (a boolean is not one), is 1, which is what every release before the field existed - * spoke. Both sides ignore keys they do not know; the rest of this depends on it. + * One rule: every upload route JSON body carries it as a top-level `protocol` key, beside `phase` + * or `constraints`. The client puts it on every POST; the server on the GET constraints document and + * on every 2xx answer to a POST (begin, parts, end and cancel). Error answers carry none. It is + * never a header, so it needs no CORS change. + * Anything missing, or not a JSON number with an integer value of at least 1 (a boolean is not one), + * is 1, which is what every release before the field existed spoke. Both sides ignore keys they do + * not know; the rest of this depends on it. * - * Every change is additive. Neither side has a minimum and nothing is ever refused over the - * number: a newer client uses a feature only once the server has advertised a protocol that has it, - * and a newer server answers an older client the way that client expects. At 1 there is nothing - * to gate, so the server does not read the client's number yet; protocolOf() is how it will. + * Every change is additive. Neither side has a minimum and nothing is ever refused over the number: + * a newer server answers an older client the way that client expects, and a newer client sends new + * keys only where a server that ignores them still does the right thing. At 1 there is nothing to + * gate, so the server does not read the client's number yet; protocolOf() is how it will. * - * The GET number is cached for up to two minutes, and a rolling deploy or a rollback can answer - * with an older server than the one that advertised it. So a feature used at 'begin' is gated on - * the GET number and survives an answer without it, and a bare upload(), which sends no GET, never - * uses one; anything after 'begin' is gated on its answer. + * One upload is many requests over minutes or days, and on a deploy or a rollback instances of two + * releases answer it side by side. So each answer is read by its own number, never by one an earlier + * call returned: 'end' can reach an older server than 'begin' did. For the same reason the + * completion token never bumps its own `v` for an additive field, because an older instance refuses + * a token it does not know, and a newer instance treats a token without the field as the old + * behavior, because an older 'begin' minted it. The GET number is cached for up to two minutes and + * a bare upload() sends no GET, so a client choosing what to send at 'begin' can be wrong about the + * server: what it sends has to be one of those new keys an older server safely ignores. * * 1: phases begin, parts, end and cancel; the GET constraints document. */ diff --git a/src/shared/types.ts b/src/shared/types.ts index beec17c..6db0c8b 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -44,6 +44,11 @@ export interface ServedConstraints { maxSize?: number; } +/** On every successful answer: the UPLOAD_PROTOCOL of the server that wrote it. Missing is 1. */ +export interface WireAnswer { + protocol?: number; +} + export interface WirePart { n: number; url: string; @@ -68,9 +73,7 @@ export interface WireUploadPlan { multipart: boolean; } -export interface WireBeginResponse { - /** The server's UPLOAD_PROTOCOL. Missing is 1. */ - protocol?: number; +export interface WireBeginResponse extends WireAnswer { completionToken: string; path: string; upload: WireUploadPlan; @@ -81,7 +84,7 @@ export interface WireLanded { etag: string; } -export interface WirePartsResponse { +export interface WirePartsResponse extends WireAnswer { partSize: number; size: number; parts: WirePart[]; @@ -89,7 +92,7 @@ export interface WirePartsResponse { multipart: boolean; } -export interface WireEndResponse { +export interface WireEndResponse extends WireAnswer { blob: Omit & { uploadedAt: string }; data: TData; } @@ -164,8 +167,6 @@ export type UploadRoute { } }); - test('a route advertising a newer protocol, or none, is uploaded to the same way', async () => { - for (const advertised of [undefined, 2, 'x']) { + test('answers from servers of different protocols, as on a deploy, still make one upload', async () => { + // begin and end can be answered by two releases: every mix of newer, none and garbage. + for (const [atBegin, atEnd] of [[2, undefined], [undefined, 2], [2, 'x'], ['x', 1]]) { const from = calls.length; const route = defaultRoute(3); onPhase = (body) => { const out = route(body) as Record; - return body.phase === 'begin' && advertised !== undefined ? { ...out, protocol: advertised } : out; + const advertised = body.phase === 'begin' ? atBegin : atEnd; + return advertised === undefined ? out : { ...out, protocol: advertised }; }; const task = upload(png(), { route: '/api/upload' }); await settle(); diff --git a/test/unit/server.test.ts b/test/unit/server.test.ts index 5236c58..5677c63 100644 --- a/test/unit/server.test.ts +++ b/test/unit/server.test.ts @@ -648,7 +648,7 @@ describe('uploadHandler: the direct transport', () => { expect(again.status).toBe(304); }); - test('the protocol is advertised on GET and begin, and nothing is refused over it', async () => { + test('the protocol is on GET and every answer, and nothing is refused over it', async () => { resetCredentialCaches(); r2Handler = beginR2; const route = uploadHandler({ bucket: bucket(), onBeforeUpload: () => ({ path: 'a.png' }) }); @@ -663,7 +663,14 @@ describe('uploadHandler: the direct transport', () => { expect(started.protocol).toBe(1); const parts = await post(route, { phase: 'parts', completionToken: started.completionToken, from: 1, protocol }); expect(parts.status).toBe(200); + expect(((await parts.json()) as WirePartsResponse).protocol).toBe(1); + const cancel = await post(route, { phase: 'cancel', completionToken: started.completionToken, protocol }); + expect(await cancel.json()).toEqual({ protocol: 1, ok: true }); } + // A refusal carries none: it is BlobError.toJSON(), read by its code. + const refused = await post(route, { phase: 'parts', completionToken: 'nope', protocol: 1 }); + expect(refused.status).toBe(403); + expect(await refused.json()).not.toHaveProperty('protocol'); }); test('a file under the threshold is one presigned object PUT, with nothing created behind it', async () => { @@ -798,6 +805,7 @@ describe('uploadHandler: the direct transport', () => { const res = await post(named, { phase: 'end', completionToken: started.completionToken, parts: [{ n: 1, etag: '"p1"' }] }); expect(res.status).toBe(200); const completed = await res.json(); + expect(completed.protocol).toBe(1); expect(completed.blob.contentType).toBe('image/png'); expect(completed.data).toEqual({ ok: true }); expect(seen).toEqual({ From 53ef65d52899d6765510f9f8c3e891b5fdc10f6e Mon Sep 17 00:00:00 2001 From: CahidArda Date: Fri, 23 Oct 2026 15:41:45 +0300 Subject: [PATCH 4/4] fix tests after merging main: protocol on the 32MiB GET, and a 404 HEAD for cancel --- test/unit/handler.test.ts | 2 +- test/unit/server.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/unit/handler.test.ts b/test/unit/handler.test.ts index f9fb207..a607c22 100644 --- a/test/unit/handler.test.ts +++ b/test/unit/handler.test.ts @@ -477,7 +477,7 @@ test('a 32MiB upload limit accepts the exact file size and rejects one byte over constraints: { maxSize: '32MiB' }, onBeforeUpload: () => ({ path: 'large/file.bin' }), }); - expect(await (await uploads.GET(new Request(url()))).json()).toEqual({ constraints: { maxSize: 33_554_432 } }); + expect(await (await uploads.GET(new Request(url()))).json()).toEqual({ protocol: 1, constraints: { maxSize: 33_554_432 } }); const begin = await post(uploads, undefined, { phase: 'begin', file: { name: 'file.bin', type: 'application/octet-stream', size: 33_554_432 } }); expect(begin.status).toBe(200); expect((await begin.json()).upload.multipart).toBe(true); diff --git a/test/unit/server.test.ts b/test/unit/server.test.ts index 6ecb1a4..03c92e6 100644 --- a/test/unit/server.test.ts +++ b/test/unit/server.test.ts @@ -921,7 +921,8 @@ describe('uploadHandler: the direct transport', () => { test('the protocol is on GET and every answer, and nothing is refused over it', async () => { resetCredentialCaches(); - r2Handler = beginR2; + // cancel HEADs a single-PUT upload's object: answer that nothing was stored. + r2Handler = (call) => (call.method === 'HEAD' ? new Response(null, { status: 404 }) : beginR2(call)); const route = uploadHandler({ bucket: bucket(), onBeforeUpload: () => ({ path: 'a.png' }) }); expect((await (await route.GET(new Request('https://app.test/api/upload'))).json()).protocol).toBe(1); const file = { name: 'a.png', type: 'image/png', size: 10 };