fix(print): index page labels once and cap how many pages a range selects (#5967)

crates/print/src/error.rs

@@ -14,6 +14,9 @@ pub enum PrintError {
     /// A range token that is neither a page number nor a page label.
     #[error("{0:?} is not a page number or label")]
     NotAPage(String),
+    /// A range that names more pages than a listing will take, with the limit.
+    #[error("the range selects more than {0} pages")]
+    TooManySelected(usize),
     #[error("{0}")]
     Invalid(String),
     #[error("{0}")]
@@ -33,9 +36,14 @@ pub fn in_english(wording: &str) -> String {
 
 /// What a person can get wrong about what to print, in English, with their holes still in them.
 ///
-/// An interface that reports these in another language translates these three, which is the whole
+/// An interface that reports these in another language translates these four, which is the whole
 /// reason they are gathered here; `wordings_are_said` keeps them in step with the errors.
-pub const WORDINGS: &[&str] = &["there are no pages to print", "page {n} is out of range (1–{count})", "{page} is not a page number or label"];
+pub const WORDINGS: &[&str] = &[
+    "there are no pages to print",
+    "page {n} is out of range (1–{count})",
+    "{page} is not a page number or label",
+    "the range selects more than {limit} pages",
+];
 
 impl PrintError {
     /// This error as one sentence, said in the language `words` translates into.
@@ -51,6 +59,7 @@ impl PrintError {
             }
             // The token goes in quoted, the way it is typed, as the error itself prints it.
             PrintError::NotAPage(token) => say(&words("{page} is not a page number or label"), &[("{page}", format!("{token:?}"))]),
+            PrintError::TooManySelected(limit) => say(&words("the range selects more than {limit} pages"), &[("{limit}", limit.to_string())]),
             fault => fault.to_string(),
         }
     }

crates/print/src/range.rs

@@ -1,7 +1,41 @@
 //! Pages to Print: all, a range ("1-3, 6, 9-", page labels allowed), odd or even pages, reverse.
 
+use std::collections::HashMap;
+
 use crate::PrintError;
 
+/// Most pages one range may select (more when the document itself has more): every selection
+/// becomes a placement on a sheet, and repeating `1-` costs a few bytes of input for each copy of
+/// the whole document.
+pub const MAX_SELECTED: usize = 100_000;
+
+/// The first page with each label, looked up once per token instead of scanning every label.
+struct Labels<'a> {
+    pages: HashMap<&'a str, usize>,
+    /// The longest label, in bytes: a token longer than that is no label, and is not hashed (a
+    /// dash-heavy token would otherwise hash every split of itself).
+    longest: usize,
+}
+
+impl<'a> Labels<'a> {
+    fn new(labels: &'a [String]) -> Self {
+        let mut pages = HashMap::with_capacity(labels.len());
+        for (i, l) in labels.iter().enumerate() {
+            pages.entry(l.as_str()).or_insert(i);
+        }
+        let longest = pages.keys().map(|l| l.len()).max().unwrap_or(0);
+        Labels { pages, longest }
+    }
+
+    fn get(&self, token: &str) -> Option<usize> {
+        if token.len() > self.longest { None } else { self.pages.get(token).copied() }
+    }
+
+    fn contains_key(&self, token: &str) -> bool {
+        self.get(token).is_some()
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)]
 pub enum Subset {
     #[default]
@@ -11,10 +45,10 @@ pub enum Subset {
 }
 
 /// Resolve one range token: a page label, or a number (1-based).
-fn page_of(tok: &str, count: usize, labels: &[String]) -> Result<usize, PrintError> {
+fn page_of(tok: &str, count: usize, labels: &Labels) -> Result<usize, PrintError> {
     let t = tok.trim();
     // Page labels win over numbers (Acrobat's "use logical page numbers").
-    if let Some(i) = labels.iter().position(|l| l == t) {
+    if let Some(i) = labels.get(t) {
         return Ok(i);
     }
     match t.parse::<usize>() {
@@ -27,12 +61,12 @@ fn page_of(tok: &str, count: usize, labels: &[String]) -> Result<usize, PrintErr
 /// Split a range whose endpoint is a logical page label containing a hyphen, such
 /// as "A-1-A-3" or "A-2-". The plain range parser must not mistake the first
 /// hyphen inside the label for the delimiter between endpoints.
-fn hyphenated_label_range(part: &str, count: usize, labels: &[String]) -> Option<(usize, usize)> {
+fn hyphenated_label_range(part: &str, count: usize, labels: &Labels) -> Option<(usize, usize)> {
     for (split, _) in part.match_indices('-') {
         let (Some(first), Some(last)) = (part.get(..split), part.get(split + 1..)) else { continue };
         let (first, last) = (first.trim(), last.trim());
         // Avoid changing the meaning of ordinary numeric and open-ended ranges.
-        if !labels.iter().any(|l| l.contains('-') && (l == first || l == last)) {
+        if ![first, last].iter().any(|l| labels.contains_key(l) && l.contains('-')) {
             continue;
         }
         let from = if first.is_empty() { Some(0) } else { page_of(first, count, labels).ok() };
@@ -52,12 +86,22 @@ pub fn select_pages(count: usize, range: Option<&str>, labels: &[String], subset
     match range.map(str::trim).filter(|r| !r.is_empty()) {
         None => pages.extend(0..count),
         Some(r) => {
+            let index = Labels::new(labels);
+            let labels = &index;
+            let limit = count.max(MAX_SELECTED);
+            let mut selected = 0usize;
+            // Checked before a span is expanded, so the list never grows past the limit.
+            let mut take = |from: usize, to: usize| {
+                selected = selected.saturating_add(from.abs_diff(to).saturating_add(1));
+                if selected > limit { Err(PrintError::TooManySelected(limit)) } else { Ok(()) }
+            };
             for part in r.split([',', ';']).map(str::trim).filter(|p| !p.is_empty()) {
                 // A complete label (e.g. "A-1") still wins over interpreting it as a range.
                 // A range of such labels (e.g. "A-1-A-3") needs a later '-' split.
-                if !labels.iter().any(|l| l == part)
+                if !labels.contains_key(part)
                     && let Some((from, to)) = hyphenated_label_range(part, count, labels)
                 {
+                    take(from, to)?;
                     if from <= to {
                         pages.extend(from..=to);
                     } else {
@@ -67,17 +111,24 @@ pub fn select_pages(count: usize, range: Option<&str>, labels: &[String], subset
                 }
                 match part.split_once('-') {
                     // A label may itself contain a dash ("A-1"): try the whole token first.
-                    Some(_) if labels.iter().any(|l| l == part) => pages.push(page_of(part, count, labels)?),
+                    Some(_) if labels.contains_key(part) => {
+                        take(0, 0)?;
+                        pages.push(page_of(part, count, labels)?);
+                    }
                     Some((a, b)) => {
                         let from = if a.trim().is_empty() { 0 } else { page_of(a, count, labels)? };
                         let to = if b.trim().is_empty() { count.checked_sub(1).ok_or(PrintError::NoPages)? } else { page_of(b, count, labels)? };
+                        take(from, to)?;
                         if from <= to {
                             pages.extend(from..=to);
                         } else {
                             pages.extend((to..=from).rev());
                         }
                     }
-                    None => pages.push(page_of(part, count, labels)?),
+                    None => {
+                        take(0, 0)?;
+                        pages.push(page_of(part, count, labels)?);
+                    }
                 }
             }
         }
@@ -129,4 +180,31 @@ mod tests {
     fn open_ended_range_covers_all_pages() {
         assert_eq!(select_pages(5, Some("-"), &[], Subset::All, false).unwrap(), vec![0, 1, 2, 3, 4]);
     }
+
+    /// Repeating a range is allowed up to the limit, and refused past it before the pages are
+    /// listed: 1,000 pages selected 100 times is the limit, once more is not.
+    #[test]
+    fn a_range_selects_at_most_the_limit() {
+        let whole = vec!["1-"; MAX_SELECTED / 1000].join(",");
+        assert_eq!(select_pages(1000, Some(&whole), &[], Subset::All, false).unwrap().len(), MAX_SELECTED);
+        let e = select_pages(1000, Some(&format!("{whole},1")), &[], Subset::All, false).unwrap_err();
+        assert!(e.to_string().contains("more than 100000 pages"), "{e}");
+        // A document longer than the limit can still be printed whole, once.
+        assert_eq!(select_pages(MAX_SELECTED + 1, Some("1-"), &[], Subset::All, false).unwrap().len(), MAX_SELECTED + 1);
+    }
+
+    /// Labels resolve through one index: the first page with a label wins, and a hyphenated
+    /// label works as an endpoint, among many labels and many dashes.
+    #[test]
+    fn labels_resolve_through_the_index() {
+        let labels: Vec<String> = (0..50_000).map(|i| format!("A-{i}")).collect();
+        assert_eq!(select_pages(labels.len(), Some("A-3-A-5"), &labels, Subset::All, false).unwrap(), vec![3, 4, 5]);
+        let dashes = "-".repeat(10_000);
+        assert!(select_pages(labels.len(), Some(&dashes), &labels, Subset::All, false).is_err());
+        // a token with no label as long as any of its halves costs no hashing of them
+        let long_token = format!("{}A-3", "-".repeat(200_000));
+        assert!(select_pages(labels.len(), Some(&long_token), &labels, Subset::All, false).is_err());
+        let twice = vec!["x".to_string(), "x".to_string()];
+        assert_eq!(select_pages(2, Some("x"), &twice, Subset::All, false).unwrap(), vec![0]);
+    }
 }

crates/print/src/tests.rs

@@ -84,7 +84,7 @@ fn page_selection() {
 /// and the numbers apart from it, and says itself in English exactly as the error prints itself.
 #[test]
 fn wordings_are_said() {
-    let mistakes = [PrintError::NoPages, PrintError::OutOfRange(7, 5), PrintError::NotAPage("x".into())];
+    let mistakes = [PrintError::NoPages, PrintError::OutOfRange(7, 5), PrintError::NotAPage("x".into()), PrintError::TooManySelected(1_000)];
     for e in &mistakes {
         assert_eq!(e.said(&in_english), e.to_string(), "{e:?} says itself in English the way it prints itself");
     }