golink collapses double slashes in URL path #89
Comments
|
Ideally if one wants to put something with path reserved characters into the path, they ought to be escaped before cleaning, I think? That is, we don't really want the components of the embedded URL to behave like part of the enclosing path, but to be a single element, right? I imagine we could do that escaping proactively at creation time without breaking anything, right? |
|
Maybe we expose a template func that could enforce this behavior when desired? |
|
oh, it's not happening later... it's happening earlier. We're using |
|
Well for this example I'm assuming we'd encode the string the user entered as the label, so |
|
in this case, the URL that's getting munged is not part of the defined go link... it's the extra path that is being added by the user at resolution time. I think |
Hm. I guess that means the browser isn't encoding it, which I suppose isn't entirely crazy since it could have been a path but for the scheme label. 🤔 |
|
I mean, it is just a path, and there's nothing for the browser to encode. |
|
The new go1.22 changes for ServeMux have certainly made the handling logic more complicated (in order to support matching on method), but it still always cleans URLs. We either have to use a different mux implementation, or none at all (which wouldn't be awful, I guess... we don't have that many endpoints). |
Both http.ServeMux as well as the http.Redirect method pass the request URL through `cleanPath` which, among other things, collapses double slashes `//` to a single slash `/`. Most of the time this is fine, since most servers treat those as identical anyway. But some destination servers need the original path unmodified. Since we're just redirecting, we don't need to be concerned with the additional benefits of `cleanPath` such as eliminating `../` path components, since that is the responsibility of the destination server to clean if needed. This change adds a separate root http.Handler for golink requests. It still uses http.ServeMux for internal endpoints, but serves golinks directly without passing the request through ServeMux. Additionally, this sets the redirect status and Location header directly rather than calling http.Redirect, since that also modifies the URL it is given. Fixes #89
Both http.ServeMux as well as the http.Redirect method pass the request URL through `cleanPath` which, among other things, collapses double slashes `//` to a single slash `/`. Most of the time this is fine, since most servers treat those as identical anyway. But some destination servers need the original path unmodified. Since we're just redirecting, we don't need to be concerned with the additional benefits of `cleanPath` such as eliminating `../` path components, since that is the responsibility of the destination server to clean if needed. This change adds a separate root http.Handler for golink requests. It still uses http.ServeMux for internal endpoints, but serves golinks directly without passing the request through ServeMux. Additionally, this sets the redirect status and Location header directly rather than calling http.Redirect, since that also modifies the URL it is given. Fixes #89 Signed-off-by: Will Norris <[email protected]>
Both http.ServeMux as well as the http.Redirect method pass the request URL through `cleanPath` which, among other things, collapses double slashes `//` to a single slash `/`. Most of the time this is fine, since most servers treat those as identical anyway. But some destination servers need the original path unmodified. Since we're just redirecting, we don't need to be concerned with the additional benefits of `cleanPath` such as eliminating `../` path components, since that is the responsibility of the destination server to clean if needed. This change adds a separate root http.Handler for golink requests. It still uses http.ServeMux for internal endpoints, but serves golinks directly without passing the request through ServeMux. Additionally, this sets the redirect status and Location header directly rather than calling http.Redirect, since that also modifies the URL it is given. Fixes #89 Signed-off-by: Will Norris <[email protected]>
Start with the go link
go/ladder => http://ladder/. When appending a full URL to the extra path, the//gets collapsed down to a single/:This isn't technically wrong, but for a project like ladder this is undesirable.
For imageproxy, I dealt with a similar issue by switching to gorilla's Mux, and disabling path cleaning. I'd rather not do that here if we can keep from it. But if I remember correctly, the default
http.ServeMuxalways cleans paths and doesn't provide the ability to disable that. There's work happening onhttp.ServeMuxin go1.22, so I can look and see if any of that would be helpful here.The text was updated successfully, but these errors were encountered: