WithCustomHeader Function does not replace existing headers

[RChild0916]

Hello all!

I'm working with pact-net as a provider using pactflow which has some bi-directional testing, and ran into an interesting issue.
We recently added authentication to our API, and needed to add an auth header using the .WithCustomHeader method like so:

`

        IPactVerifier pactVerifier = new PactVerifier(config);
        pactVerifier
            .ServiceProvider("App.Api", new Uri(_baseUrl))
            .WithPactBrokerSource(
                new Uri(_providerUrl),
                options => options
                    .TokenAuthentication(_pactflowToken)
                    .PublishResults(
                        _providerVersion,
                        options => options.ProviderTags(_providerTags)
                            .ProviderBranch(_providerBranch)))
            .WithProviderStateUrl(new Uri(_fixture.ServerUri, "/provider-states"))
            .WithCustomHeader("Authorization", $"Bearer {tokenValue}")
            .Verify();

`

The "WithCustomHeader" function seems to have an issue, which I haven't been able to discern exactly but believe it is one of two things:

1. If a header already exists within the consumer pact json, it does not replace the value as it should.
2. It is only adding an additional header, and in this case, we end up with 2 "Authorization" headers
   Either way, the end result is the same in that our API grabs the old token that exists within the pact file and attempts to use it, sees it is expired and throws a 401.

To get this working, we removed the Authorization header from the pact file and everything ran clean from the provider side, but this lead to a new issue within pactflow. Pactflow will check the request headers against our API's OAS, and sees that the authorization header is missing and fails for the consumer side check.

Is it possible to get new functionality to allow users to replace existing headers?

[YOU54F]

I would assume normal usage would be to use a combination request filters / state handlers to check the existing token matches a particular format and then provide a valid token in the request to be verified. see 4 https://docs.pact.io/provider/handling_auth#4-modify-the-request-to-use-real-credentials

withCustomHeader without looking at the code, I expect the use case would be to add a new header, rather than change an existing.

When you say you get two headers, are the different in casing?

Are you able to provide any logs?

[RChild0916]

I would assume normal usage would be to use a combination request filters / state handlers to check the existing token matches a particular format and then provide a valid token in the request to be verified. see 4 https://docs.pact.io/provider/handling_auth#4-modify-the-request-to-use-real-credentials

Yeah, this is what I am doing in the above setup by adding a new, valid token. There is the option of creating a new middleware within the API itself to check for a specific value before it hits the auth middleware, but I would ideally like to avoid this.

withCustomHeader without looking at the code, I expect the use case would be to add a new header, rather than change an existing.

When you say you get two headers, are the different in casing?

Again, I'm unsure of the exact result, but I'd wager you are correct that their are two different headers with the request.

Are you able to provide any logs?
If it is needed I can try to get some logs but this would take some time to prune company data so it is shareable.
If you are curious about whether or not it is adding a second header, I can look into this. What else are you hoping to see from the logs?

[YOU54F]

So it does indeed just add it (in the core), without considering others, which I assume is by design. If it is the case, maybe we need to warn, or error to the user, in the core, unless we do want to allow us to overwrite an existing error.

this is the user facing side

pact-net/src/PactNet/Verifier/PactVerifierSource.cs

Lines 97 to 101 in 9eb58b3

	public IPactVerifierSource WithCustomHeader(string name, string value)
	{
	this.provider.AddCustomHeader(name, value);
	return this;
	}

this is the native interop, so this is calling our rust code

pact-net/src/PactNet/Verifier/InteropVerifierProvider.cs

Lines 129 to 130 in 9eb58b3

	/// <returns>Fluent builder</returns>
	public void AddCustomHeader(string name, string value)

ffi code for pactffi_verifier_add_custom_header

https://github.com/pact-foundation/pact-reference/blob/46628a8bf6b4d514adcb317d749fb8a79130b472/rust/pact_ffi/src/verifier/mod.rs#L395-L413

which calls add_custom_header to insert some custom_headers

https://github.com/pact-foundation/pact-reference/blob/46628a8bf6b4d514adcb317d749fb8a79130b472/rust/pact_ffi/src/verifier/handle.rs#L334

Which I think gets setup here

https://github.com/pact-foundation/pact-reference/blob/46628a8bf6b4d514adcb317d749fb8a79130b472/rust/pact_verifier/src/lib.rs#L659-L677

which believe to be processed here

https://github.com/pact-foundation/pact-reference/blob/46628a8bf6b4d514adcb317d749fb8a79130b472/rust/pact_ffi/src/verifier/verifier.rs#L210-L219

We may want to consider

What should happen if

• the header doesnt exist
• the header does exist
• the header does exist but in a different case
• the header exists, but doesn't match the format (Wrong format for a bearer token)

Why would you want to add a custom header, without considering state

[YOU54F]

Are you able to provide any logs?
If it is needed I can try to get some logs but this would take some time to prune company data so it is shareable.
If you are curious about whether or not it is adding a second header, I can look into this. What else are you hoping to see from the logs?

A minimal reproducible example

You can create an example from the code in pact-net, or our example-consumer-dotnet / example-provider-dotnet repos

I want to confirm if there is indeed two headers, I can't imagine you would have two headers with the same casing, you should get one overwritten, however its possible there might be an authorization and Authorization header

[YOU54F]

I'm working with pact-net as a provider using pactflow which has some bi-directional testin

Just curious, are you using bi-directional, as well as consumer driven? So you have a provider OAS to verify against, and then also verifying a running provider with traditional consumer driven Pact?

