gotify / server

A simple server for sending and receiving messages in real-time per WebSocket. (Includes a sleek web-ui)
https://gotify.net
Other
11.45k stars 635 forks source link

Path normalisation/Stripping leading slashes #533

Open ColinHebert opened 1 year ago

ColinHebert commented 1 year ago

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.

jmattheis commented 1 year ago

I'm accepting PR's for this.

ColinHebert commented 1 year ago

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 commented 1 year ago

hey can i work on this issue ?

shubmjagtap commented 1 year ago

@ColinHebert

jmattheis commented 1 year ago

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