Skip to content

fix(proxy): properly handle query strings in sandbox proxy requests#485

Merged
Pangjiping merged 4 commits intoalibaba:mainfrom
joaquinescalante23:fix/proxy-query-string-handling
Mar 19, 2026
Merged

fix(proxy): properly handle query strings in sandbox proxy requests#485
Pangjiping merged 4 commits intoalibaba:mainfrom
joaquinescalante23:fix/proxy-query-string-handling

Conversation

@joaquinescalante23
Copy link
Copy Markdown
Contributor

Context

Fixes the issue where GET requests with query parameters fail through the sandbox proxy while POST requests succeed.

Problem

When use_server_proxy=true:

  • POST body-based requests like POST /directories work fine
  • GET query-based requests like GET /files/search?path=/workspace fail with 400 MISSING_QUERY

Root Cause

  1. Query string was embedded directly in the URL string, which can cause encoding issues with httpx
  2. content=request.stream() was passed for all methods including GET, which should not have a body

Fix

  1. Use httpx's params parameter instead of embedding query string in URL for proper encoding
  2. Only pass content body for POST/PUT/PATCH methods, not for GET requests

Verification

  • Syntax validated
  • Change is minimal and surgical
  • Only affects the proxy endpoint handler

Closes: #484 (proxy query parameters issue)

Use httpx params parameter instead of embedding query string in URL
to ensure proper encoding. Only pass content body for POST/PUT/PATCH
methods to avoid potential issues with GET requests that should not
have a body.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1271838dba

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

- Update FakeAsyncClient to capture params in build_request
- Update existing test to expect params separated from URL
- Add test_proxy_forwards_get_request_with_query_params to verify
  the fix for issue alibaba#484 (GET requests failing with 400 MISSING_QUERY
  when using use_server_proxy)
- Include DELETE in methods that forward content body
- DELETE requests may carry JSON/body payload for APIs that need it
- Add test_proxy_forwards_delete_request_with_body to verify
- URL-encoded params are expected (path=%2Fworkspace not path=/workspace)
- Use client.request('DELETE', ...) instead of client.delete() for body content
Copy link
Copy Markdown
Collaborator

@Pangjiping Pangjiping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Pangjiping Pangjiping added bug Something isn't working component/server labels Mar 19, 2026
@Pangjiping Pangjiping self-assigned this Mar 19, 2026
@Pangjiping Pangjiping merged commit c9819cb into alibaba:main Mar 19, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working component/server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

【Bug】use_server_proxy=True breaks files.download – HTTP 400 when reading files from sandbox

2 participants