Fix #11308: update server.port after binding to port 0 by joanaarnauth · Pull Request #14372 · streamlit/streamlit

joanaarnauth · 2026-03-15T18:33:11Z

Describe your changes

This PR fixes an issue where Streamlit displays incorrect URLs when server.port=0.

When port 0 is used, Tornado binds the server to an ephemeral port chosen by the operating system. However, the configuration value server.port remained 0, which caused the CLI to display URLs such as http://localhost:0.

This patch updates server.port with the actual bound port after the TCP socket is created so that the displayed URLs reflect the real listening port.

Screenshot or video (only for visual changes)

Not applicable.

GitHub Issue Link (if applicable)

Closes #11308

Testing Plan

The bug occurs specifically when server.port is set to 0 and the OS assigns an ephemeral port. A unit test was added to verify that the configuration is updated with the real port once the socket is bound.

Unit Tests (JS and/or Python)

Added the test:

test_updates_configured_port_when_binding_to_zero

in:

lib/tests/streamlit/web/server/server_test.py

This test verifies that when server.port=0, the configuration value is updated to the actual assigned port.

E2E Tests

No E2E tests are required because this change only affects the internal server configuration logic and CLI output.

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

snyk-io · 2026-03-15T18:33:23Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

github-actions · 2026-03-15T18:33:23Z

Thanks for contributing to Streamlit! 🎈

Please make sure you have read our Contributing Guide. You can find additional information about Streamlit development in the wiki.

The review process:

Initial triage: A maintainer will apply labels, approve CI to run, and trigger AI-assisted reviews. Your PR may be flagged with status:needs-product-approval if the feature requires product team sign-off.
Code review: A core maintainer will start reviewing your PR once:
- It is marked as 'ready for review', not 'draft'
- It has status:product-approved (or doesn't need it)
- All CI checks pass
- All AI review comments are addressed

We're receiving many contributions and have limited review bandwidth — please expect some delay. We appreciate your patience! 🙏

sfc-gh-lwilby · 2026-03-19T09:25:20Z

Nice fix — the approach is clean and the test covers the happy path well. The core logic is correct: read the actual bound port from the socket and update the config so downstream consumers (browser.serverPort, auth redirect URIs, CLI output) all reflect reality.

One thing I notices is that this uses _sockets, which is a private Tornado attribute

The getattr(http_server, "_sockets", None) approach works, but it's reaching into Tornado's internals. Tornado has a public API for this — you can split listen() into bind_sockets() + add_sockets(), which gives you direct access to the bound sockets:

sockets = tornado.netutil.bind_sockets(port, address)
http_server.add_sockets(sockets)
if port == 0:
    actual_port = sockets[0].getsockname()[1]
    config.set_option("server.port", actual_port, ConfigOption.STREAMLIT_DEFINITION)
    port = actual_port```

This is what listen() does internally, so the behavior is identical — but it avoids coupling to a private attribute that could change across Tornado versions.

What do you think about using this approach instead?

I'm also wondering if you have done some manual testing? Can you post some details?

When Streamlit is started with server.port=0, the OS assigns an ephemeral port. However, the configured server.port value remained 0, which caused the CLI to display URLs such as http://localhost:0. This patch updates server.port with the actual bound port after the socket is created, so the displayed URLs reflect the real listening port. A unit test was added to verify that server.port is updated when binding to port 0.

joanaarnauth · 2026-03-19T12:34:04Z

Nice fix — the approach is clean and the test covers the happy path well. The core logic is correct: read the actual bound port from the socket and update the config so downstream consumers (browser.serverPort, auth redirect URIs, CLI output) all reflect reality.

One thing I notices is that this uses _sockets, which is a private Tornado attribute

The getattr(http_server, "_sockets", None) approach works, but it's reaching into Tornado's internals. Tornado has a public API for this — you can split listen() into bind_sockets() + add_sockets(), which gives you direct access to the bound sockets:
sockets = tornado.netutil.bind_sockets(port, address)
http_server.add_sockets(sockets)
if port == 0:
    actual_port = sockets[0].getsockname()[1]
    config.set_option("server.port", actual_port, ConfigOption.STREAMLIT_DEFINITION)
    port = actual_port```

This is what listen() does internally, so the behavior is identical — but it avoids coupling to a private attribute that could change across Tornado versions.

What do you think about using this approach instead?

I'm also wondering if you have done some manual testing? Can you post some details?

Thanks that makes sense. I updated the implementation to use Tornado's public socket API via bind_sockets() and add_sockets() instead of accessing _sockets.

I also updated the existing tests in server_test.py to reflect this change:

Tests that previously mocked HTTPServer.listen() were updated to mock tornado.netutil.bind_sockets() instead.
Port rotation tests were adapted to validate the arguments passed to bind_sockets() rather than the number of calls to listen().
The tests were made more robust by removing assumptions about the default port (8501) and instead using the current configured port value. This ensures the tests pass reliably when running the full test suite (make python-tests) where other tests may modify the configuration.

I also reran the relevant tests:

uv run pytest lib/tests/streamlit/web/server/server_test.py -k binding_to_zero
uv run pytest lib/tests/streamlit/web/server/server_test.py
uv run pytest lib/tests/streamlit/web/bootstrap_test.py
make python-format
make python-lint
make python-types
make python-tests

For manual testing, I ran:

python -m streamlit run app.py --global.developmentMode false --server.port 0 --server.headless true

Before the fix, Streamlit displayed http://localhost:0.
After the fix, it displayed the actual assigned port (for example http://localhost:36773).

I also confirmed that the process was listening on the same port via ss -ltnp.

github-actions · 2026-03-25T16:44:31Z

⚠️ AI Review Failed

The AI review job failed to complete. Please check the workflow run for details.

You can retry by adding the 'ai-review' label again or manually triggering the workflow.

github-actions

Summary

This PR fixes #11308 where Streamlit displays http://localhost:0 in the CLI when server.port=0 is used to request an OS-assigned ephemeral port. The fix replaces http_server.listen(port, address) with the two-step tornado.netutil.bind_sockets(port, address) + http_server.add_sockets(sockets) pattern, which allows inspecting the actual bound port and updating server.port in the config so that displayed URLs reflect the real listening port. Existing port rotation tests are refactored to mock bind_sockets instead of http_server.listen, and a new DynamicPortTest validates the port-0 behavior with a real socket.

Reviewer consensus: All three reviewers (claude-4.6-opus-high-thinking, gemini-3.1-pro, gpt-5.3-codex-high) approved the PR unanimously. No critical or blocking issues were raised.

Code Quality

The code is clean and follows Streamlit's existing patterns. All three reviewers agreed that the bind_sockets() + add_sockets() pattern is the idiomatic Tornado approach for port discovery — Tornado's HTTPServer.listen() internally calls exactly these two functions, so the refactor is safe and functionally equivalent.

The test refactoring is a net improvement: the old PortRotateOneTest had a stale class-level which_port mock that was never exercised by the code under test. The new tests are more focused, correctly assert against bind_sockets call arguments, and verify that add_sockets is not called when binding fails.

One minor cleanup opportunity was identified by one reviewer: start_listening_unix_socket (around line 241 of server.py) still contains a local import tornado.netutil that is now redundant since the module-level import was added on line 24. This is harmless but could be cleaned up for consistency.

Test Coverage

All reviewers agreed that test coverage is adequate:

DynamicPortTest.test_updates_configured_port_when_binding_to_zero — Uses a real HTTPServer and socket to verify end-to-end that server.port is updated to a real ephemeral port > 0 after binding to port 0. Good integration-style test with proper cleanup.
PortRotateAHundredTest — Correctly updated to assert against mock_bind_sockets.call_count and that add_sockets is never called when all binds fail.
PortRotateOneTest — Now verifies the actual port values passed to successive bind_sockets calls, confirming the increment logic.
PortPermissionDeniedTest — Updated to mock bind_sockets and verify the retry-then-succeed flow.

No E2E tests are needed — this change affects internal server startup logic and CLI output only.

Backwards Compatibility

No breaking changes. All three reviewers agreed the change is fully backwards compatible. The only new behavior — updating server.port when port 0 is used — is additive and only activates when users explicitly opt into port 0. All existing port rotation, permission error retry, and manual port behaviors are preserved.

Security & Risk

No security concerns. All reviewers agreed the change is limited to socket binding mechanics and config updating. No new endpoints, routes, dependencies, auth/session handling, CORS, or external interactions are introduced. The config.set_option call uses ConfigOption.STREAMLIT_DEFINITION, correctly marking the value as set by Streamlit internals.

External test recommendation

Recommend external_test: No
Triggered categories: None
Evidence:
- lib/streamlit/web/server/server.py: Change is confined to the start_listening_tcp_socket function's socket binding mechanism. No routes, handlers, headers, auth, embedding, or cross-origin behavior is affected.
- lib/tests/streamlit/web/server/server_test.py: Only test file changes — no impact on external behavior.
Suggested external_test focus areas: N/A
Confidence: High
Assumptions and gaps: Assumes port 0 usage is a local development scenario, not a production/external deployment pattern.

Reviewer note: Two of three reviewers (claude, gemini) recommended against external tests. One reviewer (gpt) recommended external tests with medium confidence, citing URL behavior changes. After evaluation, the change only updates an internal config value used for CLI display and does not alter any routing, handler, or network-facing behavior, so external tests are not warranted.

Accessibility

No frontend changes — not applicable.

Recommendations

Remove redundant local import (optional): start_listening_unix_socket still has a local import tornado.netutil inside the function body. Since tornado.netutil is now imported at module level, this local import is redundant and can be removed for consistency.

Verdict

APPROVED: Clean, well-tested fix that correctly resolves the port-0 URL display issue using an idiomatic Tornado pattern, with unanimous approval from all three reviewers and no backwards compatibility or security concerns.

Consolidated review by claude-4.6-opus-high-thinking from 3/3 expected reviews (claude-4.6-opus-high-thinking, gemini-3.1-pro, gpt-5.3-codex-high). No reviews failed to complete.

This review also includes 2 inline comment(s) on specific code lines.

github-actions · 2026-03-25T17:18:06Z

lib/streamlit/web/server/server.py

+            # When binding to port 0, Tornado asks the OS for an ephemeral port.
+            # Update server.port with the actual bound port so displayed URLs are correct.
+            if port == 0:
+                actual_port = sockets[0].getsockname()[1]


thought: When server.address is None (the default) and port is 0, bind_sockets may return multiple sockets (e.g., IPv4 + IPv6) each with a different ephemeral port. Using sockets[0] picks just the first one. This is strictly better than the previous behavior (displaying port 0), and in practice Streamlit users typically have a specific address or use localhost, so this is fine. Worth noting as a future enhancement opportunity if multi-interface port consistency is ever needed.

This is actually already handled by Tornado. bind_sockets tracks a bound_port variable — after the first socket binds to port 0 and gets an ephemeral port, subsequent sockets have their port replaced with that value before bind() is called (source). So all returned sockets are guaranteed to share the same port, and sockets[0].getsockname()[1] will be correct.

github-actions · 2026-03-25T17:18:06Z

lib/tests/streamlit/web/server/server_test.py

+        finally:
+            http_server.stop()
+
+            sockets = getattr(http_server, "_sockets", None)


nitpick: http_server._sockets is a private Tornado attribute. Consider relying solely on http_server.stop() (line 788) for cleanup, which should suffice for test teardown. If explicit socket closure is needed for reliability, this approach is acceptable but could break if Tornado renames the internal attribute.

@joanaarnauth this one looks like it could be good to address just for defensibility.

sfc-gh-lwilby · 2026-03-25T18:41:38Z

Remove redundant local import (optional): start_listening_unix_socket still has a local import tornado.netutil inside the function body. Since tornado.netutil is now imported at module level, this local import is redundant and can be removed for consistency.

Would be nice to clean this up as well.

Otherwise looks good, will approve after these small nits! Marking "changes requested" for clarity.

sfc-gh-lwilby

Looks good to me, will approve once the two small changes are made, thanks!

…eanup

joanaarnauth · 2026-03-25T19:10:20Z

Thanks I addressed the requested changes:

removed the redundant local tornado.netutil import
simplified the test cleanup to rely only on http_server.stop()

sfc-gh-lwilby

LGTM, thank you for the contribution @joanaarnauth

joanaarnauth requested a review from a team as a code owner March 15, 2026 18:33

sfc-gh-lwilby added change:bugfix PR contains bug fix implementation impact:users PR changes affect end users labels Mar 19, 2026

sfc-gh-lwilby added ai-review If applied to PR or issue will run AI review workflow and removed ai-review If applied to PR or issue will run AI review workflow labels Mar 19, 2026

joanaarnauth added 2 commits March 19, 2026 10:19

Use Tornado bind_sockets API and update server port rotation tests

65e7fe0

joanaarnauth force-pushed the fix/11308-port-0-url branch from d9fb8ea to 65e7fe0 Compare March 19, 2026 12:24

Merge branch 'develop' into fix/11308-port-0-url

b8af4f5

sfc-gh-lwilby mentioned this pull request Mar 25, 2026

[fix] Resolve server.port=0 for Starlette/UvicornServer path #14521

Draft

3 tasks

github-actions bot reviewed Mar 25, 2026

View reviewed changes

sfc-gh-lwilby requested changes Mar 25, 2026

View reviewed changes

Address review feedback: remove redundant import and simplify test cl…

58281e4

…eanup

sfc-gh-lwilby approved these changes Mar 25, 2026

View reviewed changes

sfc-gh-lwilby enabled auto-merge (squash) March 25, 2026 20:47

sfc-gh-lwilby merged commit 9ec43d1 into streamlit:develop Mar 25, 2026
40 checks passed

Conversation

joanaarnauth commented Mar 15, 2026

Describe your changes

Screenshot or video (only for visual changes)

GitHub Issue Link (if applicable)

Testing Plan

Unit Tests (JS and/or Python)

E2E Tests

Uh oh!

snyk-io bot commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

github-actions bot commented Mar 15, 2026

Uh oh!

sfc-gh-lwilby commented Mar 19, 2026

Uh oh!

joanaarnauth commented Mar 19, 2026

Uh oh!

github-actions bot commented Mar 25, 2026

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Summary

Code Quality

Test Coverage

Backwards Compatibility

Security & Risk

External test recommendation

Accessibility

Recommendations

Verdict

Uh oh!

github-actions bot Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

sfc-gh-lwilby Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

sfc-gh-lwilby Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

sfc-gh-lwilby commented Mar 25, 2026

Uh oh!

sfc-gh-lwilby left a comment

Choose a reason for hiding this comment

Uh oh!

joanaarnauth commented Mar 25, 2026

Uh oh!

sfc-gh-lwilby left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

snyk-io bot commented Mar 15, 2026 •

edited

Loading