Problem/Motivation
Rewrite rules for gzipped CSS and JavaScript aggregates in .htaccess never match, because regex lacks a quantifier for the character class.
Steps to reproduce
RewriteRule ^(.*css_[a-zA-Z0-9-_])\.css$ $1\.css\.gz [QSA]
RewriteRule ^(.*js_[a-zA-Z0-9-_])\.js$ $1\.js\.gz [QSA]Proposed resolution
RewriteRule ^(.*css_[a-zA-Z0-9-_]+)\.css$ $1\.css\.gz [QSA]
RewriteRule ^(.*js_[a-zA-Z0-9-_]+)\.js$ $1\.js\.gz [QSA]Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
The htaccess rewrite rule that enables static gzipped JavaScript and CSS aggregates to be served to browsers has been modified to improve performance. Sites should update their .htaccess files to take advantage of this performance improvement.
Issue fork drupal-3336463
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3336463-rewrite-rules-for
changes, plain diff MR !3290
- 3336463-
changes, plain diff MR !3431
- 10.1.x
changes, plain diff MR !3291
Comments
Comment #2
sleitner commentedComment #3
sleitner commentedComment #4
sleitner commentedComment #5
cilefen commentedThere is no code to review at this time.
Comment #6
sleitner commentedComment #9
sleitner commentedComment #10
sleitner commentedComment #13
sleitner commentedComment #14
recrit commented@sleitner thanks for the patch. The patch fixed it for my build. It now correctly serves JS and CSS with "Content-Encoding: gzip".
My only thought is the quantifier might need to be a "+" instead of "*" since
"js_.js"is not a valid filename.The erroneous changes in #1040534: Rewrite rules for gzipped CSS and JavaScript aggregates cause lots of lstats for files that will never exist update the RewriteRule to the following:
RewriteRule ^(.*js_[a-zA-Z0-9-_])\.js$ $1\.js\.gz [QSA]Example Drupal JS filename:
"js_w0tbu2Mi0ldw2g1-6ceMRIw636XPaZoJEvQkqKjxQkE.js"Bug: The current rule only matches filenames
"js_w.js"- which does not match the Drupal JS filename.Expected: This rule should match
"js_w0tbu2Mi0ldw2g1-6ceMRIw636XPaZoJEvQkqKjxQkE.js"Comment #15
sleitner commented@recrit I changed "*" to "+", which should be better. Please review
Comment #16
recrit commented@sleitner thanks, looks good and worked as expected correctly serving the gzip files
Comment #17
catchI think I introduced the original bug here, fix looks good to me and preserves the behaviour the original issue was trying to fix (not looking for files that will never exist).
Comment #18
recrit commented@catch Note that this is also a bug for 9.5.x. The 10.x patch applies cleanly to both branches but I also updated the 9.5.x MR with the latest commits.
MR Summary:
- 10.1.x: https://git.drupalcode.org/project/drupal/-/merge_requests/3431
- 9.5.x: https://git.drupalcode.org/project/drupal/-/merge_requests/3290
Comment #19
longwaveCommitted and pushed 1d3e0326a5 to 10.1.x and 7f5e51d92b to 10.0.x and 03de0c83b9 to 9.5.x. Thanks!
Will open a followup to add test coverage to this, as we test other parts of .htaccess.
Comment #23
alexpottI've added the same release note as #1040534: Rewrite rules for gzipped CSS and JavaScript aggregates cause lots of lstats for files that will never exist - I've also updated the CR. I'm note really sure what we should do for people who updated due to the original issue or started a site in the meantime. Maybe we need to update the original CR for this.
Comment #24
catchI've made a new CR with the same wording - if people follow the list or anything, it ensures they'll see it again. Agreed with fixing the old one too though.
https://www.drupal.org/node/3351478
Comment #25
luenemann