mux
mux copied to clipboard
gorilla
fix(mux.go): use cleaned path as URL.Path
Fixes #673
Summary of Changes
- removed the redirection on path mismatch after cleanup
- used the cleaned up path for finding Path match
PS: Make sure your PR includes/updates tests! If you need help with this part, just ask! Will Update tests if the approach looks good
The go http servers follows the same convention to redirect when the request path isn't clean. See here. I am unsure why ? It could be to protect against path traversal? I am might be worth investigating further before diverging from this behaviour.
@chaudyg that could be true for the HTML website server.
But for REST Api based servers, this is not very intuitive as per my understanding
@elithrar could use your advice here
Needs positive & negative tests - for the mentioned issue and the delated code.
@chaudyg that could be true for the HTML website server.
But for REST Api based servers, this is not very intuitive as per my understanding
There's no difference here: these are web servers. We can't break existing cases.
I'm not discouraging anyone here. Changes proposed in the PR are not required and we can mark it as NA. Let me know what do you think?
Reason is because as @chaudyg mentioned already in comment net/http/server.go file we have implemented Redirect and same we're doing it in our mux. Removing Redirect from mux will not align with the original implementation.
Makes sense
On Tue, 31 May, 2022, 3:31 pm Mustaque Ahmed, @.***> wrote:
I'm not discouraging anyone here. Changes proposed in the PR are not required and we can mark it as NA. Let me know what do you think?
Reason is because as @chaudyg https://github.com/chaudyg mentioned already in comment https://github.com/gorilla/mux/pull/674#issuecomment-1129712157 net/http/server.go file we have implemented Redirect and same we're doing it in our mux. Removing Redirect from mux will not align with the original implementation.
— Reply to this email directly, view it on GitHub https://github.com/gorilla/mux/pull/674#issuecomment-1141930550, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGAVQE4MQOWMLQDOVHV3FJDVMXPRLANCNFSM5U5DKZRA . You are receiving this because you authored the thread.Message ID: @.***>
Makes sense … On Tue, 31 May, 2022, 3:31 pm Mustaque Ahmed, @.> wrote: I'm not discouraging anyone here. Changes proposed in the PR are not required and we can mark it as NA. Let me know what do you think? Reason is because as @chaudyg https://github.com/chaudyg mentioned already in comment <#674 (comment)> net/http/server.go file we have implemented Redirect and same we're doing it in our mux. Removing Redirect from mux will not align with the original implementation. — Reply to this email directly, view it on GitHub <#674 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGAVQE4MQOWMLQDOVHV3FJDVMXPRLANCNFSM5U5DKZRA . You are receiving this because you authored the thread.Message ID: @.>
@elithrar we can close the PR and its related issue.
