Merge branch 'main' into expose-primitives

Author: achudnovskij

src/pg/backup/mod.rs

@@ -9,6 +9,8 @@ use anyhow::{Context, Result, anyhow};
 use chrono::{DateTime, Utc};
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
+use crate::pg::parse_hex;
+
 pub mod copy;
 pub mod delete;
 pub mod delta;
@@ -107,20 +109,17 @@ pub fn format_backup_name(timeline: u32, start_lsn: u64, seg_size: u64) -> Strin
 /// the prefix, is too short, or contains non-hex digits.
 pub fn parse_timeline_from_backup_name(name: &str) -> Option<u32> {
     let rest = name.strip_prefix(BACKUP_NAME_PREFIX)?;
-    if rest.len() < 8 {
-        return None;
-    }
-    u32::from_str_radix(&rest[..8], 16).ok()
+    Some(parse_hex(rest.as_bytes().get(..8)?, 8)? as u32)
 }
 
-/// Parse `0/1A2B3C4D` (postgres pg_lsn text form) into u64
+/// Parse `0/1A2B3C4D` (postgres pg_lsn text form) into u64.
+/// Strict per pg_lsn_in_internal: 1..=8 hex digits per component, no sign
 pub fn parse_pg_lsn(s: &str) -> Result<u64> {
     let s = s.trim();
     let (hi, lo) = s
         .split_once('/')
+        .and_then(|(hi, lo)| parse_hex(hi.as_bytes(), 8).zip(parse_hex(lo.as_bytes(), 8)))
         .ok_or_else(|| anyhow!("bad LSN format: {s}"))?;
-    let hi = u64::from_str_radix(hi, 16).with_context(|| format!("bad LSN hi: {hi}"))?;
-    let lo = u64::from_str_radix(lo, 16).with_context(|| format!("bad LSN lo: {lo}"))?;
     Ok((hi << 32) | lo)
 }
 
