stufan commented on issue #1322: ModifyCachingHeaders changes headers for all 
resources
URL: 
https://github.com/apache/incubator-pagespeed-ngx/issues/1322#issuecomment-398064610
 
 
   I dig a bit into the code and I came to some findings but I am not quite 
sure how good are they to be applied. Basically if I comment  
   ```C
       if (preserve_caching_headers == kPreserveAllCachingHeaders) {
         if (StringCaseEqual(name_gs, "ETag") ||
             StringCaseEqual(name_gs, "Expires") ||
             StringCaseEqual(name_gs, "Date") ||
             StringCaseEqual(name_gs, "Last-Modified") ||
             StringCaseEqual(name_gs, "Cache-Control")) {
           continue;
         }
       } else if (preserve_caching_headers == kPreserveOnlyCacheControl) {
         // Retain the original Cache-Control header, but send the recomputed
         // values for all other cache-related headers.
         if (StringCaseEqual(name_gs, "Cache-Control")) {
           continue;
         }
       }  // else we don't preserve any headers.
   ```
   in `copy_response_headers_to_ngx` , then caching headers appear for 
pagespeed resources appear together with the HTML original cache control 
headers. It looks like when `preserve_caching_headers == 
kPreserveAllCachingHeaders` or `preserve_caching_headers == 
kPreserveOnlyCacheControl` the caching headers generated by Pagespeed (for 
Pagespeed resources) are skipped / not added to the response. 
   
   Also it seems when ModifyCachingHeaders = off it is set for all resources 
and not HTML only according to this code in `ps_resource_handler`:
   ```
       if (!options->modify_caching_headers()) {
         ctx->preserve_caching_headers = kPreserveAllCachingHeaders;
       } else if (!options->IsDownstreamCacheIntegrationEnabled()) {
         // Downstream cache integration is not enabled. Disable original
         // Cache-Control headers.
         ctx->preserve_caching_headers = kDontPreserveHeaders;
       } else if (!pagespeed_resource && !is_an_admin_handler) {
         ctx->preserve_caching_headers = kPreserveOnlyCacheControl;
         // Downstream cache integration is enabled. If a rebeaconing key has 
been
         // configured and there is a ShouldBeacon header with the correct key,
         // disable original Cache-Control headers so that the instrumented 
page is
         // served out with no-cache.
         StringPiece should_beacon(request_headers->Lookup1(kPsaShouldBeacon));
         if (options->MatchesDownstreamCacheRebeaconingKey(should_beacon)) {
           ctx->preserve_caching_headers = kDontPreserveHeaders;
         }
       }
   ```
   which doesn't look quite right. Is there anyway to understand that the 
request is only for an HTML resource and not something else?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to