Evaluate optional content visibility expressions

crates/pdf-canvas/src/optional_content_filter.rs

@@ -6,7 +6,7 @@ use pdf_optional_content::{OptionalContentMembership, OptionalContentState};
 /// Only the outermost hiding sequence is tracked: everything nested inside hidden
 /// content is hidden too, so inner sequences need no visibility lookup.
 pub(crate) struct OptionalContentFilter<'a> {
-    /// Group visibility the filter applies; at least one group is off.
+    /// Group visibility the filter applies.
     state: &'a OptionalContentState,
     /// Number of open marked-content sequences.
     depth: usize,
@@ -15,14 +15,16 @@ pub(crate) struct OptionalContentFilter<'a> {
 }
 
 impl<'a> OptionalContentFilter<'a> {
-    /// Creates a filter for `state`, or `None` when every group is on and no content
-    /// can be hidden.
-    pub(crate) fn new(state: &'a OptionalContentState) -> Option<Self> {
-        state.hides_any().then_some(Self {
+    /// Creates a filter for `state`.
+    ///
+    /// Content can be hidden even when every group is on, by an `/AnyOff` or `/AllOff`
+    /// policy or a `/Not` visibility expression, so every state is filtered.
+    pub(crate) fn new(state: &'a OptionalContentState) -> Self {
+        Self {
             state,
             depth: 0,
             hidden_from: None,
-        })
+        }
     }
 
     /// Returns a filter over the same state with no open sequences, for a nested canvas.

crates/pdf-canvas/src/pdf_canvas.rs

@@ -132,11 +132,9 @@ impl<'a, B: CanvasBackend> PdfCanvas<'a, B> {
     }
 
     /// Hides content in marked-content sequences whose optional content is off in `state`.
-    ///
-    /// A state with every group on is not attached, so marked content costs nothing.
     #[must_use]
     pub fn with_optional_content(mut self, state: &'a OptionalContentState) -> Self {
-        self.optional_content = OptionalContentFilter::new(state);
+        self.optional_content = Some(OptionalContentFilter::new(state));
         self
     }
 

crates/pdf-document/src/page_tree.rs

@@ -3,7 +3,7 @@
 use pdf_object_reader::{
     FromPdfObject, ObjectAccess, ObjectContext, ObjectReadError, ReadResult, pdf_array::PdfArray,
 };
-use std::{collections::HashMap, sync::Arc};
+use std::{any::TypeId, collections::HashMap, sync::Arc};
 
 use crate::page::PdfPage;
 
@@ -36,6 +36,13 @@ impl FromPdfObject for PageTree {
         );
         Ok(Self { pages })
     }
+
+    /// The root is decoded as the same node its kids are, so a kid naming the root is
+    /// a cycle rather than a second copy of the tree.
+    #[inline]
+    fn cycle_key() -> TypeId {
+        PageTreeNode::cycle_key()
+    }
 }
 
 /// A `/Kids` entry of a page-tree node, dispatched on its `/Type`.

crates/pdf-object-reader/src/context.rs

