fix: check for out-of-field sign chars

Author: david-r-cox

src/parse.rs

@@ -509,23 +509,44 @@ impl<'a, T: FastFloat> Parser<'a, T> {
 
         // Try strict field positioning first (no comment stripping for strict parsing)
         let strict_result = (|| -> Result<WideLine<T>> {
+          // Extract value string and check if we might be missing a negative sign
+          let val_str = line_str.get(L4..R4).ok_or_eyre("")?.trim();
+
+          // Check character just before L4 to ensure we're not missing a sign
+          // If L4 > 0 and the char at L4-1 is '-', we're probably misaligned
+          if L4 > 0 && line_str.len() > L4 {
+            if let Some(prev_char) = line_str.chars().nth(L4 - 1) {
+              if prev_char == '-' || prev_char == '+' {
+                // Sign character right before our field - reject strict parsing
+                return Err(eyre!(
+                  "potential sign character excluded from value field"
+                ));
+              }
+            }
+          }
+
           let first_pair = RowValuePair {
             row_name: line_str.get(L3..R3).ok_or_eyre("")?.trim(),
-            value: fast_float2::parse(
-              line_str.get(L4..R4).ok_or_eyre("")?.trim(),
-            )?,
+            value: fast_float2::parse(val_str)?,
           };
           let second_pair = match line_str.get(L5..R5) {
             Some(row_name) => {
               let row_name = row_name.trim();
               if row_name.is_empty() {
                 None
               } else {
+                // Check for sign before second value too
+                let val2_str = line_str.get(L6..R6).ok_or_eyre("")?.trim();
+                if L6 > 0 && line_str.len() > L6 {
+                  if let Some(prev_char) = line_str.chars().nth(L6 - 1) {
+                    if prev_char == '-' || prev_char == '+' {
+                      return Err(eyre!("potential sign character excluded from second value field"));
+                    }
+                  }
+                }
                 Some(RowValuePair {
                   row_name,
-                  value: fast_float2::parse(
-                    line_str.get(L6..R6).ok_or_eyre("")?.trim(),
-                  )?,
+                  value: fast_float2::parse(val2_str)?,
                 })
               }
             }
@@ -890,23 +911,48 @@ impl<'a, T: FastFloat> Parser<'a, T> {
 
   /// Parse bounds line using strict field positioning
   fn parse_bounds_strict(line: Span) -> Result<BoundsLine<T>> {
-    let length = line.len();
-    let bound_type = BoundType::try_from(line.get(L1..R1).ok_or_eyre("")?)?;
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        let line_str = *line.fragment();
+      } else {
+        let line_str = line;
+      }
+    }
+    let length = line_str.len();
+    let bound_type = BoundType::try_from(line_str.get(L1..R1).ok_or_eyre("")?)?;
     Ok(match bound_type {
       BoundType::Fr | BoundType::Pl => BoundsLine::<T> {
         bound_type,
-        bound_name: line.get(L2..R2).ok_or_eyre("")?.trim(),
-        column_name: line.get(L3..cmp::min(length, R3)).ok_or_eyre("")?.trim(),
+        bound_name: line_str.get(L2..R2).ok_or_eyre("")?.trim(),
+        column_name: line_str
+          .get(L3..cmp::min(length, R3))
+          .ok_or_eyre("")?
+          .trim(),
         value: None,
       },
-      _ => BoundsLine::<T> {
-        bound_type,
-        bound_name: line.get(L2..R2).ok_or_eyre("")?.trim(),
-        column_name: line.get(L3..R3).ok_or_eyre("")?.trim(),
-        value: Some(fast_float2::parse(
-          line.get(L4..cmp::min(length, R4)).ok_or_eyre("")?.trim(),
-        )?),
-      },
+      _ => {
+        // Check for sign character just before the value field
+        if L4 > 0 && line_str.len() > L4 {
+          if let Some(prev_char) = line_str.chars().nth(L4 - 1) {
+            if prev_char == '-' || prev_char == '+' {
+              return Err(eyre!(
+                "potential sign character excluded from value field"
+              ));
+            }
+          }
+        }
+        BoundsLine::<T> {
+          bound_type,
+          bound_name: line_str.get(L2..R2).ok_or_eyre("")?.trim(),
+          column_name: line_str.get(L3..R3).ok_or_eyre("")?.trim(),
+          value: Some(fast_float2::parse(
+            line_str
+              .get(L4..cmp::min(length, R4))
+              .ok_or_eyre("")?
+              .trim(),
+          )?),
+        }
+      }
     })
   }
 

tests/unit.rs

@@ -180,6 +180,55 @@ mod tests {
           }),
         ),
       },
