fix(matching): match combined headers against repeated expectations

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<String>
+  ) -> 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,

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<T: HttpPart>(
 
         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<T: HttpPart>(
           } 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));
           }

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),

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<T>(map: &HashMap<String, T>, key: &str) -> Option<(String, T)> whe
   }
 }
 
+pub(crate) fn split_header_field_lines(values: &[String]) -> Vec<String> {
+  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<String, Vec<String>>,
   actual: HashMap<String, Vec<String>>,
@@ -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());
   }