diff --git a/Source/Process.ts b/Source/Process.ts index 73d589f..a2b5e50 100644 --- a/Source/Process.ts +++ b/Source/Process.ts @@ -1871,25 +1871,63 @@ export class Process { ImageID: ImageID }); }, - GetImage: async (Data: object): Promise => { + GetImage: async (Data: object): Promise => { const GithubImageRepo = "XMOJ-Script-dev/XMOJ-Script-Pictures"; ThrowErrorIfFailed(this.CheckParams(Data, { "ImageID": "string" })); - return await fetch(new URL("https://api.github.com/repos/" + GithubImageRepo + "/contents/" + Data["ImageID"] + "?1=1"), { - method: "GET", - headers: { - "Authorization": "Bearer " + this.GithubImagePAT, - "Accept": "application/vnd.github.v3.raw", - "User-Agent": "XMOJ-Script-Server" - } - }).then((Response) => { - return Response.blob(); - }).catch((Error) => { + // Image IDs are random and never reused, so a found image can be cached + // by browsers forever and revalidated by ID alone. Errors are never + // cached, or a transient GitHub failure would stick for a year. + // UploadImage only ever issues 32 lowercase letters; "logo" and + // "logo.png" are the project logo, kept in the pictures repo so it can + // be linked through GetImage. Anything else is rejected before it + // reaches the PAT-authenticated GitHub URL, where "../" segments would + // otherwise resolve to other paths. + const IsLogo = Data["ImageID"] === "logo" || Data["ImageID"] === "logo.png"; + if (!IsLogo && !/^[a-z]{32}$/.test(Data["ImageID"])) { + return new Response(null, {status: 400, headers: {"cache-control": "no-store"}}); + } + const ETag = "\"" + Data["ImageID"] + "\""; + // The logo can be replaced in place, so it gets a day of caching and no + // ID-based ETag; only random upload IDs are immutable. + const CacheHeaders: Record = IsLogo ? { + "content-type": "image/png", + "cache-control": "public, max-age=86400" + } : { + "content-type": "image/png", + "cache-control": "public, max-age=31536000, immutable", + "etag": ETag + }; + // If-None-Match on GET uses weak comparison over a comma-separated list + // (RFC 9110 13.1.2), so ignore W/ prefixes; "*" falls through to a 200. + const IfNoneMatch = this.RequestData.headers.get("If-None-Match") || ""; + if (!IsLogo && IfNoneMatch.split(",").some((Tag) => Tag.trim().replace(/^W\//, "") === ETag)) { + return new Response(null, {status: 304, headers: CacheHeaders}); + } + let GithubResponse: Response; + try { + GithubResponse = await fetch(new URL("https://api.github.com/repos/" + GithubImageRepo + "/contents/" + Data["ImageID"]), { + method: "GET", + headers: { + "Authorization": "Bearer " + this.GithubImagePAT, + "Accept": "application/vnd.github.v3.raw", + "User-Agent": "XMOJ-Script-Server" + } + }); + } catch (Error) { Output.Error("Get image failed: " + Error + "\n" + "ImageID : \"" + Data["ImageID"] + "\"\n"); - return new Blob(); - }); + return new Response(null, {status: 502, headers: {"cache-control": "no-store"}}); + } + if (!GithubResponse.ok) { + return new Response(GithubResponse.body, { + status: GithubResponse.status, + headers: {"content-type": "application/json", "cache-control": "no-store"} + }); + } + // Stream the bytes through instead of buffering the whole image first. + return new Response(GithubResponse.body, {headers: CacheHeaders}); }, SendData: async (): Promise => { //instantly return @@ -2084,12 +2122,8 @@ export class Process { throw new Result(false, "访问的页面不存在"); } if (this.RequestData.method === "GET" && PathName === "GetImage") { - return new Response(await this.ProcessFunctions[PathName]({ + return await this.ProcessFunctions[PathName]({ ImageID: new URL(this.RequestData.url).searchParams.get("ImageID") - }), { - headers: { - "content-type": "image/png" - } }); } if (this.RequestData.method !== "POST") { diff --git a/test/process.test.js b/test/process.test.js index 69d45e1..d056e6e 100644 --- a/test/process.test.js +++ b/test/process.test.js @@ -1563,3 +1563,94 @@ test('LogoutAll still succeeds when the notification socket cannot be reached', const result = await proc.ProcessFunctions['LogoutAll']({}); assert.ok(result.Success); }); + +// GetImage: image IDs are random and never reused, so a found image can be +// cached by browsers forever. Without cache headers every page view re-fetched +// the image through the Worker and the GitHub API (~1-2s each time). +const ImageID = 'gkjfwuififximrruzslgzskeysazzbgh'; + +function imageRequest(headers = {}) { + return new Request('https://assets.xmoj-script.uk/GetImage?ImageID=' + ImageID, { headers }); +} + +test('GetImage marks a found image immutable and streams its bytes', async () => { + const bytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); + const proc = createProcess({ req: imageRequest(), fetch: async () => new Response(bytes) }); + const response = await proc.Process(); + assert.strictEqual(response.status, 200); + assert.strictEqual(response.headers.get('content-type'), 'image/png'); + assert.strictEqual(response.headers.get('cache-control'), 'public, max-age=31536000, immutable'); + assert.strictEqual(response.headers.get('etag'), '"' + ImageID + '"'); + assert.deepStrictEqual(new Uint8Array(await response.arrayBuffer()), bytes); +}); + +test('GetImage answers a matching If-None-Match with 304 without calling GitHub', async () => { + const fetchMock = test.mock.fn(async () => new Response('unexpected')); + const proc = createProcess({ req: imageRequest({ 'If-None-Match': '"' + ImageID + '"' }), fetch: fetchMock }); + const response = await proc.Process(); + assert.strictEqual(response.status, 304); + assert.strictEqual(response.headers.get('etag'), '"' + ImageID + '"'); + assert.strictEqual(fetchMock.mock.calls.length, 0); +}); + +test('GetImage passes a GitHub error through and never caches it', async () => { + const proc = createProcess({ req: imageRequest(), fetch: async () => new Response('{"message":"Not Found"}', { status: 404 }) }); + const response = await proc.Process(); + assert.strictEqual(response.status, 404); + assert.strictEqual(response.headers.get('cache-control'), 'no-store'); + assert.strictEqual(response.headers.get('etag'), null); +}); + +test('GetImage returns 502 without caching when GitHub is unreachable', async () => { + const proc = createProcess({ req: imageRequest(), fetch: async () => { throw new TypeError('network down'); } }); + const response = await proc.Process(); + assert.strictEqual(response.status, 502); + assert.strictEqual(response.headers.get('cache-control'), 'no-store'); +}); + +test('GetImage rejects IDs that UploadImage could not have produced, before calling GitHub', async () => { + for (const bad of ['..%2F..%2FREADME.md', 'README.md', 'logo.svg', 'ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEF', 'a b', '']) { + const fetchMock = test.mock.fn(async () => new Response('unexpected')); + const req = new Request('https://assets.xmoj-script.uk/GetImage?ImageID=' + bad); + const proc = createProcess({ req, fetch: fetchMock }); + const response = await proc.Process(); + assert.strictEqual(response.status, 400, 'ImageID=' + bad); + assert.strictEqual(response.headers.get('cache-control'), 'no-store'); + assert.strictEqual(fetchMock.mock.calls.length, 0, 'ImageID=' + bad); + } +}); + +test('GetImage If-None-Match uses weak comparison over a tag list', async () => { + for (const header of ['W/"' + ImageID + '"', '"other", "' + ImageID + '"', '"x",W/"' + ImageID + '"']) { + const fetchMock = test.mock.fn(async () => new Response('unexpected')); + const proc = createProcess({ req: imageRequest({ 'If-None-Match': header }), fetch: fetchMock }); + const response = await proc.Process(); + assert.strictEqual(response.status, 304, header); + assert.strictEqual(fetchMock.mock.calls.length, 0, header); + } + const proc = createProcess({ req: imageRequest({ 'If-None-Match': '"other"' }), fetch: async () => new Response('png') }); + assert.strictEqual((await proc.Process()).status, 200); +}); + +test('GetImage serves the project logo by its fixed IDs', async () => { + for (const id of ['logo', 'logo.png']) { + const fetchMock = test.mock.fn(async () => new Response('png')); + const req = new Request('https://assets.xmoj-script.uk/GetImage?ImageID=' + id); + const proc = createProcess({ req, fetch: fetchMock }); + const response = await proc.Process(); + assert.strictEqual(response.status, 200, id); + assert.strictEqual(fetchMock.mock.calls.length, 1, id); + assert.ok(String(fetchMock.mock.calls[0].arguments[0]).endsWith('/contents/' + id), id); + } +}); + +test('GetImage does not pin the logo forever: it can be replaced, unlike random IDs', async () => { + const fetchMock = test.mock.fn(async () => new Response('png')); + const req = new Request('https://assets.xmoj-script.uk/GetImage?ImageID=logo.png', { headers: { 'If-None-Match': '"logo.png"' } }); + const proc = createProcess({ req, fetch: fetchMock }); + const response = await proc.Process(); + assert.strictEqual(response.status, 200); + assert.strictEqual(fetchMock.mock.calls.length, 1); + assert.strictEqual(response.headers.get('cache-control'), 'public, max-age=86400'); + assert.strictEqual(response.headers.get('etag'), null); +});