http-tests/cache-tests

headers-omit-headers-listed-in-Cache-Control-no-cache* sets incorrect Cache-Control response header

rcanavan opened this issue · 4 comments

In my setup, headers-omit-headers-listed-in-Cache-Control-no-cache appears to be non-functional, since the specific Cache-Control headers for the test are effectively replaced in checkCached(). This is evident in the response logged by the server:

=== Server response 1
    HTTP 200 OK
    cache-control: max-age=3600

With the patch below, I get the intended Cache-Control response header:

=== Server response 1
    HTTP 200 OK
    cache-control: no-cache="a"
diff --git a/tests/headers.mjs b/tests/headers.mjs
index a7beec8..307636e 100644
--- a/tests/headers.mjs
+++ b/tests/headers.mjs
@@ -9,10 +9,10 @@ function checkCached ({name, id, kind = 'required', configuredHeaders, expectedH
     kind,
     requests: [
       {
-        response_headers: configuredHeaders.concat([
+        response_headers: [
           ['Cache-Control', `max-age=3600`],
           ['Date', 0]
-        ]),
+        ].concat(configuredHeaders),
         setup: true,
         pause_after: true
       },
mnot commented

Thanks for the issue. The root cause here appears to be that the test was using multiple instances of the same header name, but the semantics of nodejs setHeader() overwrite any existing value. 21ace25 should take care of this; can you try with that?

I still get cache-control: max-age=3600 with 21ace25.

mnot commented

Yes, that's working as designed; the headers are meant to be added together. Cache-Control: no-cache effectively overrides Cache-Control: max-age.

Looks like your patch is working correctly after all, it appears that I did not use the correct version in my last test.

thanks for the prompt fix.