+      // Regression test: line without leading spaces and large negative value
+      TestData {
+        input: "    C0000823  OBJECTRW -401552000.00\n",
+        expected: (
+          "",
+          Some(WideLine {
+            name: "C0000823",
+            first_pair: RowValuePair {
+              row_name: "OBJECTRW",
+              value: -401552000.0,
+            },
+            second_pair: None,
+          }),
+        ),
+      },
+      // Test case with properly formatted strict field positioning
+      TestData {
+        input: "    X99       ROW1             -123.0\n",
+        expected: (
+          "",
+          Some(WideLine {
+            name: "X99",
+            first_pair: RowValuePair {
+              row_name: "ROW1",
+              value: -123.0,
+            },
+            second_pair: None,
+          }),
+        ),
+      },
+      // Test case with two values where sign character falls just before second value field
+      // This should trigger the sign validation check for second value (line 538-546 in parse.rs)
+      TestData {
+        input: "    X98       ROW1              100.0   ROW2                -20\n",
+        expected: (
+          "",
+          Some(WideLine {
+            name: "X98",
+            first_pair: RowValuePair {
+              row_name: "ROW1",
+              value: 100.0,
+            },
+            second_pair: Some(RowValuePair {
+              row_name: "ROW2",
+              value: -20.0,
+            }),
+          }),
+        ),
+      },
     ];
     for case in test_cases {
       cfg_if::cfg_if! {
@@ -578,6 +627,58 @@ mod tests {
           }),
         ),
       },
