fix: guard _resolve_proxy against bare-port inputs#9366
Merged
Conversation
`_resolve_proxy(..., ":8080")` previously returned `(8080, ":8080")` because `parsed.hostname` is empty for a bare-port netloc and we fell back to the raw proxy string — so `":8080"` became the public host. Fall back to the inbound `host` arg when `parsed.hostname` is empty.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dmadisetti
approved these changes
Apr 24, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes _resolve_proxy so that bare-port proxy inputs (e.g. ":8080") don’t incorrectly become the public hostname, by falling back to the inbound host when urlparse(...).hostname is empty.
Changes:
- Update
_resolve_proxyto use the inboundhostwhenparsed.hostnameis empty (bare-port proxy case). - Add regression coverage for
":8080"and for empty-string proxy behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
marimo/_server/start.py |
Adjusts proxy parsing to avoid treating ":8080" as an external hostname; falls back to inbound host. |
tests/_server/test_startup_url.py |
Adds parametrized test cases covering bare-port proxy and empty-string proxy inputs. |
Contributor
There was a problem hiding this comment.
No issues found across 2 files
Architecture diagram
sequenceDiagram
participant Main as Server Startup
participant Resolver as _resolve_proxy()
participant Parser as urlparse (Stdlib)
Note over Main,Parser: Determine public-facing Host/Port
Main->>Resolver: resolve(port, host, proxy_string)
opt proxy_string is not None
Resolver->>Parser: Parse proxy_string
Parser-->>Resolver: Parsed (hostname, port, scheme)
Resolver->>Resolver: Extract hostname from Parser
alt NEW: Hostname is empty (e.g. ":8080")
Resolver->>Resolver: CHANGED: Fallback to inbound 'host' argument
else Hostname found
Resolver->>Resolver: Use parsed hostname
end
alt Port is explicitly in proxy
Resolver->>Resolver: Use parsed port
else Proxy is HTTPS
Resolver->>Resolver: Use port 443
else Proxy is HTTP
Resolver->>Resolver: Use port 80
end
end
Resolver-->>Main: Return (external_port, external_host)
Note over Main: Server starts using resolved identity
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.3-dev48 |
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.
_resolve_proxy(..., ":8080")previously returned(8080, ":8080")becauseparsed.hostnameis empty for a bare-port netloc and we fell back to the raw proxy string — so":8080"became the public host.Fall back to the inbound
hostarg whenparsed.hostnameis empty.