Just as there traditionally wouldn't be a verification task as you show, if you use bi-directional only

[RChild0916]

Just curious, are you using bi-directional, as well as consumer driven? So you have a provider OAS to verify against, and then also verifying a running provider with traditional consumer driven Pact?

Well in this case, we are the provider, but yes, we are also using the consumer's pact json and running it against our API as the provider.

A minimal reproducible example

You can create an example from the code in pact-net, or our example-consumer-dotnet / example-provider-dotnet repos

This would take me some time to do, I'll see if I can get to this today or tomorrow. In the meantime, I ran some tests locally and can provide the requests /responses I see in Fiddler. Overall, it appears as if .WithCustomHeader does nothing if the header already exists, as I'm not seeing a duplicate header (see fiddler response below).

Our setup:
IPactVerifier pactVerifier = new PactVerifier(config); pactVerifier .ServiceProvider("App.Api", new Uri(_baseurl)) .WithPactBrokerSource( new Uri(_pacturl), options => options .TokenAuthentication(_pactflowToken) .PublishResults( _providerVersion, options => options.ProviderTags(_providerTags) .ProviderBranch(_providerBranch))) .WithProviderStateUrl(new Uri(_fixture.ServerUri, "/provider-states")) .WithCustomHeader("authorization", $"Bearer {tokenValue}") .Verify();

The consumer pact (removed some data that isn't needed for this discussion):
{ "consumer": { "name": "Consumer" }, "provider": { "name": "App.Api" }, "interactions": [ { "description": "a request to create an object", "providerStates": [ ], "request": { "method": "POST", "path": "/test", "headers": { "authorization": "Bearer Test" }, "body": { [Removed for github] }, "matchingRules": { "$.body": { "match": "type" } } }, "response": { "status": 201, "headers": { "Content-Type": "[Removed for github]" }, "body": { [Removed for github] }, "matchingRules": { "$.body": { "match": "type" } } } } ], "metadata": { "pactSpecification": { "version": "3.0.0" } } }

Fiddler Raw request:
POST http://localhost:35000/test HTTP/1.1 authorization: Bearer Test accept-encoding: gzip, deflate host: localhost:35000 content-length: 172

Response didn't give much detail but threw a 500 because the JWT wasn't parseable (since it wasn't replaced and is just 'Test')

[YOU54F]

Cheers buddy! That is useful, I'm not a pact-net maintainer, nor a pact-rust maintainer but do use the pact ffi library, so the information you've provided is useful.

Something else we or others could do, is add test cases here

pact-net/tests/PactNet.Tests/Verifier/PactVerifierSourceTests.cs

Lines 71 to 78 in 9eb58b3

	[Fact]
	public void WithCustomHeader_WhenCalled_AddsCustomHeader()
	{
	this.verifier.WithCustomHeader("Authorization", "Bearer abcdef0123456789");
	this.verifier.Verify();
	this.mockProvider.Verify(p => p.AddCustomHeader("Authorization", "Bearer abcdef0123456789"));
	}

This uses a mock provider so under if it actually calls the native core, I'm going to a couple of tests with the ffi directly :)

[YOU54F]

This commit

pact-foundation/pact-reference@cd83d3e

shows the issue with php, directly using the pact_ffi libarary.

• consumer 1 - provides a header called testy-mc-header
• consumer 2- doesn't provide a header
• provider calls pactffi_verifier_add_custom_header($handle, 'testy-mc-header', 'some-other-header')
  • consumer 1, retains the header 'testy-mc-header', 'some-header'
  • consumer 2, has a new header 'testy-mc-header', 'some-other-header'

Code examples below.

This will probably want transferring to the pact-reference repository and discussing with @uglyog as to whether this is expected behaviour or a bug

consumer 1

Array
(
    [testy-mc-header] => Array
        (
            [0] => some-header
        )

    [content-type] => Array
        (
            [0] => application/json
        )

    [accept] => Array
        (
            [0] => */*
        )

    [accept-encoding] => Array
        (
            [0] => gzip, deflate
        )

    [host] => Array
        (
            [0] => localhost
        )

    [content-length] => Array
        (
            [0] => 315
        )

)
[Wed May 10 17:24:01 2023] 127.0.0.1:50965 [201]: POST /api/books

consumer 2's headers

Array
(
    [content-type] => Array
        (
            [0] => application/json
        )

    [accept] => Array
        (
            [0] => */*
        )

    [testy-mc-header] => Array
        (
            [0] => some-other-header
        )

    [accept-encoding] => Array
        (
            [0] => gzip, deflate
        )

    [host] => Array
        (
            [0] => localhost
        )

    [content-length] => Array
        (
            [0] => 2
        )

)
[Wed May 10 17:33:31 2023] 127.0.0.1:51142 [204]: PUT /api/books/fb5a885f-f7e8-4a50-950f-c1a64a94d500/generate-cover

[adamrodger]

Yeah this is a problem in the FFI rather than PactNet itself. It'll need a tracking issue

[RChild0916]

Yeah this is a problem in the FFI rather than PactNet itself. It'll need a tracking issue

Okay, should I create that? If so, what repo should that go under?

[YOU54F]

Yeah this is a problem in the FFI rather than PactNet itself. It'll need a tracking issue

Okay, should I create that? If so, what repo should that go under?

Hey @RChild0916 sorry, we should have said (or done it for you), if you could raise the issue against https://github.com/pact-foundation/pact-reference/ and mention this issue that would be great