fix: avoid chi route context reuse#1039
Open
aqr91 wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes a regression introduced by creating autopatch internal GET/PUT requests with the incoming request context.
Using http.NewRequestWithContext(ctx.Context(), ...) preserves user context values, but with humachi it also preserves chi.RouteContext. When autopatch serves the internal GET/PUT through the same adapter/router, chi sees an existing route context and reuses stale routing state from the outer PATCH request. As a result, the internal GET can be routed back to the generated PATCH handler, causing recursion and eventually a panic when the nested request has no body.
Example failure:
Fix
Wrap the incoming context for autopatch internal requests and filter out chi.RouteCtxKey, while preserving all other context behavior and values.
This keeps the intent of #1019:
request-scoped context values are still available;
cancellation and deadlines are preserved;
router-specific state from chi is not leaked into nested internal requests.
Notes:
This is my first PR to a community-driven library, so I may have missed some project-specific code style or contribution conventions. I also did not have much time to review the code style deeply, so I am open to adjusting the implementation if needed.
I tested this fork in my project and confirmed that it fixes the autopatch recursion issue with
humachi/chi. However, my original issue was not about preserving context values for the internal GET/PUT requests, so I cannot be 100% sure that this does not affect the behavior intended by the previous PR. The goal of this change is to preserve the incoming context while filtering out the chi routing state that causes the nested request to be routed incorrectly.