From a8f0e6ac15841507d043cbba677256add4424328 Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Fri, 22 Aug 2025 12:52:02 +0200 Subject: [PATCH 1/3] Refactor error handling --- sqlx-postgres/src/options/pgpass.rs | 110 +++++++++++++++------------- 1 file changed, 59 insertions(+), 51 deletions(-) diff --git a/sqlx-postgres/src/options/pgpass.rs b/sqlx-postgres/src/options/pgpass.rs index bf16559548..6dbeeac241 100644 --- a/sqlx-postgres/src/options/pgpass.rs +++ b/sqlx-postgres/src/options/pgpass.rs @@ -4,6 +4,12 @@ use std::fs::File; use std::io::{BufRead, BufReader}; use std::path::PathBuf; +#[derive(Debug, Clone, thiserror::Error, PartialEq, Eq)] +pub enum PGPassLineParseError { + #[error("Unexpected end of line")] + UnexpectedEOL, +} + /// try to load a password from the various pgpass file locations pub fn load_password( host: &str, @@ -112,8 +118,12 @@ fn load_password_from_reader( } else { // try to load password from line trim_newline(&mut line); - if let Some(password) = load_password_from_line(&line, host, port, username, database) { - return Some(password); + match load_password_from_line(&line, host, port, username, database) { + Err(err) => { + tracing::warn!(line = line, "Malformed line in pgpass file: {err}"); + } + Ok(Some(password)) => return Some(password), + Ok(None) => (), } } @@ -130,45 +140,36 @@ fn load_password_from_line( port: u16, username: &str, database: Option<&str>, -) -> Option { - let whole_line = line; - +) -> Result, PGPassLineParseError> { // Pgpass line ordering: hostname, port, database, username, password // See: https://www.postgresql.org/docs/9.3/libpq-pgpass.html - match line.trim_start().chars().next() { - None | Some('#') => None, - _ => { - matches_next_field(whole_line, &mut line, host)?; - matches_next_field(whole_line, &mut line, &port.to_string())?; - matches_next_field(whole_line, &mut line, database.unwrap_or_default())?; - matches_next_field(whole_line, &mut line, username)?; - Some(line.to_owned()) - } + + if let None | Some('#') = line.trim_end().chars().next() { + return Ok(None); } + + let line_matches = matches_next_field(&mut line, host)? + && matches_next_field(&mut line, &port.to_string())? + && matches_next_field(&mut line, database.unwrap_or_default())? + && matches_next_field(&mut line, username)?; + + if !line_matches { + return Ok(None); + } + + Ok(Some(line.to_owned())) } /// check if the next field matches the provided value -fn matches_next_field(whole_line: &str, line: &mut &str, value: &str) -> Option<()> { - let field = find_next_field(line); - match field { - Some(field) => { - if field == "*" || field == value { - Some(()) - } else { - None - } - } - None => { - tracing::warn!(line = whole_line, "Malformed line in pgpass file"); - None - } - } +fn matches_next_field(line: &mut &str, value: &str) -> Result { + let field = find_next_field(line)?; + Ok(field == "*" || field == value) } /// extract the next value from a line in a pgpass file /// /// `line` will get updated to point behind the field and delimiter -fn find_next_field<'a>(line: &mut &'a str) -> Option> { +fn find_next_field<'a>(line: &mut &'a str) -> Result, PGPassLineParseError> { let mut escaping = false; let mut escaped_string = None; let mut last_added = 0; @@ -181,9 +182,9 @@ fn find_next_field<'a>(line: &mut &'a str) -> Option> { if let Some(mut escaped_string) = escaped_string { escaped_string += &field[last_added..]; - return Some(Cow::Owned(escaped_string)); + return Ok(Cow::Owned(escaped_string)); } else { - return Some(Cow::Borrowed(field)); + return Ok(Cow::Borrowed(field)); } } else if c == '\\' { let s = escaped_string.get_or_insert_with(String::new); @@ -197,66 +198,73 @@ fn find_next_field<'a>(line: &mut &'a str) -> Option> { escaping = !escaping; last_added = idx + 1; } else { + if escaping && c != '\\' && c != ':' { + tracing::warn!("Superfluous escape in in pgpass file"); + } escaping = false; } } - None + Err(PGPassLineParseError::UnexpectedEOL) } #[cfg(test)] mod tests { - use super::{find_next_field, load_password_from_line, load_password_from_reader}; + use super::*; use std::borrow::Cow; #[test] fn test_find_next_field() { - fn test_case<'a>(mut input: &'a str, result: Option>, rest: &str) { + fn test_case<'a>( + mut input: &'a str, + result: Result, PGPassLineParseError>, + rest: &str, + ) { assert_eq!(find_next_field(&mut input), result); assert_eq!(input, rest); } // normal field - test_case("foo:bar:baz", Some(Cow::Borrowed("foo")), "bar:baz"); + test_case("foo:bar:baz", Ok(Cow::Borrowed("foo")), "bar:baz"); // \ escaped test_case( "foo\\\\:bar:baz", - Some(Cow::Owned("foo\\".to_owned())), + Ok(Cow::Owned("foo\\".to_owned())), "bar:baz", ); // : escaped test_case( "foo\\::bar:baz", - Some(Cow::Owned("foo:".to_owned())), + Ok(Cow::Owned("foo:".to_owned())), "bar:baz", ); // unnecessary escape test_case( "foo\\a:bar:baz", - Some(Cow::Owned("fooa".to_owned())), + Ok(Cow::Owned("fooa".to_owned())), "bar:baz", ); // other text after escape test_case( "foo\\\\a:bar:baz", - Some(Cow::Owned("foo\\a".to_owned())), + Ok(Cow::Owned("foo\\a".to_owned())), "bar:baz", ); // double escape test_case( "foo\\\\\\\\a:bar:baz", - Some(Cow::Owned("foo\\\\a".to_owned())), + Ok(Cow::Owned("foo\\\\a".to_owned())), "bar:baz", ); // utf8 support - test_case("🦀:bar:baz", Some(Cow::Borrowed("🦀")), "bar:baz"); + test_case("🦀:bar:baz", Ok(Cow::Borrowed("🦀")), "bar:baz"); // missing delimiter (eof) - test_case("foo", None, "foo"); + test_case("foo", Err(PGPassLineParseError::UnexpectedEOL), "foo"); // missing delimiter after escape - test_case("foo\\:", None, "foo\\:"); + test_case("foo\\:", Err(PGPassLineParseError::UnexpectedEOL), "foo\\:"); // missing delimiter after unused trailing escape - test_case("foo\\", None, "foo\\"); + test_case("foo\\", Err(PGPassLineParseError::UnexpectedEOL), "foo\\"); } #[test] @@ -268,19 +276,19 @@ mod tests { "localhost", 5432, "foo", - Some("bar") + Some("bar"), ), - Some("baz".to_owned()) + Ok(Some("baz".to_owned())) ); // wildcard assert_eq!( load_password_from_line("*:5432:bar:foo:baz", "localhost", 5432, "foo", Some("bar")), - Some("baz".to_owned()) + Ok(Some("baz".to_owned())) ); // accept wildcard with missing db assert_eq!( load_password_from_line("localhost:5432:*:foo:baz", "localhost", 5432, "foo", None), - Some("baz".to_owned()) + Ok(Some("baz".to_owned())) ); // doesn't match @@ -292,7 +300,7 @@ mod tests { "foo", Some("bar") ), - None + Ok(None) ); // malformed entry assert_eq!( @@ -303,7 +311,7 @@ mod tests { "foo", Some("bar") ), - None + Err(PGPassLineParseError::UnexpectedEOL) ); } From c92d9cd8ebb8638c6f72b2492a1abef25631c68c Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Fri, 22 Aug 2025 12:54:28 +0200 Subject: [PATCH 2/3] Add password test cases --- sqlx-postgres/src/options/pgpass.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/sqlx-postgres/src/options/pgpass.rs b/sqlx-postgres/src/options/pgpass.rs index 6dbeeac241..10d044faaa 100644 --- a/sqlx-postgres/src/options/pgpass.rs +++ b/sqlx-postgres/src/options/pgpass.rs @@ -313,6 +313,31 @@ mod tests { ), Err(PGPassLineParseError::UnexpectedEOL) ); + // Password with trailing whitespace + assert_eq!( + load_password_from_line("*:*:*:*:baz ", "localhost", 5432, "foo", Some("bar")), + Ok(Some("baz ".to_owned())) + ); + // Password with escaped colon + assert_eq!( + load_password_from_line("*:*:*:*:ba\\:z", "localhost", 5432, "foo", Some("bar")), + Ok(Some("ba:z".to_owned())) + ); + // Password with escaped backslash + assert_eq!( + load_password_from_line("*:*:*:*:ba\\\\z", "localhost", 5432, "foo", Some("bar")), + Ok(Some("ba\\z".to_owned())) + ); + // Password with superfluous escape + assert_eq!( + load_password_from_line("*:*:*:*:ba\\z", "localhost", 5432, "foo", Some("bar")), + Ok(Some("baz".to_owned())) + ); + // Password with trailing escape + assert_eq!( + load_password_from_line("*:*:*:*:baz\\", "localhost", 5432, "foo", Some("bar")), + Ok(Some("baz".to_owned())) + ); } #[test] From 1cf689bb49fba5510363295b999c0f233ac93077 Mon Sep 17 00:00:00 2001 From: "Kai A. Hiller" Date: Fri, 22 Aug 2025 13:31:45 +0200 Subject: [PATCH 3/3] Unescape password --- sqlx-postgres/src/options/pgpass.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/sqlx-postgres/src/options/pgpass.rs b/sqlx-postgres/src/options/pgpass.rs index 10d044faaa..985e9aaca4 100644 --- a/sqlx-postgres/src/options/pgpass.rs +++ b/sqlx-postgres/src/options/pgpass.rs @@ -157,7 +157,28 @@ fn load_password_from_line( return Ok(None); } - Ok(Some(line.to_owned())) + Ok(Some(unescape_password(line))) +} + +/// Unescape occurrences of `:` and `\` in the given password’s. +fn unescape_password(password_escaped: &str) -> String { + let mut result = String::new(); + + let mut it = password_escaped.chars(); + while let Some(char) = it.next() { + if char != '\\' { + result.push(char); + } else if let Some(c) = it.next() { + if c != ':' && c != '\\' { + tracing::warn!("Superfluous escape in pgpass file"); + } + result.push(c); + } else { + tracing::warn!("Superfluous escape at EOL in pgpass file"); + } + } + + result } /// check if the next field matches the provided value