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

Prioritize RawPath to Avoid Encoding Issues in URLs #104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d6o
Copy link

@d6o d6o commented Oct 18, 2023

Description:

This pull request addresses an encoding issue when processing URLs. We have modified the URL retrieval mechanism to first check the RawPath of the request, and if it's empty, to then check the Path. This change prioritizes the original, escaped form of the URL (RawPath) over the decoded form (Path) to avoid potential encoding issues and ambiguities.

Motivation:

Previously, requests to unregistered routes like GET /v1/%a1 would generate a panic with the message: label value \"/v1/\\xa1\" is not valid UTF-8. This is due to the fact that when decoding %a1, it results in a byte sequence that's not valid UTF-8. By checking RawPath first, we are ensuring that the original encoding of the URL is preserved, thereby avoiding such issues.

Changes:

  • Updated the URL retrieval logic to prioritize RawPath over Path.
  • Added relevant comments with references to the Go documentation for clarity.

Testing:

Ensure that the application no longer panics when sending requests to unregistered routes containing non-UTF-8 encoded characters, like GET /v1/%a1.

Note:

The implemented change references Go's URL handling mechanism as described in this documentation.

@d6o d6o changed the title Check RawPath before checking Path on Prometheus middleware Prioritize RawPath to Avoid Encoding Issues in URLs Oct 18, 2023
@d6o d6o marked this pull request as ready for review October 18, 2023 14:28
@d6o d6o force-pushed the fix-prometheus-middleware-rawpath branch from abcb163 to 3f692d7 Compare October 18, 2023 14:50
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

Successfully merging this pull request may close these issues.

None yet

1 participant