pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
91 stars 46 forks source link

Regex matching not applied on headers during verification #238

Closed mefellows closed 6 months ago

mefellows commented 1 year ago

See https://github.com/pact-foundation/pact-net/issues/420 + https://github.com/pact-foundation/pact-net/pull/433.

mefellows commented 1 year ago

I'm running into this also in the Pact JS project (see https://github.com/pact-foundation/pact-js/actions/runs/3763217001/jobs/6396572544#step:4:1563).

It seems to only affect linux and possible Windows, but not Mac OSX, as it is not failing locally for me, and is only failing on CI). I've also reproduced it locally in Docker.

rholshausen commented 1 year ago

/jira ticket

github-actions[bot] commented 1 year ago

👋 Thanks, Jira [PACT-513] ticket created.

rholshausen commented 1 year ago

Looks like the issue is that the matching rule has a path of $.Authorization[0] while the verifier is looking for $.Authorization. While this is technically correct (and a defect in the verifier), it is a bit odd. It means the rule is being applied to the first value only in a multi-value header. I.e., with Authorization: A, B it will only be applied to A and not B. So while this is OK for the Authorization header as they tend to only have single values, it may be a problem for headers that do have multiple values.

The path should be either $.Authorization or $.Authorization.* unless you only want it to only apply to the first header value and not any others.

mefellows commented 1 year ago

Ah, it makes sense. My guess is because the matching rule path is set at the time the header value is added (via the pactffi_with_header_v2 method, which takes an indexed value which may containing the matching rule JSON), it will set the matching rule on the indexed value instead of the whole header.

What's the best way to address this in the client libraries do you think?

rholshausen commented 1 year ago

Let's fix the actual problem first. We can then maybe make the pactffi_with_header_v2 function better.

rholshausen commented 6 months ago

I don't think this is an issue anymore