fix(proxy): preserve existing query params when replaying requests#681
Open
abhijeet117 wants to merge 1 commit into
Open
fix(proxy): preserve existing query params when replaying requests#681abhijeet117 wants to merge 1 commit into
abhijeet117 wants to merge 1 commit into
Conversation
apply_modifications rebuilt the request URL with parse_qs, which drops blank-valued query params such as "state=" and collapses repeated keys to only their first value. When repeat_request applied a params override, it silently changed parts of the request the caller never asked to touch, which breaks faithful replay for parameter tampering and injection tests. Rebuild the query with parse_qsl and keep_blank_values=True so every existing param, including empty and repeated ones, is carried through unchanged while the requested params are added or updated.
Contributor
Greptile SummaryThis PR updates proxy request replay to keep more of the original query string intact. The main changes are:
Confidence Score: 5/5This looks safe to merge after a small query-override cleanup.
strix/tools/proxy/caido_api.py Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
strix/tools/proxy/caido_api.py:264
**Repeated Overrides Become Strings**
When a caller passes a repeated-value override such as `{"tag": ["a", "b"]}`, `urlencode(pairs)` encodes the Python list representation as one value instead of `tag=a&tag=b`. This makes the replayed request carry a different query value from the caller's requested parameter update.
```suggestion
final_url = urlunparse(parsed._replace(query=urlencode(pairs, doseq=True)))
```
Reviews (1): Last reviewed commit: "fix(proxy): preserve existing query para..." | Re-trigger Greptile |
| if key not in overrides | ||
| ] | ||
| pairs.extend(overrides.items()) | ||
| final_url = urlunparse(parsed._replace(query=urlencode(pairs))) |
Contributor
There was a problem hiding this comment.
Repeated Overrides Become Strings
When a caller passes a repeated-value override such as {"tag": ["a", "b"]}, urlencode(pairs) encodes the Python list representation as one value instead of tag=a&tag=b. This makes the replayed request carry a different query value from the caller's requested parameter update.
Suggested change
| final_url = urlunparse(parsed._replace(query=urlencode(pairs))) | |
| final_url = urlunparse(parsed._replace(query=urlencode(pairs, doseq=True))) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/tools/proxy/caido_api.py
Line: 264
Comment:
**Repeated Overrides Become Strings**
When a caller passes a repeated-value override such as `{"tag": ["a", "b"]}`, `urlencode(pairs)` encodes the Python list representation as one value instead of `tag=a&tag=b`. This makes the replayed request carry a different query value from the caller's requested parameter update.
```suggestion
final_url = urlunparse(parsed._replace(query=urlencode(pairs, doseq=True)))
```
How can I resolve this? If you propose a fix, please make it concise.
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.
repeat_requestrebuilt the URL query withparse_qs, which drops blank-valued params such asstate=and keeps only the first value of a repeated key. Applying aparamsoverride then silently changed parts of the request the caller never touched, breaking faithful replay for parameter tampering and injection tests.The fix uses
parse_qsl(keep_blank_values=True)and rebuilds the query from ordered pairs, so unspecified params (empty and repeated ones included) pass through unchanged while requested params are still added or updated. Adds tests for blank, repeated, and updated params.