Conversation
Problem: Dropping the `color` feature removes the `anstream` layer, and with it all ANSI stripping — logging untrusted input then writes escape sequences verbatim into the output. Stored logs are the worse case: `cat`, `tail -f` and `grep` replay them raw at every future read, so OSC 52 clipboard writes, title-setting and cursor motion keep firing long after the fact. The build cannot just keep `color` to get sanitized: with styling disabled `anstream` still runs its VT state machine over every byte, which triples per-record cost. Summary of Changes: - escape C0 and DEL as `\xNN` in `print` when `color` is off - spare `\n` and `\t`, which the record format itself relies on - borrow via `Cow` so records without controls copy nothing
Problem: The colors warning still tells readers that dropping the `color` feature leaves them unprotected, which no longer holds now that such builds escape control characters themselves. Summary of Changes: - list dropping `color` among the mitigations, noting it escapes rather than strips - call out that `\n` stays unescaped in every configuration
Contributor
|
This is in violation of our AI policy |
Author
|
Is your policy documented somewhere? Would rewriting it manually or in other way allow the contribution to be accepted? |
Contributor
|
https://github.com/rust-cli/env_logger/blob/main/AI_POLICY.md Our contrib guide (and the PR template) say that a decision should be made on Issue before posting PRs |
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
2 participants
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.
What does this PR try to solve?
Closes #419
Escapes C0 and DEL as
\xNNinBufferWriter::printwhencoloris off, sparing\nand\t. ReturnsCow, so records without control characters are borrowed and never copied.Notes to reviewers
Only
#[cfg(not(feature = "color"))]changes, and that path does nothing today — no behavior change for anyone usingcolor.The fast-path scan folds bitwise rather than using
any: short-circuiting stops LLVM vectorizing it, costing ~120 ns/record instead of ~17. Commented so it doesn't get simplified back.Second commit updates the docs warning quoted in #419, which the first makes false.
Verified manually against a payload of CSI, OSC 0/8/52, BEL, CR, DEL and multi-byte UTF-8, under both
--no-default-featuresand--all-features; existing tests pass in both.LLM involvement: written with Claude Code (Opus 5) — investigation, implementation, benchmark harness, commit messages — with me directing and reviewing throughout. Figures in #419 were measured from separately built binaries, each sanity-checked for its sanitization behavior before timing.