From dad014e8eb889d6289c7ac2f13ec036aa0c9ba14 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 27 Feb 2024 16:40:23 -0500 Subject: [PATCH 1/2] lib: clean up no_std and use of std in test code 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 --- src/lib.rs | 4 ++-- src/tests.rs | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1cdd426..6471dd0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,10 +49,10 @@ unused_extern_crates, unused_qualifications )] -#![cfg_attr(not(test), no_std)] +#![no_std] extern crate alloc; -#[cfg(all(feature = "std", not(test)))] +#[cfg(any(feature = "std", test))] extern crate std; #[cfg(test)] diff --git a/src/tests.rs b/src/tests.rs index 3c32998..290f89b 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,5 +1,8 @@ #[cfg(test)] mod unit { + use alloc::{format, vec}; + use std::prelude::v1::*; + use crate::{Error, Item}; #[test] From a0f5ad0ad3e7710bf67ad9da49dcf9d88261c507 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Tue, 27 Feb 2024 15:50:12 -0500 Subject: [PATCH 2/2] pemfile: also trim leading contiguous content whitespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/pemfile.rs | 67 ++++++++++++++++++++++++++++---- tests/data/whitespace-prefix.crt | 21 ++++++++++ tests/integration.rs | 12 ++++++ 3 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 tests/data/whitespace-prefix.crt diff --git a/src/pemfile.rs b/src/pemfile.rs index f633204..2fe209c 100644 --- a/src/pemfile.rs +++ b/src/pemfile.rs @@ -215,14 +215,8 @@ fn read_one_impl( } if section.is_some() { - let mut trim = 0; - for &b in line.iter().rev() { - match b { - b'\n' | b'\r' | b' ' => trim += 1, - _ => break, - } - } - b64buf.extend(&line[..line.len() - trim]); + // Extend b64buf without leading or trailing whitespace + b64buf.extend(trim_ascii(line)); } Ok(ControlFlow::Continue(())) @@ -266,6 +260,35 @@ fn read_until_newline( } } +/// Trim contiguous leading and trailing whitespace from `line`. +/// +/// We use [u8::is_ascii_whitespace] to determine what is whitespace. +// TODO(XXX): Replace with `[u8]::trim_ascii` once stabilized[0] and available in our MSRV. +// [0]: https://github.com/rust-lang/rust/issues/94035 +const fn trim_ascii(line: &[u8]) -> &[u8] { + let mut bytes = line; + + // Note: A pattern matching based approach (instead of indexing) allows + // making the function const. + while let [first, rest @ ..] = bytes { + if first.is_ascii_whitespace() { + bytes = rest; + } else { + break; + } + } + + while let [rest @ .., last] = bytes { + if last.is_ascii_whitespace() { + bytes = rest; + } else { + break; + } + } + + bytes +} + /// Extract and return all PEM sections by reading `rd`. #[cfg(feature = "std")] pub fn read_all(rd: &mut dyn io::BufRead) -> impl Iterator> + '_ { @@ -284,3 +307,31 @@ mod base64 { ); } use self::base64::Engine; + +#[cfg(test)] +mod tests { + #[test] + fn test_trim_ascii() { + let tests: &[(&[u8], &[u8])] = &[ + (b"", b""), + (b" hello world ", b"hello world"), + (b" hello\t\r\nworld ", b"hello\t\r\nworld"), + (b"\n\r \ttest\t \r\n", b"test"), + (b" \r\n ", b""), + (b"no trimming needed", b"no trimming needed"), + ( + b"\n\n content\n\n more content\n\n", + b"content\n\n more content", + ), + ]; + + for &(input, expected) in tests { + assert_eq!( + super::trim_ascii(input), + expected, + "Failed for input: {:?}", + input, + ); + } + } +} diff --git a/tests/data/whitespace-prefix.crt b/tests/data/whitespace-prefix.crt new file mode 100644 index 0000000..227fa8a --- /dev/null +++ b/tests/data/whitespace-prefix.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- + MIIDaTCCAlGgAwIBAgIJAOq/zL+84IswMA0GCSqGSIb3DQEBCwUAMFoxCzAJBgNV + BAYTAlVTMQswCQYDVQQIDAJOQzEMMAoGA1UEBwwDUlRQMQ8wDQYDVQQKDAZOZXRB + cHAxDTALBgNVBAsMBEVTSVMxEDAOBgNVBAMMB1NTRk1DQ0EwHhcNMTcxMTAxMjEw + OTQyWhcNMjcxMDMwMjEwOTQyWjBaMQswCQYDVQQGEwJVUzELMAkGA1UECAwCTkMx + DDAKBgNVBAcMA1JUUDEPMA0GA1UECgwGTmV0QXBwMQ0wCwYDVQQLDARFU0lTMRAw + DgYDVQQDDAdTU0ZNQ0NBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA + iaD9Ee0Yrdka0I+9GTJBIW/Fp5JU6kyjaxfOldW/R9lEubegXQFhDD2Xi1HZ+fTM + f224glB9xLJXAHhipRK01C2MgC4kSH75WL1iAiYeOBloExqmK6OCX+sdyO7RXm/H + Ra9tN2INWdvyO2pnmxsSnq56mCMsUZLtrRKp89FWgcxLg5r8QxH7xwfh5k54rxjE + 144TD9yrIiQOgRSIRHUrVJ9l/F/gnwzP8wcNABeXwN71Mzl7mliPA703kONQIAyU + 0E0tLpmy/U8dZdMmTBZGB7jI9f95Hl1RunfwhR371a6z38kgkvwrLzl4qflfsPjw + K9n4omNk9rCH9H9tWkxxjwIDAQABozIwMDAdBgNVHQ4EFgQU/bFyCCnqdDFKlQBJ + ExtV6wcMYkEwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAOQMs + Pz2iBD1+3RcSOsahB36WAwPCjgPiXXXpU+Zri11+m6I0Lq+OWtf+YgaQ8ylLmCQd + 0p1wHlYA4qo896SycrhTQfy9GlS/aQqN192k3oBGoJcMIUnGUBGuEvyZ2aDUfkzy + JUqBe+0KaT7pkvvbRL7VUz34I7ouq9fQIRZ26vUDLTY3KM1n/DXBj3e30GHGMV3K + NN2twuLXPNjnryfgpliHU1rwV7r1WvrCVn4StjimP2bO5HGqD/SbiYUL2M9LOuLK + 6mqY4OHumYXq3k7CHrvt0FepsN0L14LYEt1LvpPDFWP3SdN4z4KqT9AGqBaJnhhl + Qiq8GWnAChspdBLxCg== +-----END CERTIFICATE----- diff --git a/tests/integration.rs b/tests/integration.rs index 27d5b9c..d9620ff 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -196,3 +196,15 @@ fn different_line_endings() { assert!(matches!(cert, rustls_pemfile::Item::X509Certificate(_))); } } + +#[test] +fn whitespace_prefix() { + let items = rustls_pemfile::read_all(&mut BufReader::new( + &include_bytes!("data/whitespace-prefix.crt")[..], + )) + .collect::, _>>() + .unwrap(); + + assert_eq!(items.len(), 1); + assert!(matches!(items[0], rustls_pemfile::Item::X509Certificate(_))); +}