Fix/context path for send document

rust/pact_verifier/src/pact_broker.rs

@@ -69,6 +69,84 @@ fn find_entry(map: &serde_json::Map<String, Value>, key: &str) -> Option<(String
     }
 }
 
+/// Returns `true` when `path` equals `context_path` or starts with `context_path/`.
+///
+/// This prevents a broker context path such as `/pact` from falsely matching an
+/// unrelated path like `/pactfoo/bar` via a naive `starts_with` check.
+fn has_context_prefix(path: &str, context_path: &str) -> bool {
+    path == context_path || path.starts_with(&format!("{}/", context_path))
+}
+
+/// Strips a single trailing `/` from `context_path`, leaving the root path `/` as is.
+///
+/// A broker URL configured with a trailing slash (e.g. `http://host/pact/`) would
+/// otherwise yield a context path of `/pact/`, producing double slashes (`/pact//...`)
+/// wherever the context path is prepended or joined with another path.
+fn trim_context_path(context_path: &str) -> &str {
+    if context_path == "/" {
+        context_path
+    } else {
+        context_path.trim_end_matches('/')
+    }
+}
+
+/// Extracts the path component from `url_string` (stripping scheme/host if it is an
+/// absolute URL) and prepends `broker_url`'s context path when it is missing.
+///
+/// Accepts an already-parsed `broker_url` so callers that have parsed it for
+/// another purpose need not parse it a second time.
+fn normalize_path_inner(url_string: &str, broker_url: &Url) -> String {
+    let mut path = if let Ok(parsed_url) = url_string.parse::<Url>() {
+        // Absolute URL: extract the path (plus query, if any), so we keep the broker's host.
+        match parsed_url.query() {
+            Some(query) => format!("{}?{}", parsed_url.path(), query),
+            None => parsed_url.path().to_string()
+        }
+    } else {
+        // Already a path (relative or absolute).
+        url_string.to_string()
+    };
+
+    let context_path = trim_context_path(broker_url.path());
+    if !context_path.is_empty() && context_path != "/" && path.starts_with('/') {
+        if !has_context_prefix(&path, context_path) {
+            debug!("Prepending context path '{}' to path '{}'", context_path, path);
+            path = format!("{}{}", context_path, path);
+        }
+    }
+    path
+}
+
+/// Resolves a normalised `path` against an already-parsed `broker_url`,
+/// respecting any context path embedded in the broker URL.
+///
+/// Accepts ownership of `broker_url` so callers that have already parsed it
+/// can pass it through without a second `parse::<Url>()` call.
+fn resolve_path_inner(path: &str, broker_url: Url) -> Result<Url, PactBrokerError> {
+    let context_path = trim_context_path(broker_url.path()).to_string();
+    let url = if path.is_empty() {
+        broker_url
+    } else if !context_path.is_empty() && context_path != "/" {
+        if has_context_prefix(path, &context_path) {
+            let mut base_url = broker_url;
+            base_url.set_path("/");
+            base_url.join(path)?
+        } else if path.starts_with('/') {
+            let mut base_url = broker_url;
+            base_url.set_path(path);
+            base_url
+        } else {
+            let mut base_url = broker_url;
+            let cp = format!("{}/", context_path);
+            base_url.set_path(&cp);
+            base_url.join(path)?
+        }
+    } else {
+        broker_url.join(path)?
+    };
+    Ok(url)
+}
+
 /// Errors that can occur with a Pact Broker
 #[derive(Debug, Clone, thiserror::Error)]
 pub enum PactBrokerError {
@@ -340,16 +418,22 @@ impl HALClient {
         ))
     }?;
 
