New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempting to extend cache for SVGs returns bad headers/content. #1496

Closed
benmurden opened this Issue Feb 20, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@benmurden
Copy link

benmurden commented Feb 20, 2017

In relation to #1221, I decided to try and extend the cache on SVGs and fonts that I had hashed outside of Pagespeed. With pagespeed off, everything works great, with the following headers returned.

HTTP/1.1 200 OK
Connection: keep-alive
Server: nginx
Date: Mon, 20 Feb 2017 05:34:29 GMT
Content-Type: image/svg+xml
Last-Modified: Mon, 20 Feb 2017 03:05:29 GMT
Transfer-Encoding: chunked
Etag: W/"58aa5cf9-b24"
Expires: Tue, 20 Feb 2018 05:34:29 GMT
Cache-Control: max-age=31536000
Content-Encoding: gzip
Via: 1.1 vegur

(Except that Etag looks odd)

However, with pagespeed on, I get the following, plus Chrome gives net::ERR_CONTENT_DECODING_FAILED.

HTTP/1.1 200 OK
Connection: keep-alive
Content-Type: image/svg+xml
Server: nginx
Last-Modified: Mon, 20 Feb 2017 06:25:12 GMT
Etag: "58aa8bc8-b24"
Accept-Ranges: bytes
Date: Mon, 20 Feb 2017 06:31:33 GMT
Expires: Tue, 20 Feb 2018 06:31:33 GMT
Vary: Accept-Encoding
X-Original-Content-Length: 2852
Content-Encoding: gzip
Content-Length: 1032
Etag: "58aa8bc8-b24"
Cache-Control: max-age=31536000, s-maxage=10
Accept-Ranges: bytes
Via: 1.1 vegur

Note that Etag and Accept-Ranges are both repeated.

The error appears to be because the content is no longer gzipped, though the header says otherwise. I tried to achieve this with the following config.

Config extract

http {
    gzip on;
    gzip_comp_level 2;
    gzip_min_length 512;
    gzip_proxied any;
    gzip_types
        text/css
        text/javascript
        text/xml
        text/plain
        application/javascript
        application/x-javascript
        application/json
        image/svg+xml;

    gzip_http_version 1.0;

    server_tokens off;

    include mime.types;
    default_type application/octet-stream;
    sendfile on;

    server {
        ...

        set $staticdir /app/staticfiles/;

        location /static {
            autoindex on;
            alias $staticdir;
        }

        # Adding hashes for these assets, but pagespeed doesn't set cache headers
        location ~* "/static/(.*\.[0-9a-f]{12}\.(?:svg|svgz|woff))$" {
            expires 1y;
            access_log off;
            alias $staticdir/$1;
        }

        pagespeed on;
        
        pagespeed EnableFilters prioritize_critical_css;
        pagespeed UrlValuedAttribute div img-src image;

        # Needs to exist and be writable by nginx.  Use tmpfs for best performance.
        pagespeed FileCachePath /app/var/ngx_pagespeed_cache;

        # Ensure requests for pagespeed optimized resources go to the pagespeed handler
        # and no extraneous headers get set.
        location ~ "\.pagespeed\.([a-z]\.)?[a-z]{2}\.[^.]{10}\.[^.]+" {
        add_header "" "";
        }
        location ~ "^/pagespeed_static/" { }
        location ~ "^/ngx_pagespeed_beacon$" { }
    }
}

Interestingly, the woff fonts are fine. Turning off Gzipping for image/svg+xml returns the same broken headers.

Hopefully I have just made a silly config error. Any help much appreciated.

@oschaaf

This comment has been minimized.

Copy link
Member

oschaaf commented Feb 20, 2017

Reproduced on master.
Added some lines to log the response headers in various stages during the IPRO lookup:

