Repository navigation
Match whole segments for statics in verified routes - #6896
mandar1045 wants to merge 4 commits into
Conversation
rhcarvalho
left a comment
rhcarvalho
left a comment
There was a problem hiding this comment.
I agree the current behavior is surprising when the matching is not segment-aware.
Not sure about breaking current behavior for any apps that might have a multi-segment path in :statics.
| # Like Plug.Static's :only, the whole first segment must match | ||
| defp static_path?(path, statics) do | ||
| Enum.find(statics, &String.starts_with?(path, "/" <> &1)) | ||
| [_, segment | _] = String.split(path, ["/", "#"], parts: 3) |
There was a problem hiding this comment.
Currently https://phoenix.hexdocs.pm/Phoenix.VerifiedRoutes.html#module-options doesn't saying anything about depth, only:
:statics- Optional list of static directories to treat as verified paths
Before, statics: ~w(assets/images) would work (accidentally or not), but with this change it doesn't.
| defp static_path?(path, statics) do | ||
| Enum.find(statics, &String.starts_with?(path, "/" <> &1)) | ||
| [_, segment | _] = String.split(path, ["/", "#"], parts: 3) | ||
| segment in statics | ||
| end |
There was a problem hiding this comment.
We might start from the assumption that a valid path starts with /, and thus split it in at most 2 parts instead of 3 (ignoring the first part):
| defp static_path?(path, statics) do | |
| Enum.find(statics, &String.starts_with?(path, "/" <> &1)) | |
| [_, segment | _] = String.split(path, ["/", "#"], parts: 3) | |
| segment in statics | |
| end | |
| defp static_path?("/" <> rest, statics) do | |
| [segment | _] = String.split(rest, ["/", "#"], parts: 2) | |
| segment in statics | |
| end |
|
@rhcarvalho got it , I missed that. I could match whole segments instead, so |
|
It now matches whole segments, so |
Closes #6895.
static_path?/2usedString.starts_with?, so with the default statics a routelike
~p"/images-gallery"was treated as a static file. It wasn't verified,url/1pointed it to the static host, and path prefixes were skipped.
Now the whole first segment has to match, same as
Plug.Static's:only. Pathslike
/images/logo.pngand/favicon.icoare still static.Added tests for the prefix/url case and for the missing warning.