From df4374f1dda50aa871bb9739e3f180fd4f14649d Mon Sep 17 00:00:00 2001 From: Maksym Mishchenko Date: Thu, 24 Sep 2026 21:13:33 +0200 Subject: [PATCH] fix: disambiguate prior rule file labels Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- src/interpreter.rs | 5 +- src/tests/interpreter/mod.rs | 91 +++++++++++++++++++++++++++++------- 3 files changed, 79 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c03859..b2fe012e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Rule-conflict diagnostics now retain the current source envelope and identify the previous rule by file, line, and column without nesting a second diagnostic; literal CR/LF characters in the previous file label are escaped. RVM conflict result semantics are unchanged. +- Rule-conflict diagnostics now retain the current source envelope and identify the previous rule by file, line, and column without nesting a second diagnostic; literal percent, CR, and LF characters in the previous file label are percent-encoded. RVM conflict result semantics are unchanged. ## [0.12.0](https://github.com/microsoft/regorus/compare/regorus-v0.11.0...regorus-v0.12.0) - 2026-09-01 diff --git a/src/interpreter.rs b/src/interpreter.rs index 0ac20eb7..00ccef60 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -3892,8 +3892,9 @@ impl Interpreter { r.span() .source .file() - .replace('\n', "\\n") - .replace('\r', "\\r"), + .replace('%', "%25") + .replace('\n', "%0A") + .replace('\r', "%0D"), r.span().line, r.span().col ))); diff --git a/src/tests/interpreter/mod.rs b/src/tests/interpreter/mod.rs index 4bdaa701..0806ea69 100644 --- a/src/tests/interpreter/mod.rs +++ b/src/tests/interpreter/mod.rs @@ -30,6 +30,19 @@ use timer_test_support::{ apply_engine_timer, configure_time_source, reset_time_source, GlobalTimerGuard, }; +fn assert_conflict_diagnostic(message: &str, expected_line: &str) { + let conflict_line = message + .lines() + .find(|line| line.starts_with("error: rule conflicts")) + .unwrap(); + assert_eq!(conflict_line, expected_line); + assert_eq!(message.matches("\n--> ").count(), 1); + assert_eq!(message.matches("| ^").count(), 1); + assert_eq!(message.matches("error: ").count(), 1); + assert!(!message.contains("defined here")); + assert!(!message.contains("p := 1")); +} + #[test] fn rule_conflict_reports_previous_rule_location_without_nested_diagnostic() { let mut engine = Engine::new(); @@ -50,13 +63,11 @@ fn rule_conflict_reports_previous_rule_location_without_nested_diagnostic() { let message = error.to_string(); assert!(message.contains("--> C:\\policy files\\second.rego:2:1")); assert!(message.contains("| p := 2")); - assert!(message.contains("error: rule conflicts with rule at C:\\policy files\\first.rego:2:1")); - assert_eq!(message.matches("\n--> ").count(), 1); - assert_eq!(message.matches("| ^").count(), 1); - assert_eq!(message.matches("error: ").count(), 1); - assert!(!message.contains("defined here")); + assert_conflict_diagnostic( + &message, + "error: rule conflicts with rule at C:\\policy files\\first.rego:2:1", + ); assert!(!message.contains("\n--> C:\\policy files\\first.rego:2:1")); - assert!(!message.contains("p := 1")); assert!(!message.contains('"')); } @@ -104,15 +115,13 @@ fn rule_conflict_reports_escaped_newline_in_previous_rule_file_label() { .lines() .find(|line| line.starts_with("error: rule conflicts")) .unwrap(); - assert_eq!( - conflict_line, - "error: rule conflicts with rule at C:\\policy files\\first\\nsplit.rego:2:1" + assert_conflict_diagnostic( + &message, + "error: rule conflicts with rule at C:\\policy files\\first%0Asplit.rego:2:1", ); assert!(message.contains("--> C:\\policy files\\second.rego:2:1")); assert!(message.contains("| p := 2")); assert!(!conflict_line.contains('\r')); - assert!(!conflict_line.contains('\n')); - assert!(!message.contains("defined here")); assert!(!message.contains('"')); } @@ -138,18 +147,68 @@ fn rule_conflict_reports_escaped_carriage_return_in_previous_rule_file_label() { .lines() .find(|line| line.starts_with("error: rule conflicts")) .unwrap(); - assert_eq!( - conflict_line, - "error: rule conflicts with rule at C:\\policy files\\first\\rsplit.rego:2:1" + assert_conflict_diagnostic( + &message, + "error: rule conflicts with rule at C:\\policy files\\first%0Dsplit.rego:2:1", ); assert!(message.contains("--> C:\\policy files\\second.rego:2:1")); assert!(message.contains("| p := 2")); assert!(!conflict_line.contains('\r')); - assert!(!conflict_line.contains('\n')); - assert!(!message.contains("defined here")); assert!(!message.contains('"')); } +#[test] +fn rule_conflict_preserves_literal_backslash_n_in_previous_file_label() { + let mut engine = Engine::new(); + engine + .add_policy( + r"C:\policy files\first\nsplit.rego".to_string(), + "package test\np := 1\n".to_string(), + ) + .unwrap(); + engine + .add_policy( + r"C:\policy files\second.rego".to_string(), + "package test\np := 2\n".to_string(), + ) + .unwrap(); + + let error = engine.eval_rule("data.test.p".to_string()).unwrap_err(); + let message = error.to_string(); + assert_conflict_diagnostic( + &message, + r"error: rule conflicts with rule at C:\policy files\first\nsplit.rego:2:1", + ); + assert!(message.contains("--> C:\\policy files\\second.rego:2:1")); + assert!(message.contains("| p := 2")); +} + +#[test] +fn rule_conflict_percent_encodes_percent_sequences_in_previous_file_label() { + let mut engine = Engine::new(); + engine + .add_policy( + "C:\\policy files\\first%0Asplit%0D.rego".to_string(), + "package test\np := 1\n".to_string(), + ) + .unwrap(); + engine + .add_policy( + r"C:\policy files\second.rego".to_string(), + "package test\np := 2\n".to_string(), + ) + .unwrap(); + + let error = engine.eval_rule("data.test.p".to_string()).unwrap_err(); + let message = error.to_string(); + assert_conflict_diagnostic( + &message, + "error: rule conflicts with rule at C:\\policy files\\first%250Asplit%250D.rego:2:1", + ); + assert!(message.contains("--> C:\\policy files\\second.rego:2:1")); + assert!(message.contains("| p := 2")); +} + mod timer_test_support { use super::{ExecutionTimerTestConfig, TimeSourceTestConfig}; #[cfg(any(test, not(feature = "std")))]