+      // Test FR (Free) bound type - no value field (line 924-932 in parse.rs)
+      TestData {
+        input: " FR BND1      XFREE\n",
+        expected: (
+          "",
+          Some(BoundsLine {
+            bound_type: BoundType::Fr,
+            bound_name: "BND1",
+            column_name: "XFREE",
+            value: None,
+          }),
+        ),
+      },
+      // Test PL (Plus infinity) bound type - no value field (line 924-932 in parse.rs)
+      TestData {
+        input: " PL BND1      XPLUS\n",
+        expected: (
+          "",
+          Some(BoundsLine {
+            bound_type: BoundType::Pl,
+            bound_name: "BND1",
+            column_name: "XPLUS",
+            value: None,
+          }),
+        ),
+      },
+      // Test FX (Fixed) bound type with value - should trigger sign check (line 934-956 in parse.rs)
+      TestData {
+        input: " FX BND1      XFIXED               -5\n",
+        expected: (
+          "",
+          Some(BoundsLine {
+            bound_type: BoundType::Fx,
+            bound_name: "BND1",
+            column_name: "XFIXED",
+            value: Some(-5.0),
+          }),
+        ),
+      },
+      // Test MI (Minus infinity) bound type with value - should trigger sign check (line 934-956 in parse.rs)
+      TestData {
+        input: " MI BND1      XMINUS\n",
+        expected: (
+          "",
+          Some(BoundsLine {
+            bound_type: BoundType::Mi,
+            bound_name: "BND1",
+            column_name: "XMINUS",
+            value: None,
+          }),
+        ),
+      },
     ];
     for case in test_cases {
       cfg_if::cfg_if! {
@@ -1832,6 +1933,114 @@ ENDATA
     Ok(())
   }
 
+  /// Test sign character validation for columns lines
+  /// When a sign character appears just before the expected field position,
+  /// the strict parser should detect it and fall back to flexible parsing
+  #[test]
+  fn test_columns_sign_validation() -> Result<()> {
+    // Test case: sign character at position L4-1 (position 22, just before value field at 23)
+    // L4 = 23, so position 22 should have the sign character
+    // Format: "    COL1      ROW1     -123\n"
+    // Positions: 0-3 (spaces), 4-11 (COL1), 12-13 (spaces), 14-21 (ROW1), 22 (-), 23-35 (value field)
+    let input = "    COL1      ROW1     -123\n";
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        let info = TracableInfo::new().forward(false).backward(false);
+        let result = Parser::<f32>::columns_line(LocatedSpan::new_extra(input, info));
+      } else {
+        let result = Parser::<f32>::columns_line(input);
+      }
+    }
+    // This should succeed with flexible parsing fallback
+    assert!(result.is_ok());
+
+    // Also test the second value field sign validation
+    // L6 = 48, so position 47 should have the sign character for second value
+    let input2 = "    COL2      ROW1      100   ROW2       -200\n";
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        let info = TracableInfo::new().forward(false).backward(false);
+        let result2 = Parser::<f32>::columns_line(LocatedSpan::new_extra(input2, info));
+      } else {
+        let result2 = Parser::<f32>::columns_line(input2);
+      }
+    }
+    assert!(result2.is_ok());
+
+    Ok(())
+  }
+
+  /// Test sign character validation for bounds lines
+  /// When a sign character appears just before the value field position,
+  /// the strict parser should detect it and fall back to flexible parsing
+  #[test]
+  fn test_bounds_sign_validation() -> Result<()> {
+    // Test case: sign character at position L4-1 (just before value field)
+    // For bound types that require a value (not FR or PL)
+    let input = " UP BND1      VAR1-999.99\n";
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        let info = TracableInfo::new().forward(false).backward(false);
+        let result = Parser::<f32>::bounds_line(LocatedSpan::new_extra(input, info));
+      } else {
+        let result = Parser::<f32>::bounds_line(input);
+      }
+    }
+    // This should succeed with flexible parsing fallback
+    assert!(result.is_ok());
+
+    // Additional test: ensure FR and PL bounds (no value) work correctly
+    let fr_input = " FR BND1      VAR2\n";
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        let info = TracableInfo::new().forward(false).backward(false);
+        let fr_result = Parser::<f32>::bounds_line(LocatedSpan::new_extra(fr_input, info));
+      } else {
+        let fr_result = Parser::<f32>::bounds_line(fr_input);
+      }
+    }
+    assert!(fr_result.is_ok());
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        if let Ok((_, Some(bound))) = fr_result {
+          assert_eq!(bound.bound_type, BoundType::Fr);
+          assert_eq!(bound.value, None);
+        }
+      } else {
+        if let Ok((_, Some(bound))) = fr_result {
+          assert_eq!(bound.bound_type, BoundType::Fr);
+          assert_eq!(bound.value, None);
+        }
+      }
+    }
+
+    let pl_input = " PL BND1      VAR3\n";
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        let info = TracableInfo::new().forward(false).backward(false);
+        let pl_result = Parser::<f32>::bounds_line(LocatedSpan::new_extra(pl_input, info));
+      } else {
+        let pl_result = Parser::<f32>::bounds_line(pl_input);
+      }
+    }
+    assert!(pl_result.is_ok());
+    cfg_if::cfg_if! {
+      if #[cfg(feature = "trace")] {
+        if let Ok((_, Some(bound))) = pl_result {
+          assert_eq!(bound.bound_type, BoundType::Pl);
+          assert_eq!(bound.value, None);
+        }
+      } else {
+        if let Ok((_, Some(bound))) = pl_result {
+          assert_eq!(bound.bound_type, BoundType::Pl);
+          assert_eq!(bound.value, None);
+        }
+      }
+    }
+
+    Ok(())
+  }
+
   /// Test indicator constraints with correct format
   #[test]
   fn test_indicators_format() -> Result<()> {