Problem/Motivation
The fast_404system allows a shortcut path, which is by default configured to asset extensions like txt, png, swf, etc.
One case which is not covered currently is the case of source maps for CSS and JS, which are requested when developers have browsers with dev tools open, and which cause extra 404s all going to the Drupal backend (hopefully cached if cache_ttl_4xxis active).
Since these are assets, present or not, these requests should not have to go to the router if fast_404 has been set up, just like the other asset types.
See the MAP file format wiki page for more information.
Steps to reproduce
Proposed resolution
The fast_404 extensions are currently txt|png|gif|jpe?g|css|js|ico|swf|flv|cgi|bat|pl|dll|exe|asp.
Add map to the list of extensions.
Remaining tasks
Create patchReview patch- Commit patch
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| Screenshot from 2022-01-28 10-13-11.png | 68 bytes | vikashsoni | |
| #14 | 2987864-14.patch | 4.56 KB | kunal_singh |
| #9 | 2987864-fast404-map-9.patch | 3.68 KB | bthompson1 |
| #2 | 2987864-fast404-map.patch | 901 bytes | fgm |
Comments
Comment #2
fgmTiny patch addressing the issue.
Comment #6
kristen polPatch applies cleanly to 9.1.x.
Comment #7
kristen polThe change looks like it would address the problem noted in the issue summary but I'm not sure if there is an easy way to test this manually.
Comment #8
kristen polMoving back to needs work because there are a few other places that need updating. Here are all the places that have the extensions defined:
Comment #9
bthompson1 commentedComment #10
kristen polComment #12
jaykandariComment #13
kunal_singh commentedComment #14
kunal_singh commentedComment #15
kunal_singh commentedComment #16
kristen polReviewing this for DrupalCamp Colorado.
Comment #17
kristen polThanks for the update. Marking RTBC since:
1) Patch still applies cleanly to 9.1.x.
2) Tests pass.
3) Migrate tests include
mapnow.4) After applying the patch, there are no places in core missing
mapaftertxt|png|gif|jpe?g|css|js|ico|swf|flv|cgi|bat|pl|dll|exe|asp5) Title and issue summary are clear.
6) Note: updated the issue summary to use the template, change some minor formatting, and add link to https://en.wikipedia.org/wiki/MAP_(file_format) for reference.
Comment #18
larowlanWe have no tests for that subscriber that I can see?
Can we get a follow-up task to add tests for the fast_404 behaviour?
The class was added in #2323759: Modularize kernel exception handling
Comment #20
naresh_bavaskarComment #21
naresh_bavaskarComment #22
kunal_singh commentedComment #25
quietone commentedThis needs work for the followup asked for in #18.
Comment #26
longwaveOut of scope for this issue but surely we don't support /imagecache/ paths any more?
Comment #27
ajv009 commented@quietone, I just created a followup issue for tests as said in #18 - https://www.drupal.org/project/drupal/issues/3216273
Comment #28
ajv009 commentedYes, @longwave I guess it was removed from D7/8, how about we remove it in this patch itself?
Or create a follow-up issue and patch it?
Comment #29
ajv009 commentedHey @longwave - https://www.drupal.org/project/drupal/issues/3216687
Comment #30
mradcliffe@kndr mentioned this issue in #3216273: Add tests for Fast404ExceptionHtmlSubscriber, and that it would be a good opportunity to add a kernel test so I'm adding the Needs tests tag.
It could also be solved in that issue or included in this one, but I'm adding a comment here to get more opinions.
Comment #31
quietone commentedChanging to NW for tests per #30 and removing Novice tag.
Comment #37
smustgrave commentedBelieve this could probably move forward now that #3292908: Fast 404s are slower than regular 404s is in D10
Comment #38
smustgrave commentedWill definitely need an issue summary update based on what was changed over there.