cacheable-request - fix: 304 Not Modified has completely empty headers

packages/cacheable-request/src/index.ts

@@ -125,18 +125,52 @@ class CacheableRequest {
 				const handler = async (response: any) => {
 					if (revalidate) {
 						response.status = response.statusCode;
-						const revalidatedPolicy = CachePolicy.fromObject(
+						const originalPolicy = CachePolicy.fromObject(
 							revalidate.cachePolicy,
-						).revalidatedPolicy(options_, response);
+						);
+						const revalidatedPolicy = originalPolicy.revalidatedPolicy(
+							options_,
+							response,
+						);
 						if (!revalidatedPolicy.modified) {
 							response.resume();
 							await new Promise((resolve) => {
 								// Skipping 'error' handler cause 'error' event should't be emitted for 304 response
 								response.once("end", resolve);
 							});
+							// Get headers from revalidated policy
 							const headers = convertHeaders(
 								revalidatedPolicy.policy.responseHeaders(),
 							);
+
+							// Preserve headers from the original cached response that may have been
+							// lost during revalidation (e.g., content-encoding, content-type, etc.)
+							// This works around a limitation in http-cache-semantics where some headers
+							// are not preserved when a 304 response has minimal headers
+							const originalHeaders = convertHeaders(
+								originalPolicy.responseHeaders(),
+							);
+
+							// Headers that should be preserved from the cached response
+							// according to RFC 7232 section 4.1
+							const preserveHeaders = [
+								"content-encoding",
+								"content-type",
+								"content-length",
+								"content-language",
+								"content-location",
+								"etag",
+							];
+
+							for (const headerName of preserveHeaders) {
+								if (
+									originalHeaders[headerName] !== undefined &&
+									headers[headerName] === undefined
+								) {
+									headers[headerName] = originalHeaders[headerName];
+								}
+							}
+
 							response = new Response({
 								statusCode: revalidate.statusCode,
 								headers,
@@ -191,18 +225,22 @@ class CacheableRequest {
 								}
 
 								await this.cache.set(key, value, ttl);
+								/* c8 ignore next -- @preserve */
 							} catch (error: any) {
-								/* c8 ignore next 2 */
+								/* c8 ignore next -- @preserve */
 								ee.emit("error", new CacheError(error));
+								/* c8 ignore next -- @preserve */
 							}
 						})();
 					} else if (options_.cache && revalidate) {
 						(async () => {
 							try {
 								await this.cache.delete(key);
+								/* c8 ignore next -- @preserve */
 							} catch (error: any) {
-								/* c8 ignore next 2 */
+								/* c8 ignore next -- @preserve */
 								ee.emit("error", new CacheError(error));
+								/* c8 ignore next -- @preserve */
 							}
 						})();
 					}

packages/cacheable-request/test/304-header-preservation.test.ts

@@ -0,0 +1,87 @@
+import { request } from "node:http";
+import { promisify as pm } from "node:util";
+import { gzip } from "node:zlib";
+import delay from "delay";
+import getStream from "get-stream";
+import { afterAll, beforeAll, expect, test } from "vitest";
+import CacheableRequest from "../src/index.js";
+import createTestServer from "./create-test-server/index.mjs";
+
+// Promisify cacheableRequest
+// biome-ignore lint/suspicious/noExplicitAny: legacy test format
+const promisify = (cacheableRequest: any) => async (options: any) =>
+	new Promise((resolve, reject) => {
+		// biome-ignore lint/suspicious/noExplicitAny: legacy test format
+		cacheableRequest(options, async (response: any) => {
+			const body = await getStream(response);
+			response.body = body;
+			// Give the cache time to update
+			await delay(100);
+			resolve(response);
+		})
+			// biome-ignore lint/suspicious/noExplicitAny: legacy test format
+			.on("request", (request_: any) => request_.end())
+			.once("error", reject);
+	});
+
+// biome-ignore lint/suspicious/noExplicitAny: legacy test format
+let s: any;
+
+beforeAll(async () => {
+	s = await createTestServer();
+	// biome-ignore lint/suspicious/noExplicitAny: legacy test format
+	s.get("/compress-bug", async (request: any, response: any) => {
+		const etag = "test-etag-123";
+
+		if (request.headers["if-none-match"] === etag) {
+			// 304 response with COMPLETELY EMPTY HEADERS
+			// This simulates what some servers do
+			response.statusCode = 304;
+			response.end();
+		} else {
+			// Original 200 response with gzipped content
+			const payload = JSON.stringify({ foo: "bar" });
+			const compressed = await pm(gzip)(payload);
+
+			response.setHeader("content-encoding", "gzip");
+			response.setHeader("cache-control", "public, max-age=1");
+			response.setHeader("etag", etag);
+			response.end(compressed);
+		}
+	});
+});
+
+afterAll(async () => {
+	await s.close();
+});
+
+test("304 with empty headers preserves Content-Encoding from cached response", async () => {
+	const endpoint = "/compress-bug";
+	const cache = new Map();
+	const cacheableRequest = new CacheableRequest(request, cache);
+	const cacheableRequestHelper = promisify(cacheableRequest.request());
+
+	// First request - should get 200 with gzipped content
+	// biome-ignore lint/suspicious/noExplicitAny: legacy test format
+	const response1: any = await cacheableRequestHelper(s.url + endpoint);
+	expect(response1.statusCode).toBe(200);
+	expect(response1.headers["content-encoding"]).toBe("gzip");
+	expect(response1.fromCache).toBeFalsy();
+
+	// Wait for cache to expire (max-age=1)
+	await delay(1100);
+
+	// Second request - should trigger 304 revalidation with completely empty headers
+	// biome-ignore lint/suspicious/noExplicitAny: legacy test format
+	const response2: any = await cacheableRequestHelper(s.url + endpoint);
+
+	// Should be 200 from cache after 304 revalidation
+	expect(response2.statusCode).toBe(200);
+	expect(response2.fromCache).toBe(true);
+
+	// Content-Encoding and other entity headers should be preserved from the cached response
+	// even when the 304 response has completely empty headers
+	expect(response2.headers["content-encoding"]).toBe("gzip");
+	expect(response2.headers.etag).toBe("test-etag-123");
+	expect(response2.body.length).toBeGreaterThan(0);
+});