Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trim leading contiguous whitespace, fix nightly clippy warns #41

Merged
merged 2 commits into from
Mar 1, 2024

Conversation

cpu
Copy link
Member

@cpu cpu commented Feb 27, 2024

RFC 7468 §2 says:

parsers SHOULD ignore whitespace

Previously we only stripped the trailing whitespace from the base64 content lines within the PEM section boundaries. This branch updates our processing to additionally trim leading contiguous whitespace. This felt more sensible than outright ignoring whitespace (e.g. from the middle of content) and will be sufficient to resolve #40

We base our implementation on the stdlib's unstable [u8]::trim_ascii fn, which we can't (yet) use directly due to our MSRV/stable rust requirements. A TODO is left for future cleanup. Notably this broadens our definition of what whitespace is slightly (e.g. including \t).

Along the way I noticed CI for main is failing on nightly clippy due to redundant import findings. I've applied the same fix strategy we used for Rustls (rustls/rustls#1813) and Webpki (rustls/webpki#234) to resolve this.

Like we just did in Rustls and webpki, _always_ opt-in to no_std, and
then import the std prelude in tests where necessary.

This resolves some nightly clippy warnings about redundant imports that
will arise otherwise
@cpu cpu self-assigned this Feb 27, 2024
src/pemfile.rs Outdated Show resolved Hide resolved
src/pemfile.rs Outdated Show resolved Hide resolved
src/pemfile.rs Outdated Show resolved Hide resolved
RFC 7468 §2[0] says:
> parsers SHOULD ignore whitespace

Previously we only stripped the trailing whitespace from the base64
content lines within the PEM section boundaries. This branch updates our
processing to additionally trim leading contiguous whitespace. This felt
more sensible than outright ignoring whitespace (e.g. from the middle of
content) and will be sufficient to resolve the real world PEM files
we've seen with leading whitespace.

We base our implementation on the stdlib's unstable `[u8]::trim_ascii`
fn, which we can't (yet) use directly due to our MSRV/stable rust
requirements. A TODO is left for future cleanup.

[0]: https://www.rfc-editor.org/rfc/rfc7468#section-2
@cpu cpu requested a review from ctz February 29, 2024 19:52
Copy link
Member

@ctz ctz left a comment

Choose a reason for hiding this comment

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

good stuff!

@cpu cpu merged commit 95152b4 into rustls:main Mar 1, 2024
8 checks passed
@cpu cpu deleted the cpu-whitespace-prefix-ok branch March 1, 2024 14:52
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.

Certificates with indentation fail to parse
3 participants