fix(server): preserve API mount prefix in server-proxied endpoint URLs#1297
Open
tea-artist wants to merge 1 commit into
Open
fix(server): preserve API mount prefix in server-proxied endpoint URLs#1297tea-artist wants to merge 1 commit into
tea-artist wants to merge 1 commit into
Conversation
The lifecycle API is mounted both bare and under /v1, but the use_server_proxy endpoint rewrite built the proxied URL from request.base_url only, dropping the /v1 prefix the client came through. On shared hosts (path-split deployments where /v1 routes to this server and / belongs to another backend) proxied HTTP requests hit the wrong backend's redirects and WebSocket handshakes failed outright. Derive the mount prefix from the request path and keep it in the rewritten endpoint. server.eip stays verbatim: it is the operator- specified full external base and may already carry its own path prefix. Full server suite: 1126 passed.
tea-artist
requested review from
Generalwin,
Pangjiping,
hittyt,
jwx0925 and
ninan-nn
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes server-proxied endpoint URL rewriting (use_server_proxy=true) so that it preserves the API mount prefix (e.g. /v1) that the client used to reach the lifecycle API, preventing proxied traffic from being routed to the wrong backend when the server is hosted behind path-based routing.
Changes:
- Update
use_server_proxyURL rewriting to derive and retain the request’s mount prefix whenserver.eipis not configured. - Expand endpoint behavior tests to validate both
/v1-mounted and bare-mounted requests produce correctly prefixed proxied URLs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
server/opensandbox_server/api/lifecycle.py |
Preserves the incoming mount prefix when building server-proxied endpoint URLs (unless server.eip is set). |
server/tests/test_routes_endpoint_behavior.py |
Updates/extends test coverage to assert correct proxied URL rewriting for both /v1 and bare mounts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Problem
The lifecycle API is mounted both bare and under
/v1(main.pyincludes every router twice), but theuse_server_proxy=trueendpoint rewrite builds the proxied URL fromrequest.base_urlonly, dropping the mount prefix the client actually came through:On deployments where the server is published behind a path split (e.g. an ingress/reverse proxy routing
/v1/*to opensandbox-server while/belongs to another backend), every proxied request then hits the wrong backend: plain HTTP runs into that backend's redirects, and WebSocket handshakes fail outright since WS clients do not follow redirects. This effectively breaksuse_server_proxy— the option designed exactly for clients that cannot reach sandbox subdomains directly.Fix
Derive the mount prefix from the request path and keep it in the rewritten endpoint.
server.eipis left verbatim: it is the operator-specified full external base and may already carry its own path prefix (existing test covers that semantic).Testing
/v1-mounted expectation and added a bare-mount case intest_routes_endpoint_behavior.py