You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR enhances the xtask automation tool by improving its reliability, safety, and correctness. These changes align with the "fail loudly" philosophy, ensuring the build and serve pipeline is robust and self-documenting.
Key Changes:
Fail-Fast mdBook Validation:
Added a check_mdbook helper that verifies the presence of the mdbook binary in the system PATH.
Integrated this check into build and deploy commands to provide a clear, actionable error message early in the process, preventing late-stage execution failures.
Robust Directory Resolution & Redirects:
Introduced a ResolveResult enum to strictly type the output of the file resolution logic (File, Redirect, NotFound).
Fixed a bug in the HTTP server where directory requests without a trailing slash (e.g., /python-book) would fail to
resolve relative asset links.
The server now correctly issues a 301 Moved Permanently redirect to the slash-terminated path.
Safe & Idiomatic Signal Handling:
Removed unsafe blocks and manual platform-specific extern "C" declarations for SIGINT (Ctrl+C).
Implemented signal handling using the ctrlc crate, ensuring a clean and safe exit across all operating systems.
Dependency Management:
Updated xtask/Cargo.toml to include the ctrlc dependency.
These improvements ensure that the xtask tool—the entry point for building and previewing the training books—is stable,
predictable, and follows Rust best practices for CLI development.
Thanks for the PR @SyedaAnshrahGillani. A few blockers before this can move forward:
PR#18 overlap — This rewrites resolve_site_file, which I overhauled in PR#18 with multi-layer security (percent-decoding, .. traversal blocking, null byte checks, symlink escape prevention). Your changes seem to preserve these, but the description doesn't mention fix(docs, xtask): repair cross-book links and harden local serve #18 at all. Please explicitly confirm you've reviewed that PR and all security checks are intentionally kept intact.
No testing notes — How was the 301 redirect behavior verified? A quick note would help reviewers trust the fix.
The ctrlc crate swap and ResolveResult enum are solid , just need the above sorted out.
PR#18 Overlap: I have explicitly reviewed the security hardening from PR#18. I can confirm that all security checks (percent-decoding, traversal blocking, null byte checks, and symlink escape prevention) are intentionally preserved in the rewritten resolve_site_file. I've also added explicit comments to the code (in the latest commit) to document these checks and their origin.
Testing Notes: The 301 redirect behavior and security checks were verified using a dedicated test script that simulates the server's resolution logic. The results confirmed:
301 Redirects: Requesting a directory without a trailing slash (e.g., /python-book) correctly returns a 301 Moved Permanently redirect to /python-book/.
Security: Traversal attempts (..), null bytes, and symlink escapes are all correctly identified and blocked (returning NotFound).
Correct Resolution: Requests with trailing slashes correctly resolve to the index.html within the corresponding directory.
I've also verified that the new mdbook check correctly identifies its absence in the PATH, preventing late-stage build failures.
Let me know if you need any further clarification!
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
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.
This PR enhances the xtask automation tool by improving its reliability, safety, and correctness. These changes align with the "fail loudly" philosophy, ensuring the build and serve pipeline is robust and self-documenting.
Key Changes:
Fail-Fast mdBook Validation:
Robust Directory Resolution & Redirects:
resolve relative asset links.
Safe & Idiomatic Signal Handling:
Dependency Management:
These improvements ensure that the xtask tool—the entry point for building and previewing the training books—is stable,
predictable, and follows Rust best practices for CLI development.