Unescape path values in chi adapter #821
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses an issue with Chi's handling of encoded path values by ensuring that path parameters are unescaped before use.
- Adds a new test (TestPathParamDecoding) to verify correct decoding of path parameters.
- Updates the Param function in the chi adapter to conditionally unescape path values when RawPath is present.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| adapters/humachi/humachi_test.go | Introduces tests for validating unescaping of path parameters. |
| adapters/humachi/humachi.go | Adds unescaping logic in the Param function for path parameters. |
There was a problem hiding this comment.
Pull Request Overview
This PR addresses an issue in the chi adapter where encoded path parameters were not unescaped, matching the behavior of query parameters in humago. The changes ensure that path parameters are decoded correctly using chi's new PathValue API.
- Updated the Param() method to return an unescaped path value.
- Added tests in humachi_test.go to verify proper decoding of URL-encoded path parameters.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| adapters/humachi/humachi.go | Updated the Param() method to unescape the path parameter value. |
| adapters/humachi/humachi_test.go | Added tests to confirm that URL-encoded path parameters are handled. |
Comments suppressed due to low confidence (1)
adapters/humachi/humachi_test.go:388
- Consider adding a test case that verifies the behavior when the underlying URL.RawPath is non-empty, ensuring that the unescaping branch is fully exercised.
resp = tapi.Get("/test/hello%20%2Fworld%3Ftest%23fragment")
|
Hey @danielgtaylor, can I have some feedback on this? Thank you! |
danielgtaylor
left a comment
There was a problem hiding this comment.
LGTM thanks! 👍 Sorry for the delay in reviewing this!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #821 +/- ##
=======================================
Coverage 92.47% 92.47%
=======================================
Files 23 23
Lines 5502 5502
=======================================
Hits 5088 5088
Misses 354 354
Partials 60 60 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hello, I recently (painfully) discovered Chi has a never-resolved inconsistency with encoded path values:
go-chi/chi#642
Since I noticed that
humagodoesn't suffer from this issue, I think Huma could take care of the Chi issue in thehumachipackage and make sure path values are always unescaped, just like query params