diff --git a/rust/pact_matching/src/engine/interpreter.rs b/rust/pact_matching/src/engine/interpreter.rs index 62bfb323..e5db8f59 100644 --- a/rust/pact_matching/src/engine/interpreter.rs +++ b/rust/pact_matching/src/engine/interpreter.rs @@ -20,7 +20,7 @@ use crate::engine::context::PlanMatchingContext; use crate::engine::value_resolvers::ValueResolver; use crate::field_rules::scope_from_plan_path; #[cfg(feature = "xml")] use crate::engine::xml::XmlValue; -use crate::headers::{parse_charset_parameters, strip_whitespace}; +use crate::headers::{parse_charset_parameters, split_header_field_lines, strip_whitespace}; use crate::json::type_of; use crate::matchingrules::DoMatch; #[cfg(feature = "xml")] use crate::xml::resolve_attr_namespaces; @@ -321,6 +321,7 @@ impl ExecutionPlanInterpreter { "error" => self.execute_error(action, value_resolver, node, &action_path), "header:parse" => self.execute_header_parse(action, value_resolver, node, &action_path), "header:normalize-commas" => self.execute_normalize_comma_whitespace(action, value_resolver, node, &action_path), + "header:split-values" => self.execute_split_header_values(value_resolver, node, &action_path), "for-each" => self.execute_for_each(value_resolver, node, &action_path), #[cfg(feature = "multipart")] "multipart:parse" => self.execute_multipart_parse(action, value_resolver, node, &action_path), @@ -2547,6 +2548,32 @@ impl ExecutionPlanInterpreter { } } + fn execute_split_header_values( + &mut self, + value_resolver: &dyn ValueResolver, + node: ExecutionPlanNode, + action_path: &Vec + ) -> ExecutionPlanNode { + let ExecutionPlanNode { node_type, children, .. } = node; + let (result_children, values) = match self.evaluate_children(value_resolver, &node_type, children, action_path, true) { + Ok(value) => value, + Err(value) => return value + }; + let values = values.iter() + .flat_map(|value| match value.as_value().unwrap_or_default() { + NodeValue::STRING(value) => split_header_field_lines(&[value]), + NodeValue::SLIST(values) => split_header_field_lines(&values), + _ => vec![] + }) + .collect(); + + ExecutionPlanNode { + node_type, + result: Some(NodeResult::VALUE(NodeValue::SLIST(values))), + children: result_children + } + } + fn execute_for_each( &mut self, value_resolver: &dyn ValueResolver, diff --git a/rust/pact_matching/src/engine/mod.rs b/rust/pact_matching/src/engine/mod.rs index b4e343b6..e9fc6170 100644 --- a/rust/pact_matching/src/engine/mod.rs +++ b/rust/pact_matching/src/engine/mod.rs @@ -18,7 +18,7 @@ use serde_json::Value::Object; use tracing::debug; use pact_models::bodies::OptionalBody; use pact_models::content_types::TEXT; -use pact_models::headers::{MULTI_VALUE_HEADERS, PARAMETERISED_HEADERS}; +use pact_models::headers::{MULTI_VALUE_HEADERS, PARAMETERISED_HEADERS, SINGLE_VALUE_HEADERS}; use pact_models::json_utils::json_to_string; use pact_models::http_parts::HttpPart; use pact_models::matchingrules::{MatchingRule, RuleList, RuleLogic}; @@ -1989,11 +1989,21 @@ fn setup_header_plan( let item_path = DocPath::root().join(key); let path = doc_path.join(key); + let actual_value = if value.len() > 1 + && !SINGLE_VALUE_HEADERS.contains(&key.to_lowercase().as_str()) + && !PARAMETERISED_HEADERS.contains(&key.to_lowercase().as_str()) + && !MULTI_VALUE_HEADERS.contains(&key.to_lowercase().as_str()) { + let mut split_values = ExecutionPlanNode::action("header:split-values"); + split_values.add(ExecutionPlanNode::resolve_value(path.clone())); + split_values + } else { + ExecutionPlanNode::resolve_value(path.clone()) + }; if context.matcher_is_defined(&item_path) { let matchers = context.select_best_matcher(&item_path); item_node.add(ExecutionPlanNode::annotation(format!("{} {}", key, matchers.generate_description(true)))); presence_check.add(build_matching_rule_node(&ExecutionPlanNode::value_node(item_value), - &ExecutionPlanNode::resolve_value(&path), &matchers, true, + &actual_value, &matchers, true, context.config.show_types_in_errors)); } else if PARAMETERISED_HEADERS.contains(&key.to_lowercase().as_str()) { item_node.add(ExecutionPlanNode::annotation(format!("{}={}", key, item_value.to_string()))); @@ -2031,7 +2041,7 @@ fn setup_header_plan( } else { item_check .add(ExecutionPlanNode::value_node(item_value)) - .add(ExecutionPlanNode::resolve_value(doc_path.join(key))) + .add(actual_value) .add(ExecutionPlanNode::value_node(NodeValue::NULL)) .add(ExecutionPlanNode::value_node(context.config.show_types_in_errors)); } diff --git a/rust/pact_matching/src/engine/tests/header_tests.rs b/rust/pact_matching/src/engine/tests/header_tests.rs index 3b4d1a80..a1e5f23d 100644 --- a/rust/pact_matching/src/engine/tests/header_tests.rs +++ b/rust/pact_matching/src/engine/tests/header_tests.rs @@ -3,12 +3,38 @@ use maplit::hashmap; use pact_models::matchingrules; use pact_models::matchingrules::MatchingRule; use pact_models::prelude::v4::SynchronousHttp; -use pact_models::v4::http_parts::HttpRequest; +use pact_models::v4::http_parts::{HttpRequest, HttpResponse}; use pact_models::v4::interaction::V4Interaction; -use crate::engine::{execute_request_plan, ExecutionPlan, setup_header_plan}; +use crate::engine::{ + build_request_plan, build_response_plan, execute_request_plan, execute_response_plan, + ExecutionPlan, NodeResult, NodeValue, setup_header_plan +}; use crate::engine::context::PlanMatchingContext; +#[test] +fn match_repeated_custom_headers_in_request_and_response_plans() { + let expected_headers = Some(hashmap! { + "X-Request".to_string() => vec!["request1".to_string(), "request2".to_string()] + }); + let actual_headers = Some(hashmap! { + "X-Request".to_string() => vec!["request1, request2".to_string()] + }); + let context = PlanMatchingContext::default(); + + let expected_request = HttpRequest { headers: expected_headers.clone(), ..HttpRequest::default() }; + let actual_request = HttpRequest { headers: actual_headers.clone(), ..HttpRequest::default() }; + let request_plan = build_request_plan(&expected_request, &context).unwrap(); + let request_result = execute_request_plan(&request_plan, &actual_request, &context).unwrap(); + assert_eq!(request_result.plan_root.result, Some(NodeResult::VALUE(NodeValue::BOOL(true)))); + + let expected_response = HttpResponse { headers: expected_headers, ..HttpResponse::default() }; + let actual_response = HttpResponse { headers: actual_headers, ..HttpResponse::default() }; + let response_plan = build_response_plan(&expected_response, &context).unwrap(); + let response_result = execute_response_plan(&response_plan, &actual_response, &context).unwrap(); + assert_eq!(response_result.plan_root.result, Some(NodeResult::VALUE(NodeValue::BOOL(true)))); +} + #[test] fn match_headers_where_there_are_none() { let expected = HttpRequest::default(); @@ -570,7 +596,9 @@ fn match_headers_with_values_having_different_lengths() { ), %match:equality ( ['test', 'test2'], - $.headers['REF-CODE'], + %header:split-values ( + $.headers['REF-CODE'] + ), NULL, BOOL(false) ), @@ -635,7 +663,9 @@ fn match_headers_with_values_having_different_lengths() { ) => BOOL(true), %match:equality ( ['test', 'test2'] => ['test', 'test2'], - $.headers['REF-CODE'] => ['test', 'test2'], + %header:split-values ( + $.headers['REF-CODE'] => ['test', 'test2'] + ) => ['test', 'test2'], NULL => NULL, BOOL(false) => BOOL(false) ) => BOOL(true), @@ -700,10 +730,12 @@ fn match_headers_with_values_having_different_lengths() { ) => BOOL(true), %match:equality ( ['test', 'test2'] => ['test', 'test2'], - $.headers['REF-CODE'] => 'test', + %header:split-values ( + $.headers['REF-CODE'] => 'test' + ) => ['test'], NULL => NULL, BOOL(false) => BOOL(false) - ) => ERROR(Expected 'test' to be equal to ['test', 'test2']), + ) => ERROR(Expected ['test'] to be equal to ['test', 'test2']), %error ( 'Expected header \'REF-CODE\' but was missing' ) @@ -1143,7 +1175,9 @@ fn match_headers_with_min_type_matching_rules() { ), %match:min-type ( ['1234', '4567'], - $.headers['REF-ID'], + %header:split-values ( + $.headers['REF-ID'] + ), json:{"min":2}, BOOL(false) ), @@ -1208,7 +1242,9 @@ fn match_headers_with_min_type_matching_rules() { ) => BOOL(true), %match:min-type ( ['1234', '4567'] => ['1234', '4567'], - $.headers['REF-ID'] => ['1', '1', '1'], + %header:split-values ( + $.headers['REF-ID'] => ['1', '1', '1'] + ) => ['1', '1', '1'], json:{"min":2} => json:{"min":2}, BOOL(false) => BOOL(false) ) => BOOL(true), @@ -1273,7 +1309,9 @@ fn match_headers_with_min_type_matching_rules() { ) => BOOL(true), %match:min-type ( ['1234', '4567'] => ['1234', '4567'], - $.headers['REF-ID'] => '1', + %header:split-values ( + $.headers['REF-ID'] => '1' + ) => ['1'], json:{"min":2} => json:{"min":2}, BOOL(false) => BOOL(false) ) => ERROR(Expected [1] \(size 1\) to have minimum size of 2), diff --git a/rust/pact_matching/src/headers.rs b/rust/pact_matching/src/headers.rs index d7bfc468..8fbcd7f5 100644 --- a/rust/pact_matching/src/headers.rs +++ b/rust/pact_matching/src/headers.rs @@ -5,7 +5,7 @@ use std::iter::FromIterator; use itertools::Itertools; use maplit::hashmap; -use pact_models::headers::PARAMETERISED_HEADERS; +use pact_models::headers::{PARAMETERISED_HEADERS, SINGLE_VALUE_HEADERS}; use pact_models::matchingrules::MatchingRule; use pact_models::path_exp::DocPath; use tracing::{instrument, debug}; @@ -139,6 +139,34 @@ fn find_entry(map: &HashMap, key: &str) -> Option<(String, T)> whe } } +pub(crate) fn split_header_field_lines(values: &[String]) -> Vec { + values.iter().flat_map(|value| { + let mut quoted = false; + let mut escaped = false; + let mut nested = 0usize; + let mut start = 0; + let mut members = vec![]; + for (index, character) in value.char_indices() { + if escaped { + escaped = false; + } else if quoted && character == '\\' { + escaped = true; + } else if character == '"' { + quoted = !quoted; + } else if !quoted && (character == '{' || character == '[') { + nested += 1; + } else if !quoted && (character == '}' || character == ']') { + nested = nested.saturating_sub(1); + } else if character == ',' && !quoted && nested == 0 { + members.push(value[start..index].trim().to_string()); + start = index + 1; + } + } + members.push(value[start..].trim().to_string()); + members + }).collect() +} + fn match_header_maps( expected: HashMap>, actual: HashMap>, @@ -154,6 +182,12 @@ fn match_header_maps( mismatch: format!("Expected an empty header '{}' but actual value was '{}'", key, actual_values.join(", ")) }]); } else { let mut mismatches = vec![]; + let actual_values = if value.len() > 1 + && !SINGLE_VALUE_HEADERS.contains(&key.to_lowercase().as_str()) { + split_header_field_lines(&actual_values) + } else { + actual_values + }; // Special case when the headers only have 1 value to improve messaging if value.len() == 1 && actual_values.len() == 1 { @@ -235,6 +269,7 @@ mod tests { use pact_models::matchingrules::MatchingRule; use pact_models::matchingrules::expressions::{MatchingRuleDefinition, ValueType}; use pretty_assertions::assert_eq; + use rstest::rstest; use crate::{CoreMatchingContext, DiffConfig, HeaderMatchingContext, Mismatch, CommonMismatch}; use crate::headers::{match_header_value, match_headers, parse_charset_parameters}; @@ -565,6 +600,93 @@ mod tests { }); } + #[test] + fn match_headers_repeated_custom_header_matches_combined_field_line() { + let expected = Some(hashmap! { + "X-Request".to_string() => vec!["request1".to_string(), "request2".to_string()] + }); + let actual = Some(hashmap! { + "x-request".to_string() => vec!["request1, request2".to_string()] + }); + + let result = match_headers(expected, actual, &CoreMatchingContext::default()); + + assert!(result.values().all(Vec::is_empty), "{result:?}"); + } + + #[rstest( + case(&["request1", "request2"], &["request1,request2"], true), + case(&["request1", "request2"], &["request1 ,\t request2"], true), + case(&["request1", "request2"], &["request1", "request2"], true), + case(&["request1", "request2", "request3"], &["request1, request2", "request3"], true), + case(&["request1", "request2"], &["request1, wrong", "request2"], false), + case(&["request1", "request2"], &["request1", "wrong, request2"], false), + case(&["\"one,two\"", "three"], &["\"one,two\", three"], true), + case(&[r#""one\",two""#, "three"], &[r#""one\",two", three"#], true), + case(&[r#"{"foo":"bar,baz","arr":[1,2,3]}"#], &[r#"{"foo":"bar,baz","arr":[1,2,3]}"#], true), + case(&[r#"{"foo":"bar,baz","arr":[1,2,3]}"#, "tail"], &[r#"{"foo":"bar,baz","arr":[1,2,3]}, tail"#], true), + case(&["request1", "request2"], &["request1"], false), + case(&["request1", "request2"], &["request1, wrong"], false), + case(&["request1", "request2"], &["request2, request1"], false), + case(&["request1", "request2"], &["request1, request2, extra"], false), + case(&["request1", "request2"], &["\"request1, request2\""], false) + )] + fn match_headers_combined_custom_header_compatibility( + #[case] expected_values: &[&str], + #[case] actual_values: &[&str], + #[case] should_match: bool + ) { + let expected = Some(hashmap! { + "X-Request".to_string() => expected_values.iter().map(|value| value.to_string()).collect() + }); + let actual = Some(hashmap! { + "x-request".to_string() => actual_values.iter().map(|value| value.to_string()).collect() + }); + let result = match_headers(expected.clone(), actual.clone(), &CoreMatchingContext::default()); + assert_eq!(result.values().all(Vec::is_empty), should_match, + "expected={expected:?}, actual={actual:?}, result={result:?}"); + assert_eq!(expected.as_ref().unwrap()["X-Request"].len(), expected_values.len()); + assert_eq!(actual.as_ref().unwrap()["x-request"].len(), actual_values.len()); + } + + #[test] + fn match_headers_combined_custom_header_preserves_indexed_matchers() { + let rules = matchingrules! { + "header" => { + "$.x-request.0" => [ MatchingRule::Regex("^first-[0-9]+$".to_string()) ], + "$.x-request.1" => [ MatchingRule::Regex("^second-[0-9]+$".to_string()) ] + } + }; + let context = CoreMatchingContext { + matchers: rules.rules_for_category("header").unwrap(), + ..CoreMatchingContext::default() + }; + for (actual_value, should_match) in [("first-42, second-99", true), ("first-42, first-99", false)] { + let expected = Some(hashmap! { + "X-Request".to_string() => vec!["first-1".to_string(), "second-2".to_string()] + }); + let actual = Some(hashmap! { + "x-request".to_string() => vec![actual_value.to_string()] + }); + let result = match_headers(expected, actual, &context); + assert_eq!(result.values().all(Vec::is_empty), should_match, "{result:?}"); + } + } + + #[test] + fn match_headers_does_not_split_single_value_headers() { + for header in ["Set-Cookie", "Date", "User-Agent"] { + let expected = Some(hashmap! { + header.to_string() => vec!["one".to_string(), "two".to_string()] + }); + let actual = Some(hashmap! { + header.to_string() => vec!["one, two".to_string()] + }); + let result = match_headers(expected, actual, &CoreMatchingContext::default()); + assert!(!result.values().all(Vec::is_empty), "{header}: {result:?}"); + } + } + #[test] fn match_headers_returns_a_mismatch_if_the_header_values_have_different_lengths() { let expected = Some(hashmap!{ @@ -772,7 +894,7 @@ mod tests { }.rules_for_category("header").unwrap_or_default(), &hashmap!{} )); let expected = hashmap! { "X-Id".to_string() => vec!["1".to_string(), "3".to_string()] }; - let actual = hashmap! { "X-Id".to_string() => vec!["1".to_string(), "2".to_string(), "3".to_string(), "4".to_string()]}; + let actual = hashmap! { "X-Id".to_string() => vec!["1, 2".to_string(), "3, 4".to_string()]}; let result = match_headers(Some(expected), Some(actual), &context); expect!(result.values().flatten()).to(be_empty()); } @@ -811,13 +933,13 @@ mod tests { "header" => { "X-Id" => [ MatchingRule::EachValue(MatchingRuleDefinition::new("100".to_string(), ValueType::String, - MatchingRule::Regex("\\d+".to_string()), None, "".to_string())) + MatchingRule::Regex("^\\d+$".to_string()), None, "".to_string())) ] } }.rules_for_category("header").unwrap_or_default(), &hashmap!{} )); let expected = hashmap! { "X-Id".to_string() => vec!["1".to_string(), "2".to_string()] }; - let actual = hashmap! { "X-Id".to_string() => vec!["3".to_string(), "4".to_string(), "567".to_string()]}; + let actual = hashmap! { "X-Id".to_string() => vec!["3, 4".to_string(), "567".to_string()]}; let result = match_headers(Some(expected), Some(actual), &context); expect!(result.values().flatten()).to(be_empty()); }