-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
StaticResource only matches folder-like URL prefix #5534
Conversation
The fact that I wonder if the |
Codecov Report
@@ Coverage Diff @@
## master #5534 +/- ##
=======================================
Coverage 93.31% 93.31%
=======================================
Files 102 102
Lines 30313 30325 +12
Branches 2723 2724 +1
=======================================
+ Hits 28287 28299 +12
Misses 1849 1849
Partials 177 177
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
OK, I’m fairly certain this is ready for review. Again, I don’t think the docs need to change because I think of this as a bug fix, but I’m not sure. |
@webknjaz Thoughts on this? Seems to resolve some surprising behaviour. Maybe a more efficient solution would be to ensure that that static resource is always at the end of the list, so it's the last one checked? This sounds reasonable to me, as a web server should be handling static assets on a production system where performance matters, therefore the static resource would never be useful. |
@Dreamsorcerer I'm not sure about that. Making the priority assumptions should probably be on the user. Besides, this would be a breaking change for people who used to rely on the order. |
Backport to 3.8: 💚 backport PR created✅ Backport PR branch: Backported as #6179 🤖 @patchback |
Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com> (cherry picked from commit 9c7f3d3)
What do these changes do?
I think this fixes #5250.
A static resource like:
currently matches
/foobar
but returns 404. This PR ensures that aforementioned static route does not match/foobar
Are there changes in behavior for the user?
If a user expected
/foobar
to load the file/www/foo/ar
, this PR will violate their expectation.Related issue number
Fixes #5250
Checklist
I'm not aware of any docs that need to change, but I didn't tick that box because I am not certain.
CONTRIBUTORS.txt
CHANGES
folder<issue_id>.<type>
for example (588.bugfix)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.