From 7e4f62fdb07e6c9616da77f1f53902be23a95ee4 Mon Sep 17 00:00:00 2001 From: Xiwei Pan Date: Fri, 18 Sep 2026 00:46:14 +0800 Subject: [PATCH] Validate graph invariants during deserialization --- src/topology/bipartite_graph.rs | 57 ++++++++++++++++------ src/topology/directed_graph.rs | 25 ++++++---- src/topology/graph.rs | 25 ++++++---- src/topology/mixed_graph.rs | 55 +++++++++++++++------ src/topology/planar_graph.rs | 37 +++++++++----- src/unit_tests/topology/bipartite_graph.rs | 19 ++++++++ src/unit_tests/topology/directed_graph.rs | 8 +++ src/unit_tests/topology/graph.rs | 8 +++ src/unit_tests/topology/mixed_graph.rs | 10 ++++ src/unit_tests/topology/planar_graph.rs | 16 ++++++ 10 files changed, 198 insertions(+), 62 deletions(-) diff --git a/src/topology/bipartite_graph.rs b/src/topology/bipartite_graph.rs index a99d5dbdf..28f02e7e0 100644 --- a/src/topology/bipartite_graph.rs +++ b/src/topology/bipartite_graph.rs @@ -22,6 +22,7 @@ use serde::{Deserialize, Serialize}; /// assert!(g.has_edge(0, 2)); // left 0 -> right 0 (unified index 2) /// ``` #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(try_from = "BipartiteGraphData")] pub struct BipartiteGraph { left_size: usize, right_size: usize, @@ -29,6 +30,20 @@ pub struct BipartiteGraph { edges: Vec<(usize, usize)>, } +#[derive(Deserialize)] +struct BipartiteGraphData { + left_size: usize, + right_size: usize, + edges: Vec<(usize, usize)>, +} + +impl TryFrom for BipartiteGraph { + type Error = crate::registry::ConstructionError; + fn try_from(data: BipartiteGraphData) -> Result { + Self::try_new(data.left_size, data.right_size, data.edges) + } +} + impl BipartiteGraph { /// Create a new bipartite graph. /// @@ -40,27 +55,39 @@ impl BipartiteGraph { /// /// # Panics /// - /// Panics if any edge references an out-of-bounds left or right vertex index. + /// Panics if any edge references an out-of-bounds left or right vertex index, + /// or if the combined vertex count overflows `usize`. pub fn new(left_size: usize, right_size: usize, edges: Vec<(usize, usize)>) -> Self { + Self::try_new(left_size, right_size, edges).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + left_size: usize, + right_size: usize, + edges: Vec<(usize, usize)>, + ) -> Result { + left_size + .checked_add(right_size) + .ok_or("bipartite vertex count overflows usize")?; for &(u, v) in &edges { - assert!( - u < left_size, - "left vertex {} out of bounds (left_size={})", - u, - left_size - ); - assert!( - v < right_size, - "right vertex {} out of bounds (right_size={})", - v, - right_size - ); + if u >= left_size { + return Err( + format!("left vertex {} out of bounds (left_size={})", u, left_size).into(), + ); + } + if v >= right_size { + return Err(format!( + "right vertex {} out of bounds (right_size={})", + v, right_size + ) + .into()); + } } - Self { + Ok(Self { left_size, right_size, edges, - } + }) } /// Returns the number of vertices in the left partition. diff --git a/src/topology/directed_graph.rs b/src/topology/directed_graph.rs index 84fe30027..1b30bb469 100644 --- a/src/topology/directed_graph.rs +++ b/src/topology/directed_graph.rs @@ -53,21 +53,28 @@ impl DirectedGraph { /// /// Panics if any arc references a vertex index >= `num_vertices`. pub fn new(num_vertices: usize, arcs: Vec<(usize, usize)>) -> Self { + Self::try_new(num_vertices, arcs).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + arcs: Vec<(usize, usize)>, + ) -> Result { let mut inner = DiGraph::new(); for _ in 0..num_vertices { inner.add_node(()); } for (u, v) in arcs { - assert!( - u < num_vertices && v < num_vertices, - "arc ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "arc ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } inner.add_edge(NodeIndex::new(u), NodeIndex::new(v), ()); } - Self { inner } + Ok(Self { inner }) } /// Creates an empty directed graph with the given number of vertices and no arcs. @@ -263,7 +270,7 @@ impl<'de> Deserialize<'de> for DirectedGraph { arcs: Vec<(usize, usize)>, } let data = GraphData::deserialize(deserializer)?; - Ok(DirectedGraph::new(data.num_vertices, data.arcs)) + DirectedGraph::try_new(data.num_vertices, data.arcs).map_err(serde::de::Error::custom) } } diff --git a/src/topology/graph.rs b/src/topology/graph.rs index 263b64aeb..b21d5d223 100644 --- a/src/topology/graph.rs +++ b/src/topology/graph.rs @@ -116,21 +116,28 @@ impl SimpleGraph { /// /// Panics if any edge references a vertex index >= num_vertices. pub fn new(num_vertices: usize, edges: Vec<(usize, usize)>) -> Self { + Self::try_new(num_vertices, edges).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + edges: Vec<(usize, usize)>, + ) -> Result { let mut inner = UnGraph::new_undirected(); for _ in 0..num_vertices { inner.add_node(()); } for (u, v) in edges { - assert!( - u < num_vertices && v < num_vertices, - "edge ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "edge ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } inner.add_edge(NodeIndex::new(u), NodeIndex::new(v), ()); } - Self { inner } + Ok(Self { inner }) } /// Creates an empty graph with the given number of vertices. @@ -279,7 +286,7 @@ impl<'de> Deserialize<'de> for SimpleGraph { edges: Vec<(usize, usize)>, } let data = GraphData::deserialize(deserializer)?; - Ok(SimpleGraph::new(data.num_vertices, data.edges)) + SimpleGraph::try_new(data.num_vertices, data.edges).map_err(serde::de::Error::custom) } } diff --git a/src/topology/mixed_graph.rs b/src/topology/mixed_graph.rs index 9d95fbac9..af69ea157 100644 --- a/src/topology/mixed_graph.rs +++ b/src/topology/mixed_graph.rs @@ -12,12 +12,27 @@ use serde::{Deserialize, Serialize}; /// so higher-level models can use that order as part of their configuration /// semantics, but edge-membership queries treat them as unordered pairs. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(try_from = "MixedGraphData")] pub struct MixedGraph { num_vertices: usize, arcs: Vec<(usize, usize)>, edges: Vec<(usize, usize)>, } +#[derive(Deserialize)] +struct MixedGraphData { + num_vertices: usize, + arcs: Vec<(usize, usize)>, + edges: Vec<(usize, usize)>, +} + +impl TryFrom for MixedGraph { + type Error = crate::registry::ConstructionError; + fn try_from(data: MixedGraphData) -> Result { + Self::try_new(data.num_vertices, data.arcs, data.edges) + } +} + impl MixedGraph { /// Create a new mixed graph. /// @@ -25,31 +40,39 @@ impl MixedGraph { /// /// Panics if any endpoint references a vertex outside `0..num_vertices`. pub fn new(num_vertices: usize, arcs: Vec<(usize, usize)>, edges: Vec<(usize, usize)>) -> Self { + Self::try_new(num_vertices, arcs, edges).unwrap_or_else(|error| panic!("{error}")) + } + + fn try_new( + num_vertices: usize, + arcs: Vec<(usize, usize)>, + edges: Vec<(usize, usize)>, + ) -> Result { for &(u, v) in &arcs { - assert!( - u < num_vertices && v < num_vertices, - "arc ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "arc ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } } for &(u, v) in &edges { - assert!( - u < num_vertices && v < num_vertices, - "edge ({}, {}) references vertex >= num_vertices ({})", - u, - v, - num_vertices - ); + if !(u < num_vertices && v < num_vertices) { + return Err(format!( + "edge ({}, {}) references vertex >= num_vertices ({})", + u, v, num_vertices + ) + .into()); + } } - Self { + Ok(Self { num_vertices, arcs, edges, - } + }) } /// Create an empty mixed graph with no arcs or undirected edges. diff --git a/src/topology/planar_graph.rs b/src/topology/planar_graph.rs index a29a2b042..868eb7484 100644 --- a/src/topology/planar_graph.rs +++ b/src/topology/planar_graph.rs @@ -19,7 +19,7 @@ use serde::{Deserialize, Serialize}; /// assert_eq!(g.num_vertices(), 4); /// assert_eq!(g.num_edges(), 6); /// ``` -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize)] pub struct PlanarGraph { inner: SimpleGraph, } @@ -30,24 +30,35 @@ impl PlanarGraph { /// # Panics /// Panics if the graph violates the necessary planarity condition |E| <= 3|V| - 6. pub fn new(num_vertices: usize, edges: Vec<(usize, usize)>) -> Self { - let inner = SimpleGraph::new(num_vertices, edges); - if num_vertices >= 3 { - let max_edges = 3 * num_vertices - 6; - assert!( - inner.num_edges() <= max_edges, - "graph has {} edges but a planar graph on {} vertices can have at most {} edges", - inner.num_edges(), - num_vertices, - max_edges - ); - } - Self { inner } + Self::from_inner(SimpleGraph::new(num_vertices, edges)) + .unwrap_or_else(|error| panic!("{error}")) } /// Get a reference to the underlying SimpleGraph. pub fn inner(&self) -> &SimpleGraph { &self.inner } + fn from_inner(inner: SimpleGraph) -> Result { + let num_vertices = inner.num_vertices(); + if num_vertices >= 3 { + let max_edges = 3 * (num_vertices as u128) - 6; + if inner.num_edges() as u128 > max_edges { + return Err(format!("graph has {} edges but a planar graph on {num_vertices} vertices can have at most {max_edges} edges", inner.num_edges()).into()); + } + } + Ok(Self { inner }) + } +} + +impl<'de> Deserialize<'de> for PlanarGraph { + fn deserialize>(deserializer: D) -> Result { + #[derive(Deserialize)] + struct PlanarGraphData { + inner: SimpleGraph, + } + let data = PlanarGraphData::deserialize(deserializer)?; + Self::from_inner(data.inner).map_err(serde::de::Error::custom) + } } impl Graph for PlanarGraph { diff --git a/src/unit_tests/topology/bipartite_graph.rs b/src/unit_tests/topology/bipartite_graph.rs index 46e3b381a..0aa6dd7e8 100644 --- a/src/unit_tests/topology/bipartite_graph.rs +++ b/src/unit_tests/topology/bipartite_graph.rs @@ -58,3 +58,22 @@ fn test_bipartite_graph_invalid_left_index() { fn test_bipartite_graph_invalid_right_index() { BipartiteGraph::new(2, 2, vec![(0, 2)]); } + +#[test] +fn deserialize_checks_partition_endpoints_and_total_size() { + for edges in [vec![(1, 0)], vec![(0, 1)]] { + assert!(serde_json::from_value::(serde_json::json!({ + "left_size": 1, "right_size": 1, "edges": edges + })) + .is_err()); + } + assert!(serde_json::from_value::(serde_json::json!({ + "left_size": usize::MAX, "right_size": 1, "edges": [] + })) + .is_err()); + let graph: BipartiteGraph = serde_json::from_value(serde_json::json!({ + "left_size": 1, "right_size": 1, "edges": [[0, 0]] + })) + .unwrap(); + assert_eq!(graph.edges(), vec![(0, 1)]); +} diff --git a/src/unit_tests/topology/directed_graph.rs b/src/unit_tests/topology/directed_graph.rs index 086728ae0..98b8e420b 100644 --- a/src/unit_tests/topology/directed_graph.rs +++ b/src/unit_tests/topology/directed_graph.rs @@ -234,3 +234,11 @@ fn test_directed_graph_json_format() { fn test_directed_graph_invalid_arc() { DirectedGraph::new(3, vec![(0, 5)]); } + +#[test] +fn deserialize_rejects_out_of_range_arcs() { + assert!(serde_json::from_value::(serde_json::json!({ + "num_vertices": 2, "arcs": [[2, 0]] + })) + .is_err()); +} diff --git a/src/unit_tests/topology/graph.rs b/src/unit_tests/topology/graph.rs index da89fb5ba..f98e2e4d2 100644 --- a/src/unit_tests/topology/graph.rs +++ b/src/unit_tests/topology/graph.rs @@ -153,3 +153,11 @@ fn test_simplegraph_json_format() { assert!(!json_str.contains("node_holes")); assert!(json_str.contains("num_vertices")); } + +#[test] +fn deserialize_rejects_out_of_range_edges() { + assert!(serde_json::from_value::(serde_json::json!({ + "num_vertices": 2, "edges": [[0, 2]] + })) + .is_err()); +} diff --git a/src/unit_tests/topology/mixed_graph.rs b/src/unit_tests/topology/mixed_graph.rs index 316e4ac02..8dcea1800 100644 --- a/src/unit_tests/topology/mixed_graph.rs +++ b/src/unit_tests/topology/mixed_graph.rs @@ -56,3 +56,13 @@ fn test_mixed_graph_serialization_roundtrip() { fn test_mixed_graph_panics_on_out_of_bounds_arc() { MixedGraph::new(3, vec![(0, 3)], vec![]); } + +#[test] +fn deserialize_checks_both_arc_and_edge_endpoints() { + for (arcs, edges) in [(vec![(0, 2)], vec![]), (vec![], vec![(2, 0)])] { + assert!(serde_json::from_value::(serde_json::json!({ + "num_vertices": 2, "arcs": arcs, "edges": edges + })) + .is_err()); + } +} diff --git a/src/unit_tests/topology/planar_graph.rs b/src/unit_tests/topology/planar_graph.rs index 7fae09cb6..463985412 100644 --- a/src/unit_tests/topology/planar_graph.rs +++ b/src/unit_tests/topology/planar_graph.rs @@ -44,3 +44,19 @@ fn test_planar_graph_tree() { let g = PlanarGraph::new(4, vec![(0, 1), (1, 2), (2, 3)]); assert_eq!(g.num_edges(), 3); } + +#[test] +fn deserialize_checks_the_edge_bound() { + let edges: Vec<_> = (0..5) + .flat_map(|u| ((u + 1)..5).map(move |v| (u, v))) + .collect(); + assert!(serde_json::from_value::(serde_json::json!({ + "inner": {"num_vertices": 5, "edges": edges} + })) + .is_err()); + let graph: PlanarGraph = serde_json::from_value(serde_json::json!({ + "inner": {"num_vertices": 2, "edges": [[0, 1]]} + })) + .unwrap(); + assert_eq!(graph.num_edges(), 1); +}