Skip to content
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

Path normalisation/Stripping leading slashes #533

Open
ColinHebert opened this issue Dec 26, 2022 · 5 comments
Open

Path normalisation/Stripping leading slashes #533

ColinHebert opened this issue Dec 26, 2022 · 5 comments
Labels
a:feature New feature or request good first issue Good for newcomers

Comments

@ColinHebert
Copy link

ColinHebert commented Dec 26, 2022

Is your feature request related to a problem? Please describe.
When working with a relatively naive setup of gotify, one may have a trailing slash when specifying the url of the host (http://localhost/ instead of http://localhost for example).
To avoid any issues/ambiguity, the developer using this URL should append /message to the configured url to send a message regardless of the url specified by the customer to ensure that a slash is in there.

This can lead to the fully formed looking like this http://localhost//mesage, unfortunately with the way gotify is currently built, the double slash is not normalised and a query at this URL will lead to a 404

Describe the solution you'd like
Gotify should normalise the URL it receives to deduplicate slashes when possible, those are not meant to have any bearing on the URL.

Describe alternatives you've considered
The alternative is to leave with misconfigurations and hope they are being detected early on by the API user.

@ColinHebert ColinHebert added the a:feature New feature or request label Dec 26, 2022
@jmattheis jmattheis added the good first issue Good for newcomers label Dec 26, 2022
@jmattheis
Copy link
Member

I'm accepting PR's for this.

@ColinHebert
Copy link
Author

Leaving that for whomever wants to take it.
FWIW, gin, the HTTP server, has a nice parameter RemoveExtraSlash which should handle that neatly.
It should be a case of editing this https://github.com/gotify/server/blob/master/router/router.go#L25 to add g.RemoveExtraSlash = true.

Caveats, testing is a bit of a chore, while at it, it's worth checking why a new gin engine is created from scratch rather than gin.Default() which has a hard crash recovery mechanism and logging enabled (niceties provided out of the box).

@shubmjagtap
Copy link

hey can i work on this issue ?

@shubmjagtap
Copy link

@ColinHebert

@jmattheis
Copy link
Member

@shubmjagtap Yes, but make sure to test this with different reverse proxy configurations, that it'll work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:feature New feature or request good first issue Good for newcomers
Development

No branches or pull requests

3 participants