refactor: get rid of indexing and slicing in check_armored_payload() (#15)

Indexing can potentially panic, without indexing
it is easier to make sure there will be no crash.
This commit is contained in:
l
2026-08-18 12:21:27 +02:00
committed by missytake
parent 6f5dd21c8e
commit 3ac927ed2d
2 changed files with 13 additions and 19 deletions
+1 -1
View File
@@ -116,7 +116,7 @@ pub fn check_encrypted(mail: &mailparse::ParsedMail, outgoing: bool) -> bool {
return false; return false;
} }
}; };
if !check_armored_payload(payload, outgoing) { if !check_armored_payload(&payload, outgoing) {
log::debug!("check_encrypted: armored payload check failed"); log::debug!("check_encrypted: armored payload check failed");
return false; return false;
} }
+12 -18
View File
@@ -96,23 +96,19 @@ fn check_openpgp_payload(payload: &[u8]) -> Result<bool, error::Error> {
/// ///
/// Returns `true` if the `payload` is a valid PGP message, /// Returns `true` if the `payload` is a valid PGP message,
/// `outgoing` informs whether the message is outgoing or incoming /// `outgoing` informs whether the message is outgoing or incoming
pub fn check_armored_payload(mut payload: String, outgoing: bool) -> bool { pub fn check_armored_payload(payload: &str, outgoing: bool) -> bool {
const PREFIX: &str = "-----BEGIN PGP MESSAGE-----\r\n"; const PREFIX: &str = "-----BEGIN PGP MESSAGE-----\r\n";
if !payload.starts_with(PREFIX) { let Some(payload) = payload.strip_prefix(PREFIX) else {
log::debug!("check_armored_payload: Did not find PGP MESSAGE prefix"); log::debug!("check_armored_payload: Did not find PGP MESSAGE prefix");
return false; return false;
} };
payload = payload[PREFIX.len()..].to_string();
while payload.ends_with("\r\n") { let payload = payload.trim_end_matches("\r\n");
payload.truncate(payload.len() - 2);
}
const SUFFIX: &str = "-----END PGP MESSAGE-----"; const SUFFIX: &str = "-----END PGP MESSAGE-----";
if !payload.ends_with(SUFFIX) { let Some(mut payload) = payload.strip_suffix(SUFFIX) else {
log::debug!("check_armored_payload: Did not find PGP MESSAGE suffix"); log::debug!("check_armored_payload: Did not find PGP MESSAGE suffix");
return false; return false;
} };
payload.truncate(payload.len() - SUFFIX.len());
const VERSION_COMMENT: &str = "Version: "; const VERSION_COMMENT: &str = "Version: ";
if payload.starts_with(VERSION_COMMENT) { if payload.starts_with(VERSION_COMMENT) {
@@ -123,20 +119,18 @@ pub fn check_armored_payload(mut payload: String, outgoing: bool) -> bool {
} }
// Remove comments from incoming messages // Remove comments from incoming messages
if let Some((_, right)) = payload.split_once("\r\n") { if let Some((_, right)) = payload.split_once("\r\n") {
payload = right.to_string(); payload = right;
} }
} }
while payload.starts_with("\r\n") { let mut payload = payload.trim_start_matches("\r\n");
payload = payload[2..].to_string();
}
// Remove CRC24. // Remove CRC24.
if let Some((left, _)) = payload.rsplit_once('=') { if let Some((left, _)) = payload.rsplit_once('=') {
payload = left.to_string(); payload = left;
} }
payload = payload.replace(['\r', '\n'], ""); let payload = payload.replace(['\r', '\n'], "");
let payload = match BASE64_STANDARD.decode(payload.as_bytes()) { let payload = match BASE64_STANDARD.decode(payload.as_bytes()) {
Ok(v) => v, Ok(v) => v,
Err(_) => { Err(_) => {
@@ -260,10 +254,10 @@ Definitely not base64 encoded PGP message content.
fn test_check_armored_payload(#[case] pgp_message: &str, #[case] expected: (bool, bool)) { fn test_check_armored_payload(#[case] pgp_message: &str, #[case] expected: (bool, bool)) {
let (expected_outgoing, expected_incoming) = expected; let (expected_outgoing, expected_incoming) = expected;
let result = check_armored_payload(pgp_message.replace('\n', "\r\n").to_string(), true); let result = check_armored_payload(&pgp_message.replace('\n', "\r\n"), true);
assert_eq!(result, expected_outgoing); assert_eq!(result, expected_outgoing);
let result = check_armored_payload(pgp_message.replace('\n', "\r\n").to_string(), false); let result = check_armored_payload(&pgp_message.replace('\n', "\r\n"), false);
assert_eq!(result, expected_incoming); assert_eq!(result, expected_incoming);
} }
} }