@@ -139,13 +138,9 @@ pub fn format_pg_lsn(lsn: u64) -> impl std::fmt::Display {
 
 /// Match `base_<24hex>` and optional `_D_<24hex>` delta and `_<8hex>` LSN
 pub fn looks_like_backup_name(s: &str) -> bool {
-    let Some(rest) = s.strip_prefix(BACKUP_NAME_PREFIX) else {
-        return false;
-    };
-    if rest.len() < 24 {
-        return false;
-    }
-    rest[..24].chars().all(|c| c.is_ascii_hexdigit())
+    s.strip_prefix(BACKUP_NAME_PREFIX)
+        .and_then(|rest| rest.as_bytes().get(..24))
+        .is_some_and(|w| w.iter().all(u8::is_ascii_hexdigit))
 }
 
 /// Strip wal-g sentinel suffix to recover backup name
@@ -566,6 +561,42 @@ mod tests {
         assert_eq!(parse_pg_lsn("FF/FF").unwrap(), (0xFF_u64 << 32) | 0xFF);
     }
 
+    #[test]
+    fn rejects_malformed_lsn() {
+        for s in [
+            "",
+            "1",
+            "/",
+            "1/",
+            "/1",
+            "0x1/0",
+            "+1/0",
+            "1/+0",
+            "-1/0",
+            "000000001/0", // 9 digits, pg caps components at 8
+            "0/000000001",
+            "1FFFFFFFF/0", // hi overflow must not shift into oblivion
+            "0/1FFFFFFFF", // lo overflow must not bleed into hi
+            "1 /0",
+            "0/ 1",
+            "g/0",
+            "1/2/3",
+        ] {
+            assert!(parse_pg_lsn(s).is_err(), "{s:?} should be rejected");
+        }
+    }
+
+    #[test]
+    fn backup_name_parsers_reject_sign_and_multibyte() {
+        assert_eq!(parse_timeline_from_backup_name("base_+0000001rest"), None);
+        // char straddling the 8- and 24-byte windows: must not panic
+        assert_eq!(parse_timeline_from_backup_name("base_0000000é0"), None);
+        assert!(!looks_like_backup_name(&format!(
+            "base_{}é",
+            "0".repeat(23)
+        )));
+    }
+
     #[test]
     fn formats_lsn_uppercase() {
         assert_eq!(format_pg_lsn(0x0300_0000).to_string(), "0/3000000");

src/pg/mod.rs

@@ -6,3 +6,35 @@ pub mod walparser;
 
 pub const WAL_FOLDER: &str = "wal_005";
 pub const BASEBACKUP_FOLDER: &str = "basebackups_005";
+
+/// Fold 1..=max_digits ascii hex bytes into u64; rejects sign, prefix,
+/// whitespace. Callers slice with `.get(..n)` to stay panic-free on
+/// arbitrary byte-length input
+pub(crate) fn parse_hex(b: &[u8], max_digits: usize) -> Option<u64> {
+    debug_assert!(max_digits <= 16);
+    if b.is_empty() || b.len() > max_digits {
+        return None;
+    }
+    b.iter().try_fold(0u64, |acc, &c| {
+        Some((acc << 4) | (c as char).to_digit(16)? as u64)
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn parse_hex_strict() {
+        assert_eq!(parse_hex(b"0", 8), Some(0));
+        assert_eq!(parse_hex(b"DeadBeef", 8), Some(0xDEAD_BEEF));
+        assert_eq!(parse_hex(b"FFFFFFFFFFFFFFFF", 16), Some(u64::MAX));
+        assert_eq!(parse_hex(b"", 8), None);
+        assert_eq!(parse_hex(b"123456789", 8), None);
+        assert_eq!(parse_hex(b"+1", 8), None);
+        assert_eq!(parse_hex(b"-1", 8), None);
+        assert_eq!(parse_hex(b" 1", 8), None);
+        assert_eq!(parse_hex(b"0x1", 8), None);
+        assert_eq!(parse_hex("é1".as_bytes(), 8), None);
+    }
+}

src/pg/wal/segment.rs

@@ -73,19 +73,19 @@ pub enum SegmentError {
 
 impl SegmentName {
     pub fn parse(s: &str) -> Result<Self, SegmentError> {
-        if s.len() != SEGMENT_NAME_LEN {
-            return Err(SegmentError::BadLength(s.len()));
+        let b = s.as_bytes();
+        if b.len() != SEGMENT_NAME_LEN {
+            return Err(SegmentError::BadLength(b.len()));
         }
-        let timeline =
-            u32::from_str_radix(&s[0..8], 16).map_err(|_| SegmentError::NonHex(s.into()))?;
-        let log_id =
-            u32::from_str_radix(&s[8..16], 16).map_err(|_| SegmentError::NonHex(s.into()))?;
-        let seg_no =
-            u32::from_str_radix(&s[16..24], 16).map_err(|_| SegmentError::NonHex(s.into()))?;
+        let field = |i: usize| {
+            crate::pg::parse_hex(&b[i..i + 8], 8)
+                .map(|v| v as u32)
+                .ok_or_else(|| SegmentError::NonHex(s.into()))
+        };
         Ok(SegmentName {
-            timeline,
-            log_id,
-            seg_no,
+            timeline: field(0)?,
+            log_id: field(8)?,
+            seg_no: field(16)?,
         })
     }
 
@@ -162,6 +162,17 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn rejects_sign_and_multibyte() {
+        // from_str_radix would have accepted the '+'
+        assert!(matches!(
+            SegmentName::parse("+00000010000000000000001").unwrap_err(),
+            SegmentError::NonHex(_)
+        ));
+        // 24 bytes with char straddling field boundary: byte slicing must not panic
+        assert!(SegmentName::parse("0000000é000000000000001").is_err());
+    }
+
     #[test]
     fn start_lsn_computation() {
         let s = SegmentName::parse("000000010000000200000003").unwrap();

src/pg/wal_summaries.rs

@@ -28,6 +28,8 @@ use std::path::{Path, PathBuf};
 use roaring::RoaringBitmap;
 use thiserror::Error;
 
+use crate::pg::parse_hex;
+
 use crate::pg::backup::delta::PagedFileDeltaMap;
 use crate::pg::backup::format_pg_lsn;
 use crate::pg::walparser::RelFileNode;
@@ -159,22 +161,15 @@ pub fn list_summary_files(dir: &Path) -> Result<Vec<SummaryFile>, SummaryError>
 /// empty; caller fills). Returns `None` if the name doesn't match
 pub fn parse_summary_filename(name: &str) -> Option<SummaryFile> {
     let stem = name.strip_suffix(".summary")?;
-    if stem.len() != 40 {
-        return None;
-    }
-    if !stem.bytes().all(|b| b.is_ascii_hexdigit()) {
+    let b = stem.as_bytes();
+    if b.len() != 40 {
         return None;
     }
-    let timeline = u32::from_str_radix(&stem[0..8], 16).ok()?;
-    let start_hi = u32::from_str_radix(&stem[8..16], 16).ok()?;
-    let start_lo = u32::from_str_radix(&stem[16..24], 16).ok()?;
-    let end_hi = u32::from_str_radix(&stem[24..32], 16).ok()?;
-    let end_lo = u32::from_str_radix(&stem[32..40], 16).ok()?;
     Some(SummaryFile {
         path: PathBuf::new(),
-        timeline,
-        start_lsn: ((start_hi as u64) << 32) | (start_lo as u64),
-        end_lsn: ((end_hi as u64) << 32) | (end_lo as u64),
+        timeline: parse_hex(&b[0..8], 8)? as u32,
+        start_lsn: parse_hex(&b[8..24], 16)?,
+        end_lsn: parse_hex(&b[24..40], 16)?,
     })
 }