-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Trailing slash leads to 404 (regression?) #18060
Comments
What is an example of a link that was valid and is now not valid. |
https://codeberg.org/Codeberg/gitea/ for example. |
we should add a testcase to https://github.com/go-gitea/gitea/blob/main/integrations/links_test.go#L20 too ... |
#17997 was marked as breaking, but as far as I understand this issue here is a side-effect and the breaking part was only relevant to the handling of % signs? If this behaviour is desired, it was just forgotten to mention this as breaking in the 1.15.8 changelog? Anyway, I actually encountered this bug myself earlier today, too, by visiting some proposal out of my browser history. It must have gotten there somehow, so I'm wondering if Gitea internally links to the URL with the trailing slash somewhere, too? |
no it wasn't intentional. Probably simplest if the 404 handler just redirects back to "/foo" from "/foo/" I guess. |
However, this is interesting because the NotFound is coming from src/net/http/server.go:NotFound() Which is surprising as it should be using the NotFound handler in the mux so things are slightly more interesting ... |
PR go-gitea#17997 means that urls with terminal '/' are no longer immediately mapped to the url without a terminal slash. However, it has revealed that the NotFound handler appears to have been lost. This PR adds back in a NotFound handler that simply redirects to a path without the terminal slash or runs the NotFound handler. Fix go-gitea#18060 Signed-off-by: Andrew Thornton <art27@cantab.net>
It looks like the NotFound handler has gone missing too. |
PR #17997 means that urls with terminal '/' are no longer immediately mapped to the url without a terminal slash. However, it has revealed that the NotFound handler appears to have been lost. This PR adds back in a NotFound handler that simply redirects to a path without the terminal slash or runs the NotFound handler. Fix #18060 Signed-off-by: Andrew Thornton <art27@cantab.net>
Backport go-gitea#18062 PR go-gitea#17997 means that urls with terminal '/' are no longer immediately mapped to the url without a terminal slash. However, it has revealed that the NotFound handler appears to have been lost. This PR adds back in a NotFound handler that simply redirects to a path without the terminal slash or runs the NotFound handler. Fix go-gitea#18060 Signed-off-by: Andrew Thornton <art27@cantab.net>
As an intermediate fix for nginx users, here's what we did in the
In case of a 404 error, it just rewrites the URL to remove the slash internally. Could have some sick side-effects and never-ending redirects, but so far it works. Our nginx config: https://git.fsfe.org/fsfe-system-hackers/git-server/src/branch/master/templates/nginx-site.conf.j2 |
Backport #18062 PR #17997 means that urls with terminal '/' are no longer immediately mapped to the url without a terminal slash. However, it has revealed that the NotFound handler appears to have been lost. This PR adds back in a NotFound handler that simply redirects to a path without the terminal slash or runs the NotFound handler. Fix #18060 Signed-off-by: Andrew Thornton <art27@cantab.net>
A consequence of forcibly setting the RoutePath to the escaped url is that the auto routing to endpoints without terminal slashes fails (Causing go-gitea#18060.) This failure raises the possibility that forcibly setting the RoutePath causes other unexpected behaviours too. Therefore, instead we should simply pre-escape the URL in the process registering handler. Then the request URL will be properly escaped for all the following calls. Fix go-gitea#17938 Fix go-gitea#18060 Replace go-gitea#18062 Replace go-gitea#17997 Signed-off-by: Andrew Thornton <art27@cantab.net>
A consequence of forcibly setting the RoutePath to the escaped url is that the auto routing to endpoints without terminal slashes fails (Causing #18060.) This failure raises the possibility that forcibly setting the RoutePath causes other unexpected behaviors too. Therefore, instead we should simply pre-escape the URL in the process registering handler. Then the request URL will be properly escaped for all the following calls. Fix #17938 Fix #18060 Replace #18062 Replace #17997 Signed-off-by: Andrew Thornton <art27@cantab.net>
…ea#18086) Backport go-gitea#18086 A consequence of forcibly setting the RoutePath to the escaped url is that the auto routing to endpoints without terminal slashes fails (Causing go-gitea#18060.) This failure raises the possibility that forcibly setting the RoutePath causes other unexpected behaviors too. Therefore, instead we should simply pre-escape the URL in the process registering handler. Then the request URL will be properly escaped for all the following calls. Fix go-gitea#17938 Fix go-gitea#18060 Replace go-gitea#18062 Replace go-gitea#17997 Signed-off-by: Andrew Thornton <art27@cantab.net>
#18098) Backport #18086 A consequence of forcibly setting the RoutePath to the escaped url is that the auto routing to endpoints without terminal slashes fails (Causing #18060.) This failure raises the possibility that forcibly setting the RoutePath causes other unexpected behaviors too. Therefore, instead we should simply pre-escape the URL in the process registering handler. Then the request URL will be properly escaped for all the following calls. Fix #17938 Fix #18060 Replace #18062 Replace #17997 Signed-off-by: Andrew Thornton <art27@cantab.net>
PR go-gitea#17997 means that urls with terminal '/' are no longer immediately mapped to the url without a terminal slash. However, it has revealed that the NotFound handler appears to have been lost. This PR adds back in a NotFound handler that simply redirects to a path without the terminal slash or runs the NotFound handler. Fix go-gitea#18060 Signed-off-by: Andrew Thornton <art27@cantab.net>
…ea#18086) A consequence of forcibly setting the RoutePath to the escaped url is that the auto routing to endpoints without terminal slashes fails (Causing go-gitea#18060.) This failure raises the possibility that forcibly setting the RoutePath causes other unexpected behaviors too. Therefore, instead we should simply pre-escape the URL in the process registering handler. Then the request URL will be properly escaped for all the following calls. Fix go-gitea#17938 Fix go-gitea#18060 Replace go-gitea#18062 Replace go-gitea#17997 Signed-off-by: Andrew Thornton <art27@cantab.net>
Gitea Version
3a77465
Git Version
2.30.2 (not relevant?)
Operating System
Debian GNU/Linux 11
How are you running Gitea?
local development via make
Database
SQLite
Can you reproduce the bug on the Gitea demo site?
Yes
Log Gist
No response
Description
Release 1.15.8 might contain a regression. At Codeberg.org we were made aware by users that a trailing slash in the URL leads to a simple "404 page not found" now, breaking some links on the site. If this route is no longer to be used, a forward would be a good option instead.
I made a bisect:
So 3a77465 (#17997) (#18001) are relevant.
Screenshots
No response
The text was updated successfully, but these errors were encountered: