Match flexible static file version in nginx sample config #12521
Conversation
scottsb
force-pushed
the
SB/flexible-static-file-version-nginx
branch
from
eff3b09 to
32d30a5
Compare
|
FYI, I had a typo in the original commit. I just force-pushed an update to my branch that corrects it. |
| @@ -100,7 +100,7 @@ location /static/ { | |||
|
|
|||
| # Remove signature of the static files that is used to overcome the browser cache | |||
| location ~ ^/static/version { | |||
| rewrite ^/static/(version\d*/)?(.*)$ /static/$2 last; | |||
| rewrite ^/static/(version[^/]+/)?(.*)$ /static/$2 last; | |||
There was a problem hiding this comment.
I think that this regex change will have the desired effect but what if we updated it to keep in the same style as before? Something like rewrite ^/static/(version[\w\d]*/)?(.*)$ /static/$2 last; this would then match any word or digit character. Or was there another reason for your choice @scottsb
There was a problem hiding this comment.
Because of the fact that the version can be specified as a command line argument and no characters are filtered out, I don't think \w\d is sufficient. For example, that would not match a hyphen, which is allowed by the --content-version argument.
There was a problem hiding this comment.
Do you know if there is validation on the --content-version call? I would worry that anything but / would be a bit too wide and could cause issues.
There was a problem hiding this comment.
There is no validation. Whatever is passed in is used:
I tested, and it will even accept characters that really shouldn't be allowed like / and even ?. Validation should be added to the deploy command to block those, but here in nginx, I can't see any issue with the broad match.
I'll open a separate issue for the need for validation on the command.
There was a problem hiding this comment.
Thanks for the feedback @scottsb good to know that you will open an issue about validation on the command side. I would also suggest that in this new issue it might be a good idea to also update the regex to fit the requirements of the new validation.
|
Hi @scottsb, |
Description
Magento 2.2 allows you to specify a custom version for static files being deployed. That means it may not necessarily be numeric like it is when using the default timestamp version numbers. This generalizes the nginx sample config to match custom versions as well.