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

HTTP Cache - Improve message for "No patterns for file revving match" #741

Closed
KeironLowe92 opened this Issue Jan 7, 2018 · 13 comments

Comments

Projects
None yet
4 participants
@KeironLowe92

KeironLowe92 commented Jan 7, 2018

Trying to set some Cache Control headers and no matter what I do it results in Sonarwhal reporting an error.

So I originally got...

Static resources should have a long cache value (31536000) and use the immutable directive:
max-age=31536000, public  http-cache

So I updated the below header, which Sonarwhal then reports...

    .htaccess
    <filesMatch "\.(ico|jpg|jpeg|png|gif|js|css|webmanifest)$">
        Header set Cache-Control "max-age=31536000, immutable"
    </filesMatch>

    Sonarwhal
    No patterns for file revving match https://xxxxx.local/xxxxx.webmanifest  http-cache

I've tried the following and they all result in the no patterns error...

    <filesMatch "\.(ico|jpg|jpeg|png|gif|js|css|webmanifest)$">
        Header set Cache-Control "max-age=31536000, public, immutable"
    </filesMatch>

    <filesMatch "\.(ico|jpg|jpeg|png|gif|js|css|webmanifest)$">
        Header set Cache-Control "max-age=31536000, public immutable"
    </filesMatch>

    <filesMatch "\.(ico|jpg|jpeg|png|gif|js|css|webmanifest)$">
        Header set Cache-Control "public, max-age=31536000, immutable"
    </filesMatch>
  • __sonarwhal version:v0.21.0
  • __Node.js version:v8.5.0
  • __npm version:5.6.0
sonarwhal’s configuration
{
    "browserslist": [],
    "connector": {
        "name": "chrome",
        "options": {
            "waitFor": 1000
        }
    },
    "formatters": [
        "stylish"
    ],
    "ignoredUrls": [
        {
            "domain": "www.google-analytics.com",
            "rules": ["*"]
        },
        {
            "domain": "code.jquery.com",
            "rules": ["*"]
        }
    ],
    "rules": {
        "amp-validator": "off",
        "apple-touch-icons": "error",
        "axe": "error",
        "content-type": "error",
        "disown-opener": "error",
        "highest-available-document-mode": "error",
        "html-checker": "error",
        "http-cache": "error",
        "image-optimization-cloudinary": "off",
        "manifest-app-name": "error",
        "manifest-exists": "error",
        "manifest-file-extension": "error",
        "manifest-is-valid": "error",
        "meta-charset-utf-8": "error",
        "meta-viewport": "error",
        "no-disallowed-headers": "off",
        "no-friendly-error-pages": "error",
        "no-html-only-headers": "error",
        "no-http-redirects": "error",
        "no-protocol-relative-urls": "error",
        "no-vulnerable-javascript-libraries": "error",
        "ssllabs": "off",
        "strict-transport-security": "error",
        "validate-set-cookie-header": "error",
        "x-content-type-options": "error"
    },
    "rulesTimeout": 120000
}
@alrra

This comment has been minimized.

Member

alrra commented Jan 7, 2018

No patterns for file revving match https://xxxxx.local/xxxxx.webmanifest http-cac

That error is not related to the HTTP headers, it's related to the filename/path of the cached files. Namely, you also need to have some kind of filename/path based revving in place in order to be able (if needed) to serve new versions of those cached files.

In your case, that URL needs to be something such as: https://xxxxx.local/xxxxx.<version|sha>.webmanifest, because if you only have https://xxxxx.local/xxxxx.webmanifest, if you release a new version of xxxxx.webmanifest, user agents that already have that file in the cache will not fetch the new version.


See:


@molant We should improve the error message and documentation regarding this.

@molant molant changed the title from HTTP Cache - No patterns for file revving match to HTTP Cache - Improve message for "No patterns for file revving match" Jan 8, 2018

@molant

This comment has been minimized.

Member

molant commented Jan 8, 2018

We should improve the error message and documentation regarding this.

I've updated the title of the issue. Suggestions are welcome

@molant molant added the enhacement label Jan 8, 2018

@KeironLowe92

This comment has been minimized.

KeironLowe92 commented Jan 8, 2018

@alrra Makes sense, in that case, I've got a few files which end in ?v=whatever but still result in the error. Shouldn't we support that as it's another way of versioning files for cache?