@@ -143,7 +143,7 @@ impl<'read, A: ObjectAccess + ?Sized> DictionaryContext<'read, A> {
     /// Resolves an entry's reference chain without entering its eager decoding scope.
     ///
     /// For an entry that may refer back to an object being decoded, whose eager read would
-    /// reenter the active path.
+    /// reenter an object still being decoded.
     pub fn resolve_entry(&mut self, key: &[u8]) -> ReadResult<Option<PdfObject>> {
         let Some(object) = self.dictionary.get(key) else {
             return Ok(None);

crates/pdf-object-reader/src/decode.rs

@@ -2,6 +2,7 @@
 
 use num_traits::FromPrimitive;
 use pdf_graphics::{LineCap, LineJoin, MaskMode, TextRenderingMode};
+use std::any::TypeId;
 
 use crate::context::ObjectContext;
 use crate::error::{ObjectReadError, ReadResult};
@@ -22,8 +23,9 @@ use pdf_graphics::{color::Color, interval::Interval, rect::Rect, transform::Tran
 /// Implementations should use context reads for recursion rather than starting
 /// fresh sessions.
 ///
-/// There are no ownership or thread-safety bounds on eager outputs.
-/// Shared reading additionally requires `Self: Send + Sync + 'static`.
+/// Outputs are `'static`, so the reader can tell decoders apart when it detects
+/// cycles (see [`FromPdfObject::cycle_key`]). Shared reading additionally requires
+/// `Self: Send + Sync`.
 ///
 /// Implementations must not retain strong pointers to other cached values as
 /// graph edges: store `ObjectHandle<T>` for indirect edges to avoid ownership
@@ -51,12 +53,22 @@ use pdf_graphics::{color::Color, interval::Interval, rect::Rect, transform::Tran
 ///     let _ = reader.read_shared_indirect::<Local>(id);
 /// }
 /// ```
-pub trait FromPdfObject: Sized {
+pub trait FromPdfObject: Sized + 'static {
     /// Converts the context's object, retaining its session for all nested reads.
     ///
     /// Return domain failures using `ObjectReadError::Decode`; shape and child
     /// failures propagate through `ReadResult`.
     fn from_pdf_object(context: ObjectContext<'_, impl ObjectAccess + ?Sized>) -> ReadResult<Self>;
+
+    /// Identifies this decoder when the reader detects cycles: an object a decoder is
+    /// still decoding cannot be entered again by a decoder with the same key.
+    ///
+    /// A decoder that decodes its object only through another decoder returns that
+    /// decoder's key, so reaching the object through either one is the same visit.
+    #[inline]
+    fn cycle_key() -> TypeId {
+        TypeId::of::<Self>()
+    }
 }
 
 macro_rules! value_decoder {
@@ -120,6 +132,11 @@ impl<T: FromPdfObject> FromPdfObject for Option<T> {
             T::from_pdf_object(context).map(Some)
         }
     }
+
+    #[inline]
+    fn cycle_key() -> TypeId {
+        T::cycle_key()
+    }
 }
 
 /// Decodes array elements in source order within the current traversal.

crates/pdf-object-reader/src/error.rs

@@ -63,21 +63,11 @@ pub enum ObjectReadError {
         #[source]
         source: BoxedError,
     },
-    /// Reference traversal revisited an object active in the current read.
-    #[error("cyclic reference encountered at {repeated:?}; active path: {path:?}")]
+    /// A decoder followed a reference back to an object it was still decoding.
+    #[error("cyclic reference encountered at {repeated:?}")]
     CyclicReference {
-        /// The object identifier repeated in the active path.
+        /// The object the same decoder was already decoding.
         repeated: ObjectId,
-        /// The active traversal path at the point the cycle was detected.
-        path: Vec<ObjectId>,
-    },
-    /// Reference traversal exceeded the configured depth limit.
-    #[error("reference depth exceeded maximum {maximum}; active path: {path:?}")]
-    ReferenceDepthExceeded {
-        /// The maximum permitted number of simultaneously active references.
-        maximum: usize,
-        /// The path whose next reference would exceed the limit.
-        path: Vec<ObjectId>,
     },
     /// Nested decoding, including direct containers, exceeded its depth limit.
     #[error("decoding depth exceeded maximum {maximum}")]

crates/pdf-object-reader/src/reader.rs

@@ -1,5 +1,6 @@
 //! Reader orchestration and session-local traversal state.
 
+use std::any::TypeId;
 use std::collections::HashSet;
 use std::num::NonZeroUsize;
 
@@ -17,27 +18,18 @@ use crate::pdf_object::PdfObject;
 use crate::source::ObjectSource;
 use std::sync::Arc;
 
-/// Bounds reference traversal and nested decoding, including direct containers.
+/// Bounds nested decoding, including direct containers and reference hops.
 ///
-/// Both limits default to 128. They count active stack depth, not total objects.
+/// The limit defaults to 128. It counts active stack depth, not total objects.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct ReadLimits {
-    max_reference_depth: NonZeroUsize,
     max_decode_depth: NonZeroUsize,
 }
 
 impl ReadLimits {
-    /// Configures independent limits for active references and decoder invocations.
-    pub fn new(max_reference_depth: NonZeroUsize, max_decode_depth: NonZeroUsize) -> Self {
-        Self {
-            max_reference_depth,
-            max_decode_depth,
-        }
-    }
-
-    /// Returns the maximum number of simultaneously active indirect references.
-    pub fn max_reference_depth(self) -> NonZeroUsize {
-        self.max_reference_depth
+    /// Configures the limit on simultaneously active decoder invocations.
+    pub fn new(max_decode_depth: NonZeroUsize) -> Self {
+        Self { max_decode_depth }
     }
 
     /// Returns the maximum nesting of decoders, including direct array elements.
@@ -48,10 +40,7 @@ impl ReadLimits {
 
 impl Default for ReadLimits {
     fn default() -> Self {
-        Self::new(
-            NonZeroUsize::new(128).unwrap_or(NonZeroUsize::MIN),
-            NonZeroUsize::new(128).unwrap_or(NonZeroUsize::MIN),
-        )
+        Self::new(NonZeroUsize::new(128).unwrap_or(NonZeroUsize::MIN))
     }
 }
 
@@ -108,7 +97,7 @@ impl<S, C> ObjectReader<S, C> {
         &self.content_stream_ids
     }
 
-    /// Returns the reference and decoding depth limits.
+    /// Returns the decoding depth limit.
     pub fn limits(&self) -> ReadLimits {
         self.limits
     }
@@ -117,7 +106,6 @@ impl<S, C> ObjectReader<S, C> {
     pub fn session(&self) -> ReadSession<'_, S, C> {
         ReadSession {
             reader: self,
-            active_path: Vec::new(),
             active_ids: HashSet::new(),
             decode_depth: 0,
         }
@@ -203,10 +191,10 @@ pub trait ObjectAccess {
     /// decoding must use the read operations, which retain the scope.
     fn resolve(&mut self, object: &ObjectVariant) -> ReadResult<PdfObject>;
 
-    /// Resolves and eagerly decodes within the current active path.
+    /// Resolves and eagerly decodes within the current traversal.
     fn read<T: FromPdfObject>(&mut self, object: &ObjectVariant) -> ReadResult<T>;
 
-    /// Acquires and eagerly decodes within the current active path.
+    /// Acquires and eagerly decodes within the current traversal.
     fn read_indirect<T: FromPdfObject>(&mut self, id: ObjectId) -> ReadResult<T>;
 
     /// Reads through typed caching without waiting on recursive or concurrent loads.
@@ -222,7 +210,12 @@ pub trait ObjectAccess {
 
 /// Borrows a reader and holds the active state of one logical traversal.
 ///
-/// An eager decoder's reference path remains active until it returns. Typed
+/// An object an eager decoder reached by reference stays active until it returns. A cycle is a
+/// decoder re-entering an object it is still decoding, compared by
+/// [`FromPdfObject::cycle_key`]; another decoder may read that object, as when a
+/// membership dictionary names itself in its own visibility expression. Each decoder
+/// decodes an object the same way every time, so only the same decoder re-entering it
+/// could recurse forever. Typed
 /// cache lookups are handled before eager cycle rejection, so recursive shared
 /// reads can return an already-published pending handle. Sibling visits are not
 /// cycles once their earlier traversal scope has ended.
@@ -238,18 +231,12 @@ pub trait ObjectAccess {
 /// ```
 pub struct ReadSession<'reader, S, C = ConcurrentObjectCache> {
     reader: &'reader ObjectReader<S, C>,
-    active_path: Vec<ObjectId>,
-    active_ids: HashSet<ObjectId>,
+    /// The objects being decoded, each with the cycle key of the decoder reading it.
+    active_ids: HashSet<(ObjectId, TypeId)>,
+    /// The number of active decoder invocations, bounded by [`ReadLimits`].
     decode_depth: usize,
 }
 
-impl<S, C> ReadSession<'_, S, C> {
-    /// Returns the current indirect-object path for diagnostics.
-    pub fn active_path(&self) -> &[ObjectId] {
-        &self.active_path
-    }
-}
-
 impl<S, C> ObjectAccess for ReadSession<'_, S, C>
 where
     S: ObjectSource,
@@ -292,21 +279,13 @@ where
 
     fn resolve(&mut self, object: &ObjectVariant) -> ReadResult<PdfObject> {
         // Raw probes have chain-local tracking, so shared recursive edges can be inspected.
-        let mut path = Vec::new();
         let mut ids = HashSet::new();
         let mut object = PdfObject::new(object.clone());
         while let ObjectVariant::Reference(id) = object.value() {
             let id = *id;
             if !ids.insert(id) {
-                return Err(ObjectReadError::CyclicReference { repeated: id, path });
+                return Err(ObjectReadError::CyclicReference { repeated: id });
             }
-            if path.len() >= self.reader.limits.max_reference_depth().get() {
-                return Err(ObjectReadError::ReferenceDepthExceeded {
-                    maximum: self.reader.limits.max_reference_depth().get(),
-                    path,
-                });
-            }
-            path.push(id);
             object = self.load_object(id)?;
         }
         Ok(object)
@@ -381,24 +360,13 @@ where
         // The child owns its traversal scope. Parent state remains intact on errors and unwinding.
         let mut child = ReadSession {
             reader: self.reader,
-            active_path: self.active_path.clone(),
             active_ids: self.active_ids.clone(),
             decode_depth: self.decode_depth.saturating_add(1),
         };
         let object = loop {
-            if !child.active_ids.insert(id) {
-                return Err(ObjectReadError::CyclicReference {
-                    repeated: id,
-                    path: child.active_path,
-                });
-            }
-            if child.active_path.len() >= child.reader.limits.max_reference_depth().get() {
-                return Err(ObjectReadError::ReferenceDepthExceeded {
-                    maximum: child.reader.limits.max_reference_depth().get(),
-                    path: child.active_path,
-                });
+            if !child.active_ids.insert((id, T::cycle_key())) {
+                return Err(ObjectReadError::CyclicReference { repeated: id });
             }
-            child.active_path.push(id);
             let object = child.load_object(id)?;
             match object.value() {
                 ObjectVariant::Reference(next) => id = *next,

crates/pdf-optional-content/src/decode.rs

@@ -122,7 +122,8 @@ impl FromPdfObject for OptionalContentMembership {
                 // `/OCGs` is a single group or an array of them.
                 groups: context.optional_list(b"OCGs")?,
                 policy: context.optional(b"P")?.unwrap_or_default(),
-                has_visibility_expression: context.contains(b"VE"),
+                // A malformed expression falls back to `/OCGs` and `/P`.
+                visibility_expression: context.lenient(b"VE"),
             }),
             // A single group is a membership of that group under the default `AnyOn` policy.
             // A missing or invalid `/Type` is ignored and the dictionary read as a group.
@@ -134,7 +135,7 @@ impl FromPdfObject for OptionalContentMembership {
                         name.as_deref(),
                     )?],
                     policy: OptionalContentPolicy::AnyOn,
-                    has_visibility_expression: false,
+                    visibility_expression: None,
                 })
             }
         }

crates/pdf-optional-content/src/lib.rs

@@ -9,6 +9,8 @@ pub mod error;
 pub mod ocg_state;
 /// Optional content group visibility applied to page content and annotations.
 pub mod optional_content;
+/// `/VE` visibility expressions of optional content membership dictionaries.
+pub mod visibility_expression;
 mod wire;
 
 pub use error::OptionalContentError;
@@ -18,3 +20,4 @@ pub use optional_content::{
     OptionalContentGroupId, OptionalContentMembership, OptionalContentPolicy,
     OptionalContentProperties, OptionalContentState,
 };
+pub use visibility_expression::VisibilityExpression;