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

Context.Path() confusing behaviour for 404 endpoints. #2384

Closed
brietaylor opened this issue Jan 27, 2023 · 1 comment
Closed

Context.Path() confusing behaviour for 404 endpoints. #2384

brietaylor opened this issue Jan 27, 2023 · 1 comment

Comments

@brietaylor
Copy link
Contributor

brietaylor commented Jan 27, 2023

As documented: "Context.Path() returns the registered path for the handler."

I couldn't really find any documentation of what this is supposed to return if there's no registered handler (eg. a 404 in a middleware). Currently it just returns the request path, seems confusing to me, since the registered paths are trusted strings, whereas request paths can be anything.

This is causing an issue for us in the Prometheus middleware, which assumes Context.path to be a safe string from registered handlers, but it is not. labstack/echo-contrib#86

IMO it should be set to something like "" in this case, which would fix the issue in the middleware above, but also this is somewhat of a breaking change? Curious to hear others' thoughts on this.

Edit: Opened a PR: #2385

@aldas
Copy link
Contributor

aldas commented Feb 1, 2023

done with #2385

@aldas aldas closed this as completed Feb 1, 2023
danimo added a commit to b3scale/b3scale that referenced this issue Nov 14, 2023
In labstack/echo#2385, a fix for
labstack/echo#2384 was added.
The proposed fix is to use the path from the Request URL.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants