Skip to content

Fix/context path for send document - #539

Merged
rholshausen merged 3 commits into
masterfrom
fix/context-path-for-send-document
Jul 28, 2026
Merged

Fix/context path for send document#539
rholshausen merged 3 commits into
masterfrom
fix/context-path-for-send-document

Conversation

@YOU54F

@YOU54F YOU54F commented Jul 8, 2026

Copy link
Copy Markdown
Member

relates to #420

Tested locally with pact-broker-docker project running with docker compose up

Modified nginx.conf file

https://github.com/pact-foundation/pact-broker-docker/blob/master/ssl/nginx.conf

server {
  listen      443 ssl default_server;
  server_name localhost;
  ssl_certificate /etc/nginx/ssl/nginx-selfsigned.crt;
  ssl_certificate_key /etc/nginx/ssl/nginx-selfsigned.key;
  ssl_protocols TLSv1 TLSv1.1 TLSv1.2;
  ssl_prefer_server_ciphers on;
  ssl_ecdh_curve secp384r1;
  ssl_session_cache shared:SSL:10m;
  ssl_stapling on;
  ssl_stapling_verify on;

    location /pact/ {
      proxy_pass http://pact-broker:9292/;
      proxy_set_header Host $host;
      proxy_set_header X-Forwarded-Scheme "https";
      proxy_set_header X-Forwarded-Port "443";
      proxy_set_header X-Forwarded-Ssl "on";
      proxy_set_header X-Real-IP $remote_addr;
  }
}

server {
  listen      80 default_server;
  server_name localhost;

    location /pact/ {
      proxy_pass http://pact-broker:9292/;
      proxy_set_header Host $host;
      proxy_set_header X-Real-IP $remote_addr;
}
}

Modified docker compose to update PACT_BROKER_BASE_URL to

PACT_BROKER_BASE_URL: 'http://localhost/pact http://localhost/pact http://localhost:9292/pact http://pact-broker:9292/pact https://host.docker.internal/pact http://host.docker.internal/pact http://host.docker.internal:9292/pact'

Running the following command without this change will return

cargo run -- --broker-url http://localhost/pact --provider-name "Example API" --consumer-version-selectors '{"deployed":true}' --loglevel trace

output

2026-07-08T15:59:51.751140Z  INFO verifier: pact_verifier::pact_broker: error response for pacts for verification: Link/Resource was not found - Request to pact broker path '/pacts/provider/Example%20API/for-verification' failed: 404 Not Found. URL: 'http://localhost/pact/'
2026-07-08T15:59:51.751193Z ERROR verifier: pact_verifier: No pacts found for provider 'Example API' matching the given consumer version selectors in pact broker 'http://localhost/pact/': Link/Resource was not found - Request to pact broker path '/pacts/provider/Example%20API/for-verification' failed: 404 Not Found. URL: 'http://localhost/pact/'
2026-07-08T15:59:51.751239Z ERROR verifier: pact_verifier: Failed to load pact - \x1b[31mNo pacts found for provider 'Example API' matching the given consumer version selectors in pact broker 'http://localhost/pact/'\x1b[0m

after fix

output


The pact at http://localhost/pact/pacts/provider/Example%20API/consumer/Example%20App/pact-version/64cae7e32cafa53621693e516338df283a2160f1 is being verified because the pact content belongs to the consumer version matching the following criterion:
    * consumer version(s) currently deployed to production (e15da45d3943bf10793a6d04cfb9f5dabe430fe2)

Verifying a pact between Example App and Example API
...

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Pact Broker context-path handling in the Rust verifier’s HAL client so that broker links (including absolute hrefs) resolve correctly when the broker is hosted behind a reverse proxy under a subpath (e.g. /pact), addressing the “context path stripped” behavior described in #420.

Changes:

  • Added a normalize_path_from_url helper to consistently derive a request path from broker link URLs and apply the configured context path when missing.
  • Updated fetch_url and send_document to use the new normalization logic instead of relying on Url::join semantics that can drop the subpath.
  • Added tests intended to cover subpath + templated link navigation.

Comment thread rust/pact_verifier/src/pact_broker.rs Outdated
Comment thread rust/pact_verifier/src/pact_broker.rs
Comment thread rust/pact-core-mock-server Outdated
@rholshausen

Copy link
Copy Markdown
Contributor

There are two things that can be fixed:

  • resolve_path (pre-existing, unchanged) still does the naive path.starts_with(context_path) check without a boundary. Since normalize_path_from_url now always produces a context-prefixed path before calling resolve_path, this is currently masked — but it's a latent inconsistency in the same file the PR is touching. Worth hardening at the same time so the two functions agree on what "matches the context path" means (e.g. share a small helper), otherwise a future caller of resolve_path with a raw path could hit the same collision bug this PR just fixed elsewhere.
  • self.url.parse::<Url>() is now parsed twice per request path (fetch_url/send_document → normalize_path_from_url → resolve_path, each re-parsing self.url). Minor, non-hot-path inefficiency; could pass the already-parsed Url (or just the context path) through instead of re-parsing the string twice.

@YOU54F
YOU54F force-pushed the fix/context-path-for-send-document branch from bb80897 to e3d90c1 Compare July 9, 2026 11:13
@YOU54F
YOU54F requested a review from Copilot July 9, 2026 11:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comment thread rust/pact_verifier/src/pact_broker.rs Outdated
url_string.to_string()
};

let context_path = broker_url.path();
Comment thread rust/pact_verifier/src/pact_broker.rs Outdated
/// 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 = broker_url.path().to_string();
Comment on lines 412 to +416
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
…rve query strings

Addresses remaining review comments on PR #539:
- A broker URL with a trailing slash (e.g. http://host/pact/) produced a
  context path of "/pact/", causing normalize_path_inner and resolve_path_inner
  to emit double slashes ("/pact//...") when prepending/joining paths.
- normalize_path_inner extracted only the path from absolute broker link URLs,
  silently dropping any query string (e.g. pagination params).
- fetch() logged the same info!/trace! lines that fetch_with_url() already logs,
  duplicating log output on every broker fetch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rholshausen
rholshausen merged commit d0c1fd4 into master Jul 28, 2026
29 of 30 checks passed
@rholshausen
rholshausen deleted the fix/context-path-for-send-document branch July 28, 2026 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants