Skip to content

Add rustup doc --serve to serve docs over local HTTP - #4986

Open
Aneesh-382005 wants to merge 2 commits into
rust-lang:mainfrom
Aneesh-382005:doc-serve-http
Open

Add rustup doc --serve to serve docs over local HTTP#4986
Aneesh-382005 wants to merge 2 commits into
rust-lang:mainfrom
Aneesh-382005:doc-serve-http

Conversation

@Aneesh-382005

Copy link
Copy Markdown

The Problem

rustup doc currently opens the local Rust documentation as file:// URLs using the system's default browser, but sandbox browsers are unable to access the documentation. The browser opens, just to display an "Access denied" error

This PR

Adds an opt-in --serve flag. So instead of building a file:// URL, rustup doc --serve opens an http://127.0.0.1:<port>/...
URL instead, which can be accessed by a sandboxed browser. Default rustup doc behavior is completely unchanged.

Design decisions

  • Bind only to 127.0.0.1 on an ephemeral port.
  • Serve the documentation directory as root so it keeps the relative links/CSS/JS/search working
  • Reject path traversal attempts (.. or absolute paths) to keep requests confined to the doc root.
  • Keep the server running until the user exits with Ctrl+C. Did not go with any idle timeouts because a timeout would be arbitrary and could interrupt reading.
  • Promote hyper (server + http1) from a dev-dependency to a dependency. Although reqwest already depends on hyper, that only exposes its client-side functionality internally, so a direct dependency is required to use the server APIs. Cargo.lock adds no new packages, and the release binary grows from 14,001,792 to 14,204,752 bytes (+198.2 KiB, +1.45%).
  • I did think of automatic fallback but rustup can tell whether the browser was launched, but not whether it successfully rendered the documentation.

Testing

  • cargo build and cargo clippy --all-targets --features test pass cleanly.
  • cargo test --features test download::tests: 5/5 tests pass.
  • Manual: Verified rustup doc --serve and rustup doc --serve std::vec. Docs, assets, and relative links load correctly, invalid paths return 404, and Ctrl-C stops the server and releases the port.

Fixes #4975

@djc djc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this looks like a good start.

View changes since this review

Comment thread src/cli/rustup_mode.rs Outdated
Comment thread src/cli/doc_server.rs Outdated
Comment thread src/cli/doc_server.rs Outdated
Comment thread src/cli/doc_server.rs Outdated
Comment thread src/cli/doc_server.rs Outdated
Comment thread src/cli/doc_server.rs Outdated
Comment thread src/cli/doc_server.rs Outdated
Comment thread src/cli/docs.rs Outdated
Comment thread src/cli/docs.rs Outdated
Comment thread src/cli/docs.rs Outdated

@djc djc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM modulo a few more nits.

Please squash changes addressing review feedback back into the originating commit -- there should be two commits at the end.

View changes since this review

Comment thread src/cli/docs.rs Outdated
Comment thread src/cli/docs.rs Outdated
Comment thread src/cli/docs.rs Outdated
Comment thread src/cli/docs.rs Outdated
Comment thread src/cli/docs.rs Outdated
@Aneesh-382005

Copy link
Copy Markdown
Author

Thanks for the review rounds, appreciate the feedback. Nits addressed, and changes have been squashed into two commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve rustup doc UX with sandboxed default browser

2 participants