Edit: Apologies, just seen your second link, ignore me!

@nico3333fr

This comment has been minimized.

nico3333fr commented Jan 16, 2018

Same issue here: I've tested cache-control on this test: https://sonarwhal.com/scanner/b485abe8-7a04-4057-b63e-e71ba88cf7b1
I've used: Header set Cache-Control "public, max-age=31536000, immutable"

I've used this pattern for cache-busting for some files namefile_timestamp.extension for namefile.extension, and I get this message: No patterns for file revving match https://van11y.net/layout/images/icon_meca_1473335663.svg (for this file icon_meca.svg)

Do I miss something or is it an issue?

@alrra

This comment has been minimized.

Member

alrra commented Jan 16, 2018

Do I miss something or is it an issue?

@nico3333fr No, it's not an issue, the problem is just that sonarwhal looks by default for the most common ways of doing file revving (i.e.: icon.12345.svg or icon-12345.svg).

For your specific case (i.e. <name>_<version>.svg), you will need to configure sonarwhal by specifying a regex.

@molant What do you think about adding _<version> to the list?


Notes:

  • I wish there was a 100% correct way to detect all the patterns people are using for file revving, but there isn't, that is why we opted for the current solution.
  • Currently you can only configure sonarwhal if you are using the CLI as the online scanner does allow them yet (that is something we plan to allow in the future).
@molant

This comment has been minimized.

Member

molant commented Jan 16, 2018

What do you think about adding _<version> to the list?

Sounds good to me, shouldn't have to be too complicated.

@molant

This comment has been minimized.

Member

molant commented Jan 16, 2018

Going to update the regex to /\/(\w|-|_)+(\.|-|_)\w+\.\w+$/i

If you have any other patterns to propose now is the time. You can test it in regex101

molant added a commit to molant/hint that referenced this issue Jan 16, 2018

Fix: Add new patterns to `http-cache` revving
* Add `_` as another separator for file revving in `http-cache` rule
* Point to the docs when no pattern is found so users know it can be
  configured

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Fix webhintio#741

molant added a commit to molant/hint that referenced this issue Jan 16, 2018

Fix: Add new patterns to `http-cache` revving
* Add `_` as another separator for file revving in `http-cache` rule
* Point to the docs when no pattern is found so users know it can be
  configured

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Fix webhintio#741

@molant molant referenced this issue Jan 16, 2018

Closed

Fix: Add new patterns to `http-cache` revving #760

4 of 4 tasks complete
@molant

This comment has been minimized.

Member

molant commented Jan 16, 2018

@nico3333fr, @KeironLowe92 the PR is #760 in case you want to take a look and provide feedback.

molant added a commit to molant/hint that referenced this issue Jan 16, 2018

Fix: Add new patterns to `http-cache` revving
* Add `_` as another separator for file revving in `http-cache` rule
* Point to the docs when no pattern is found so users know it can be
  configured

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Fix webhintio#741

molant added a commit to molant/hint that referenced this issue Jan 16, 2018

Fix: Add new patterns to `http-cache` revving
* Add `_` as another separator for file revving in `http-cache` rule
* Point to the docs when no pattern is found so users know it can be
  configured

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Fix webhintio#741

@alrra alrra closed this in 87d2af7 Jan 16, 2018

@nico3333fr

This comment has been minimized.

nico3333fr commented Jan 17, 2018

@molant is it pushed to the online version? I've tested online, and I still get the message https://sonarwhal.com/scanner/7ebec8cd-5fef-4915-97b5-1586e1f5da46

@alrra

This comment has been minimized.

Member

alrra commented Jan 17, 2018

is it pushed to the online version?

@nico3333fr Not yet, I will release a new version of sonarwhal, and we'll update the online scanner.

@nico3333fr

This comment has been minimized.

nico3333fr commented Jan 17, 2018

@alrra great, looking forward to seeing it in action 👍

@alrra

This comment has been minimized.

Member

alrra commented Jan 17, 2018

looking forward to seeing it in action 👍

@nico3333fr Scanner was updated.

See: https://sonarwhal.com/scanner/ca679d32-875c-4292-8cb0-a9b81c10e44f.

@nico3333fr

This comment has been minimized.

nico3333fr commented Jan 24, 2018

@alrra this is great 👍

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