2017/02/16 06:17:01 [info] 65209#0: [ngx_pagespeed 1.13.0.0-8400] Trying to serve rewritten resource in-place: http://192.168.1.226:10050/static/test.svg
2017/02/16 06:17:01 [debug] 65209#0: *5 http finalize request: -4, "/static/test.svg?" a:1, c:2
2017/02/16 06:17:01 [debug] 65209#0: *5 http request count:2 blk:0
2017/02/16 06:17:01 [debug] 65209#0: timer delta: 63
2017/02/16 06:17:01 [debug] 65209#0: worker cycle
2017/02/16 06:17:01 [debug] 65209#0: epoll timer: 47077
2017/02/16 06:17:01 [debug] 65209#0: [ngx_pagespeed 1.13.0.0-8400] [0216/061701:VERBOSE1:cache_url_async_fetcher.cc(295)] Found in cache: http://192.168.1.226:10050/static/test.svg (192.168.1.226)HTTP/1.1 200 OK
Server: nginx
Content-Type: image/svg+xml
Last-Modified: Thu, 16 Feb 2017 03:14:57 GMT
ETag: "58a51931-dc7"
Cache-Control: max-age=31536000
Accept-Ranges: bytes
Date: Thu, 16 Feb 2017 05:17:01 GMT
Expires: Fri, 16 Feb 2018 05:17:01 GMT
Vary: Accept-Encoding
X-Original-Content-Length: 3527
Content-Encoding: gzip
Content-Length: 1670


2017/02/16 06:17:01 [debug] 65209#0: pagespeed [0000000001E36118] ps_base_fetch_handler()HandleHeaders: HTTP/1.1 501 OK
Server: nginx
Content-Type: image/svg+xml
Last-Modified: Thu, 16 Feb 2017 03:14:57 GMT
ETag: "58a51931-dc7"
Cache-Control: max-age=31536000
Accept-Ranges: bytes
Date: Thu, 16 Feb 2017 05:17:01 GMT
Expires: Fri, 16 Feb 2018 05:17:01 GMT
Vary: Accept-Encoding
X-Original-Content-Length: 3527
Content-Encoding: gzip
Content-Length: 1670


2017/02/16 06:17:01 [debug] 65209#0: epoll: fd:8 ev:0001 d:0000000001D24230
2017/02/16 06:17:01 [debug] 65209#0: pagespeed [0000000001E2F180] event: H. bf:0000000001E36118 (ipro lookup) - refcnt:1 - det: N
2017/02/16 06:17:01 [debug] 65209#0: *5 ps fetch handler: /static/test.svg
2017/02/16 06:17:01 [debug] 65209#0: *5 ps in place check header filter initial: /static/test.svg
2017/02/16 06:17:01 [info] 65209#0: [ngx_pagespeed 1.13.0.0-8400] Could not rewrite resource in-place because URL is not in cache: http://192.168.1.226:10050/static/test.svg
2017/02/16 06:17:01 [debug] 65209#0: *5 content phase: 10
2017/02/16 06:17:01 [debug] 65209#0: *5 content phase: 11
2017/02/16 06:17:01 [debug] 65209#0: *5 content phase: 12
2017/02/16 06:17:01 [debug] 65209#0: *5 http filename: "/home/oschaaf/code/google/mps-compression-trouble/nginx/html/static/test.svg"
2017/02/16 06:17:01 [debug] 65209#0: *5 add cleanup: 0000000001E306B8
2017/02/16 06:17:01 [debug] 65209#0: *5 http static fd: 15
2017/02/16 06:17:01 [debug] 65209#0: *5 http set discard body
2017/02/16 06:17:01 [debug] 65209#0: *5 http pagespeed html rewrite header filter "/static/test.svg"
2017/02/16 06:17:01 [debug] 65209#0: *5 ps in place check header filter recording: /static/test.svg
2017/02/16 06:17:01 [debug] 65209#0: *5 HTTP/1.1 200 OK

So CacheUrlAsyncFetcher finds the svg cache, with a 200/OK status. ngx_pagespeed receives a 501 status (not in cache) -- so the status gets updated between those two steps. But all the headers associated to the 200/OK remain set -- which I suspect is causing this trouble.

Maybe the fix is as simple as clearing the response headers before updating the status to 501.

oschaaf added a commit that referenced this issue Feb 20, 2017

Clean the response headers when sending kNotInCache
When we send out a 501 in the IPRO path for a http cache
hit, clear the response headers so we do not leave
behind stray headers.

Fixes #1496

oschaaf added a commit that referenced this issue Feb 21, 2017

Clean the response headers when sending kNotInCache (#1497)
When we send out a 501 in the IPRO path for a http cache
hit, clear the response headers so we do not leave
behind stray headers.

Fixes #1496
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment