fix: etag caching HTTP headers (#2504)

denoland · Jun 25, 2024 · 72811da · 72811da
1 parent 0f6b4f6
commit 72811da
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 10 deletions.
diff --git a/src/build_cache.ts b/src/build_cache.ts
@@ -18,6 +18,7 @@ export interface StaticFile {
  hash: string | null;
  size: number;
  readable: ReadableStream<Uint8Array> | Uint8Array;
+ close(): void;
 }
 
 export interface BuildCache {
@@ -101,6 +102,7 @@ export class ProdBuildCache implements BuildCache {
  hash: info.hash,
  size: stat.size,
  readable: file.readable,
+ close: () => file.close(),
  };
  }
 

diff --git a/src/dev/dev_build_cache.ts b/src/dev/dev_build_cache.ts
@@ -50,6 +50,7 @@ export class MemoryBuildCache implements DevBuildCache {
  hash: processed.hash,
  readable: processed.content,
  size: processed.content.byteLength,
+ close: () => {},
  };
  }
 
@@ -65,6 +66,7 @@ export class MemoryBuildCache implements DevBuildCache {
  hash: null,
  size: stat.size,
  readable: file.readable,
+ close: () => file.close(),
  };
  } catch (_err) {
  return null;

diff --git a/src/middlewares/static_files.ts b/src/middlewares/static_files.ts
@@ -35,13 +35,15 @@ export function staticFiles<T>(): MiddlewareFn<T> {
  }
 
  if (req.method !== "GET" && req.method !== "HEAD") {
+ file.close();
  return new Response("Method Not Allowed", { status: 405 });
  }
 
  const cacheKey = url.searchParams.get(ASSET_CACHE_BUST_KEY);
  if (cacheKey !== null && BUILD_ID !== cacheKey) {
  url.searchParams.delete(ASSET_CACHE_BUST_KEY);
  const location = url.pathname + url.search;
+ file.close();
  return new Response(null, {
  status: 307,
  headers: {
@@ -67,15 +69,19 @@ export function staticFiles<T>(): MiddlewareFn<T> {
  } else {
  const ifNoneMatch = req.headers.get("If-None-Match");
  if (
- etag !== null &&
- (ifNoneMatch === etag || ifNoneMatch === "W/" + etag)
+ ifNoneMatch !== null &&
+ (ifNoneMatch === etag || ifNoneMatch === `W/"${etag}"`)
  ) {
+ file.close();
  return new Response(null, { status: 304, headers });
+ } else if (etag !== null) {
+ headers.set("Etag", `W/"${etag}"`);
  }
  }
 
  headers.set("Content-Length", String(file.size));
  if (req.method === "HEAD") {
+ file.close();
  return new Response(null, { status: 200, headers });
  }
 

diff --git a/src/middlewares/static_files_test.ts b/src/middlewares/static_files_test.ts
@@ -20,6 +20,7 @@ class MockBuildCache implements BuildCache {
  hash: info.hash,
  size: text.byteLength,
  readable: text,
+ close: () => {},
  });
  }
  }
@@ -79,18 +80,22 @@ Deno.test("static files - etag", async () => {
  { buildCache },
  );
 
- const headers = new Headers();
- headers.append("If-None-Match", "123");
  const cacheUrl = `/foo.css?${ASSET_CACHE_BUST_KEY}=${BUILD_ID}`;
- const res = await server.get(cacheUrl, { headers });
+ let res = await server.get(cacheUrl);
+ await res.body?.cancel();
+ expect(res.headers.get("Etag")).toEqual('W/"123"');
+
+ let headers = new Headers();
+ headers.append("If-None-Match", "123");
+ res = await server.get(cacheUrl, { headers });
  await res.body?.cancel();
  expect(res.status).toEqual(304);
 
- const headers2 = new Headers();
- headers2.append("If-None-Match", "W/123");
- const res2 = await server.get(cacheUrl, { headers });
- await res2.body?.cancel();
- expect(res2.status).toEqual(304);
+ headers = new Headers();
+ headers.append("If-None-Match", 'W/"123"');
+ res = await server.get(cacheUrl, { headers });
+ await res.body?.cancel();
+ expect(res.status).toEqual(304);
 });
 
 Deno.test("static files - 404 on missing favicon.ico", async () => {