Let browsers cache GetImage responses

Source/Process.ts

@@ -1871,25 +1871,63 @@ export class Process {
         ImageID: ImageID
       });
     },
-    GetImage: async (Data: object): Promise<Blob> => {
+    GetImage: async (Data: object): Promise<Response> => {
       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<string, string> = 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<Result> => {
       //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") {

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);
+});