-
-
Notifications
You must be signed in to change notification settings - Fork 366
fix: prevent httpx.Client base_url mutation across services #1245
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
base: main
Are you sure you want to change the base?
Conversation
When a custom httpx.Client was passed via ClientOptions, the same instance was shared across all Supabase services (PostgREST, Storage, Auth, Functions). Each service mutated the base_url when initialized, causing subsequent requests to other services to hit the wrong API endpoints, resulting in 404 errors and KeyError exceptions. This fix creates independent copies of the httpx_client for each service using copy.copy(), ensuring each service has its own instance with a separate base_url that can be mutated independently. Changes: - Copy httpx_client for each service initialization (auth, postgrest, storage, functions) in both sync and async clients - Add test_httpx_client_base_url_isolation for both sync and async clients to verify services maintain independent base_urls Fixes #1244 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Pull Request Test Coverage Report for Build 18242876423Details
💛 - Coveralls |
I'm not convinced that this is the correct approach to fixing it. I think a much better approach is to avoid modifying input objects in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think copy.copy
is enough to fixing. At least, we need copy.deepcopy
but even then, I'm not sure this is the correct approach. See my other comment for further info.
@o-santi feel free to ditch this PR in favor of yours. |
My thoughts on this is that we want to share the configuration like I think in almost all of the libraries we have a Below is an example of the code inside of async def _request(
self,
method: RequestMethod,
url: str,
json: Optional[dict[Any, Any]] = None,
) -> Response:
try:
response = await self._client.request(method, f"{self.base_url}{url}", json=json)
response.raise_for_status()
except HTTPStatusError as exc:
resp = exc.response.json()
raise StorageApiError(resp["message"], resp["error"], resp["statusCode"])
return response What are your thoughts on this approach @o-santi? |
Yep, I was basically going to do exactly that, minus the manual concatenation of the url. I will open a new PR with these changes shortly. |
@o-santi I'm not sure of the Python side, but if possible lets try to not do URL manipulation using raw string, and leverage some URL type/URL library to handle it. |
Summary
Fixes #1244 by preventing
httpx.Client
base_url mutation when a custom httpx client is shared across services.Problem
When passing a custom
httpx.Client
viaSyncClientOptions(httpx_client=...)
, the same client instance was shared across all Supabase services (PostgREST, Storage, Auth, Functions). Each service mutated thehttpx.Client.base_url
when initialized, causing subsequent requests to other services to hit the wrong API endpoints.Symptoms:
/rest/v1/object/list/...
instead of/storage/v1/object/list/...
KeyError: 'error'
because the 404 response format differs between PostgREST and Storage APIsRoot Cause
The issue occurred in how services handle injected
httpx_client
:Timeline:
httpx_client.base_url = ".../storage/v1/"
(Works)httpx_client.base_url = ".../rest/v1/"
Clobbers storage URL/rest/v1/
→ 404 →KeyError: 'error'
Solution
Create independent copies of the
httpx_client
for each service usingcopy.copy()
. This ensures each service gets its own instance with a separatebase_url
that can be mutated independently, while still sharing the underlying connection pool and transport configuration.Changes
_sync/client.py
to copy httpx_client for auth, postgrest, storage, and functions services_async/client.py
with the same fix for async clientstest_httpx_client_base_url_isolation
for both sync and async clientsTest Plan
✅ New tests added:
test_httpx_client_base_url_isolation
(sync and async)✅ All existing tests pass (34 passed, 12 xfailed, 86 xpassed)
✅ Fix verified to resolve the issue described in #1244
Additional Notes
copy.copy()
method🤖 Generated with Claude Code