-    let base_url = self.url.parse::<Url>()?;
-    let joined_url = base_url.join(&link_url)?;
-    self.fetch(joined_url.path().into()).await
+    // Parse broker URL once and use both inner functions to avoid double-parsing.
+    let broker_url = self.url.parse::<Url>()?;
+    let path = normalize_path_inner(&link_url, &broker_url);
+    let url = resolve_path_inner(&path, broker_url)?;
+    self.fetch_with_url(&path, url).await
   }
 
   async fn fetch(&self, path: &str) -> Result<Value, PactBrokerError> {
-    info!("Fetching path '{}' from pact broker", path);
-    trace!(%path, broker_url = %self.url, ">> fetch");
-
     let url = self.resolve_path(path)?;
+    self.fetch_with_url(path, url).await
+  }
+
+  /// Performs the actual GET request using an already-resolved `url`.
+  async fn fetch_with_url(&self, path: &str, url: Url) -> Result<Value, PactBrokerError> {
+    info!("Fetching path '{}' from pact broker", path);
+    trace!(%path, broker_url = %self.url, ">> fetch_with_url");
     debug!("Final broker URL: {}", url);
 
     let request_builder = match self.auth {
@@ -364,41 +448,18 @@ impl HALClient {
     let response = with_retries(self.retries, request_builder).await
       .map_err(|err| {
           PactBrokerError::IoError(format!("Failed to access pact broker path '{}' - {}. URL: '{}'",
-              &path,
+              path,
               err,
               &self.url,
           ))
       })?;
 
-    self.parse_broker_response(path.to_string(), response)
-        .await
+    self.parse_broker_response(path.to_string(), response).await
   }
 
   fn resolve_path(&self, path: &str) -> Result<Url, PactBrokerError> {
     let broker_url = self.url.parse::<Url>()?;
-    let context_path = broker_url.path();
-    let url = if path.is_empty() {
-      broker_url
-    } else if !context_path.is_empty() && context_path != "/" {
-      if path.starts_with(context_path) {
-        let mut base_url = broker_url.clone();
-        base_url.set_path("/");
-        base_url.join(path)?
-      } else if path.starts_with("/") {
-        let mut base_url = broker_url.clone();
-        base_url.set_path(path);
-        base_url
-      } else {
-        let mut base_url = broker_url.clone();
-        let mut cp = context_path.to_string();
-        cp.push('/');
-        base_url.set_path(cp.as_str());
-        base_url.join(path)?
-      }
-    } else {
-      broker_url.join(path)?
-    };
-    Ok(url)
+    resolve_path_inner(path, broker_url)
   }
 
   async fn parse_broker_response(
@@ -528,13 +589,10 @@ impl HALClient {
   async fn send_document(&self, url: &str, body: &str, method: Method) -> Result<Value, PactBrokerError> {
     debug!("Sending JSON to {} using {}: {}", url, method, body);
 
-    let base_url = &self.url.parse::<Url>()?;
-    let url = if url.starts_with("/") {
-      base_url.join(url)?
-    } else {
-      let url = url.parse::<Url>()?;
-      base_url.join(&url.path())?
-    };
+    // Parse broker URL once and use inner functions for both steps.
+    let broker_url = self.url.parse::<Url>()?;
+    let path = normalize_path_inner(url, &broker_url);
+    let url = resolve_path_inner(&path, broker_url)?;
 
     let request_builder = match self.auth {
       Some(ref auth) => match auth {
@@ -2677,6 +2735,150 @@ mod tests {
     expect!(result.path_info).to(be_some().value(serde_json::Value::String("Yay! You found your way here".to_string())));
   }
 
+
+  #[test]
+  fn resolve_path_with_context_prefixed_path() {
+    // Path already contains the context prefix — resolve_path must not double-prepend it.
+    let client = HALClientBuilder::builder()
+      .with_url("http://127.0.0.1:8080/pact", None)
+      .build();
+
+    let path = "/pact/pacts/provider/Example%20API/for-verification";
+    let resolved = client.resolve_path(path).expect("Should resolve path");
+
+    expect!(resolved.path()).to(be_equal_to("/pact/pacts/provider/Example%20API/for-verification"));
+  }
+
+  #[test]
+  fn resolve_path_does_not_match_context_prefix_without_boundary() {
+    // "/pactfoo/bar" starts with "/pact" lexically but must NOT be treated as
+    // already having context path "/pact".
+    let client = HALClientBuilder::builder()
+      .with_url("http://127.0.0.1:8080/pact", None)
+      .build();
+
+    let path = "/pactfoo/bar";
+    let resolved = client.resolve_path(path).expect("Should resolve path");
+
+    // The path does not start with /pact/, so it is treated as an absolute path
+    // without a context prefix and set directly.
+    expect!(resolved.path()).to(be_equal_to("/pactfoo/bar"));
+  }
+
+  #[test]
+  fn normalize_path_from_url_prepends_context_for_absolute_url_missing_context() {
+    // Broker lives at http://host/pact but the link contains an absolute URL
+    // from a different host that omits the context path.  normalize_path_inner
+    // must strip the foreign host and prepend the broker's context path.
+    let broker_url = "http://127.0.0.1:8080/pact".parse::<Url>().unwrap();
+    let absolute_link = "http://other-broker.example.com/pacts/provider/Example%20API/for-verification";
+
+    let normalized = normalize_path_inner(absolute_link, &broker_url);
+
+    expect!(normalized).to(be_equal_to("/pact/pacts/provider/Example%20API/for-verification"));
+  }
+
+  #[test]
+  fn normalize_path_from_url_does_not_double_prepend_context_path() {
+    // When the absolute URL already carries the correct context path the
+    // function must leave it unchanged.
+    let broker_url = "http://127.0.0.1:8080/pact".parse::<Url>().unwrap();
+    let absolute_link = "http://other-broker.example.com/pact/pacts/provider/Example%20API/latest";
+
+    let normalized = normalize_path_inner(absolute_link, &broker_url);
+
+    expect!(normalized).to(be_equal_to("/pact/pacts/provider/Example%20API/latest"));
+  }
+
+  #[test]
+  fn normalize_path_from_url_drops_query_string() {
+    // Copilot review comment: normalize_path_inner extracts only `parsed_url.path()`
+    // from an absolute URL, so any query string on a broker-provided href (e.g.
+    // pagination or filter params) is silently discarded.
+    let broker_url = "http://127.0.0.1:8080/pact".parse::<Url>().unwrap();
+    let absolute_link = "http://other-broker.example.com/pact/pacts/provider/Example%20API?page=2&size=10";
+
+    let normalized = normalize_path_inner(absolute_link, &broker_url);
+
+    expect!(normalized).to(be_equal_to("/pact/pacts/provider/Example%20API?page=2&size=10"));
+  }
+
+  #[test]
+  fn normalize_path_from_url_context_with_trailing_slash_does_not_double_slash() {
+    // Copilot review comment: if the broker URL has a trailing slash (e.g.
+    // "http://host/pact/"), context_path is "/pact/" and the naive
+    // `format!("{}{}", context_path, path)` prepend produces "/pact//...".
+    let broker_url = "http://127.0.0.1:8080/pact/".parse::<Url>().unwrap();
+    let absolute_link = "http://other-broker.example.com/pacts/provider/Example%20API/for-verification";
+
+    let normalized = normalize_path_inner(absolute_link, &broker_url);
+
+    expect!(normalized).to(be_equal_to("/pact/pacts/provider/Example%20API/for-verification"));
+  }
+
+  #[test]
+  fn resolve_path_context_with_trailing_slash_does_not_double_slash() {
+    // Copilot review comment: resolve_path_inner also takes broker_url.path()
+    // verbatim, so a trailing-slash context path ("/pact/") makes
+    // `format!("{}/", context_path)` produce "/pact//", which can 404 on
+    // brokers/proxies that don't normalize repeated slashes.
+    let client = HALClientBuilder::builder()
+      .with_url("http://127.0.0.1:8080/pact/", None)
+      .build();
+
+    let path = "pacts/provider/Example%20API/for-verification";
+    let resolved = client.resolve_path(path).expect("Should resolve path");
+
+    expect!(resolved.path()).to(be_equal_to("/pact/pacts/provider/Example%20API/for-verification"));
+  }
+
+  #[test_log::test(tokio::test)]
+  async fn subpath_broker_templates_are_substituted() {
+    let pact_broker = PactBuilderAsync::new("RustPactVerifier", "TemplateTest")
+      .interaction("fetch root", "", |mut i| async move {
+        i.request.path("/pact");
+        i.response
+          .header("Content-Type", "application/hal+json")
+          .json_body(json_pattern!({
+            "_links": {
+              "pb:test": {
+                "href": "http://localhost/pact/test/{id}",
+                "templated": true
+              }
+            }
+          }));
+        i
+      })
+      .await
+      .interaction("fetch templated", "", |mut i| async move {
+        i.request.path("/pact/test/123");
+        i.response
+          .header("Content-Type", "application/json")
+          .json_body(json_pattern!("success"));
+        i
+      })
+      .await
+      .start_mock_server(None, None);
+
+    let base_url = pact_broker.url().to_string();
+    let broker_url = if base_url.ends_with('/') {
+      format!("{}pact", base_url)
+    } else {
+      format!("{}/pact", base_url)
+    };
+
+    let client = HALClientBuilder::builder()
+      .with_url(broker_url, None)
+      .build();
+
+    // Navigate to templated link
+    let mut template_vals = HashMap::new();
+    template_vals.insert("id".to_string(), "123".to_string());
+
+    let client_after = client.navigate("pb:test", &template_vals).await.unwrap();
+    expect!(client_after.path_info).to(be_some());
+  }
+
   #[test_log::test(tokio::test)]
   async fn navigate_takes_context_paths_into_account() {
     let pact_broker = PactBuilderAsync::new("RustPactVerifier", "PactBrokerStub")