[V3] Cannot match response headers having comma separated values (Provider Contract Testing)

saw-jan commented 3 years ago

Steps to Reproduce

Create interaction with response that has comma-separated values.
Generate pact interaction (Consumer side test)
Run Provider side test.

Expected behaviour

running following interaction for provider test

const aGETInfoInteraction = (provider) => {
    return provider
      .uponReceiving('a GET request')
      .withRequest({
        method: 'GET',
        path: '/api',
        headers: client.basicAuth,
      })
      .willRespondWith({
        status: 200,
        headers: {
          'content-type': 'application/json',
          'Access-Control-Expose-Headers': 'Content-Length, Content-Type, Expires',
        },
      });
  };

should match the response headers and pass the provider test

Actual behaviour

gives following error:

1) Verifying a pact between client and server - a GET request returns a response which 
    1.1) includes header 'Access-Control-Expose-Headers' with value 'Expires'
           Expected header 'Access-Control-Expose-Headers' to have value 'Expires' but was ''

Relevant log files

 RUNS  tests/providerTest.js
[2021-04-27T09:51:15Z DEBUG pact_js_v3] Initialising Pact native library version 0.0.6
[2021-04-27T09:51:15Z DEBUG pact_js_v3::verify] pacts = [File("/mnt/sda1/www/pactv3/tests/pacts/client-server.json")]
[2021-04-27T09:51:15Z DEBUG pact_js_v3::verify] provider_info = ProviderInfo { name: "server", protocol: "http", host: "localhost", port: Some(9000), path: "/" }
[2021-04-27T09:51:15Z DEBUG pact_js_v3::verify] request_filter done
[2021-04-27T09:51:15Z WARN  pact_js_v3::verify] publishVerificationResult must be a boolean value. Ignoring it
[2021-04-27T09:51:15Z DEBUG pact_js_v3::verify] Starting background task
[2021-04-27T09:51:15Z DEBUG pact_js_v3::verify] Done
Verifying a pact between client and serverrify] Background verification task started
[2021-04-27T09:51:15Z INFO  pact_verifier] Running provider verification for 'a GET request'
[2021-04-27T09:51:15Z INFO  pact_verifier::provider_client] Sending request to provider at http://localhost:9000/
[2021-04-27T09:51:15Z DEBUG pact_verifier::provider_client] Provider details = ProviderInfo { name: "server", protocol: "http", host: "localhost", port: Some(9000), path: "/" }
[2021-04-27T09:51:15Z DEBUG pact_verifier::provider_client] Sending request Request ( method: GET, path: /api, query: None, headers: Some({"Authorization": ["Basic YWRtaW46YWRtaW4="]}), body: Missing )

 RUNS  tests/providerTest.js
[2021-04-27T09:51:15Z DEBUG reqwest::connect] starting new connection: http://localhost:9000/
[2021-04-27T09:51:15Z DEBUG reqwest::async_impl::client] response '200 OK' for http://localhost:9000/api
[2021-04-27T09:51:15Z DEBUG pact_verifier::provider_client] Received native response: Response { url: Url { scheme: "http", host: Some(Domain("localhost")), port: Some(9000), path: "/api", query: None, fragment: None }, status: 200, headers: {"x-powered-by": "Express", "content-type": "application/json; charset=utf-8", "access-control-expose-headers": "Content-Length, Content-Type, Expires", "content-length": "8", "etag": "W/\"8-DsbRUFSXgCUKl3LAa2GbzEag5WA\"", "date": "Tue, 27 Apr 2021 09:51:15 GMT", "connection": "keep-alive", "keep-alive": "timeout=5"} }
[2021-04-27T09:51:15Z INFO  pact_verifier::provider_client] Received response: Response ( status: 200, headers: Some({"access-control-expose-headers": ["Content-Length, Content-Type, Expires"], "content-length": ["8"], "content-type": ["application/json; charset=utf-8"], "keep-alive": ["timeout=5"], "x-powered-by": ["Express"], "etag": ["W/\"8-DsbRUFSXgCUKl3LAa2GbzEag5WA\""], "connection": ["keep-alive"], "date": ["Tue, 27 Apr 2021 09:51:15 GMT"]}), body: Present(8 bytes, application/json;charset=utf-8) )
[2021-04-27T09:51:15Z INFO  pact_matching] comparing to expected response: Response ( status: 200, headers: Some({"content-type": ["application/json"], "Access-Control-Expose-Headers": ["Content-Length", "Content-Type", "Expires"]}), body: Missing )
[2021-04-27T09:51:15Z DEBUG pact_matching] expected content type = 'application/json', actual content type = 'application/json;charset=utf-8'
[2021-04-27T09:51:15Z DEBUG pact_matching] content type header matcher = 'None'
[2021-04-27T09:51:15Z DEBUG pact_matching::matchers] String -> String: comparing 'Content-Length' to 'Content-LengthContent-TypeExpires' using Equality
[2021-04-27T09:51:15Z DEBUG pact_matching::matchers] String -> String: comparing 'Content-Type' to '' using Equality
[2021-04-27T09:51:15Z DEBUG pact_matching::matchers] String -> String: comparing 'Expires' to '' using Equality
  a GET request
    returns a response which
      has status code 200 (OK)
      includes headers
        "content-type" with value "application/json" (OK)
        "Access-Control-Expose-Headers" with value "Content-Length, Content-Type, Expires" (FAILED)
      has a matching body (OK)

Failures:

1) Verifying a pact between client and server - a GET request returns a response which 
    1.1) includes header 'Access-Control-Expose-Headers' with value 'Expires'
               Expected header 'Access-Control-Expose-Headers' to have value 'Expires' but was ''

Software versions

OS: Ubuntu 20.04
Consumer Pact library: 10.0.0-beta.34
Provider Pact library:
Node Version: v12.20.0

Issue Checklist

Please confirm the following:

[x] I have upgraded to the latest
[x] I have the read the FAQs in the Readme
[x] I have triple checked, that there are no unhandled promises in my code
[x] I have set my log level to debug and attached a log file showing the complete request/response cycle
[ ] For bonus points and virtual high fives, I have created a reproduceable git repository (see below) to illustrate the problem

mefellows commented 3 years ago

Thanks for this, sounds like a bug indeed. It almost looks like it's taking the header string, and converted it into a list of items.

tonynguyenit18 commented 3 years ago

@mefellows I would love to give this a try, is it ok? @saw-jan How do you config response header in provider? Would you mind to attach the code that you config it? And the pact of the interaction too if it's possible.

mefellows commented 3 years ago

Thanks Tony, yes of course - would very much appreciate any help. I'll be around tomorrow for support if needed.

tonynguyenit18 commented 3 years ago

@mefellows I am trying to reproduce the issue, but I couldn't. What do you normal do to reproduce the issue?

What I have done is.

Setup simple Consumer and Provider
Install "@pact-foundation/pact": "^10.0.0-beta.34", for both
In stall Node v12.20.0

Add simple interaction in consumer having

const aGETInfoInteraction = (provider) => {
return provider
  .uponReceiving('a GET request')
  .withRequest({
    method: 'GET',
    path: '/api',
    headers: client.basicAuth,
  })
  .willRespondWith({
    status: 200,
    headers: {
      'content-type': 'application/json',
      'Access-Control-Expose-Headers': 'Content-Length, Content-Type, Expires',
    },
  });
};

Then I have the pact expect response with

{
"status": 200,
"headers": {
"content-type": "application/json",
"Access-Control-Expose-Headers": "Content-Length, Content-Type, Expires"
}
}

Add provider to fulfil /api endpoint with. I am waiting for @saw-jan to provide example of provider so I try with
```
...
res.header("Access-Control-Expose-Headers", 'Content-Length, Content-Type, Expires')
...
```
OR
```
...
res.append("Access-Control-Expose-Headers","Content-Length")
res.append("Access-Control-Expose-Headers"," Content-Type")
res.append("Access-Control-Expose-Headers"," Expires")
...
```
But both work fine (pass test) to me The only thing I don't setup is running it in Ubuntu 20.04, I ran the test in Mac OS. Do we really need to use Ubuntu 20.04 to reproduce the issue? How do you do normally? Do you use Docker for it?

mefellows commented 3 years ago

Thanks @tonynguyenit18!

Do we really need to use Ubuntu 20.04 to reproduce the issue? How do you do normally? Do you use Docker for it?

There is some responsibility on the OP to provide a reproducible example. We only have so much time, so unless there is some obvious defect we will generally rely on an example that reliably reproduces the problem.

[2021-04-27T09:51:15Z DEBUG pact_matching::matchers] String -> String: comparing 'Content-Length' to 'Content-LengthContent-TypeExpires' using Equality

This is the line that looks like a bug on our side, but it looks like we're going to need more data to help.

@saw-jan could you please help create a minimal example that illuminates the issue?

saw-jan commented 3 years ago

@mefellows I would love to give this a try, is it ok? @saw-jan How do you config response header in provider? Would you mind to attach the code that you config it? And the pact of the interaction too if it's possible.

@tonynguyenit18 Sorry, I should have mentioned clearly that this issue is not with Consumer testing but with Provider testing

tonynguyenit18 commented 3 years ago

@mefellows I would love to give this a try, is it ok? @saw-jan How do you config response header in provider? Would you mind to attach the code that you config it? And the pact of the interaction too if it's possible.

@tonynguyenit18 Sorry, I should have mentioned clearly that this issue is not with Consumer testing but with Provider testing

Yep. I got it @saw-jan , but before doing Provider testing you need to have pact published to broker by running Consumer testing.

Would you please provider that contract?
Would you please provide Provider configuration in Provider side AND how you set up Provider api response header (If applicable) too?

saw-jan commented 3 years ago

@tonynguyenit18 As per your request, here are the codes that I have implemented.

 const aGETInfoInteraction = (provider) => {
    return provider
      .uponReceiving('a GET request')
      .withRequest({
        method: 'GET',
        path: '/api',
        headers: client.basicAuth,
      })
      .willRespondWith({
        status: 200,
        headers: {
          'content-type': 'application/json',
          'Access-Control-Expose-Headers':
            'Content-Length, Content-Type, Expires',
        },
      });
  };

  it('get info', () => {
    aGETInfoInteraction(provider);

    return provider.executeTest(async () => {
      return client
        .getInfo()
        .then((res) => {})
        .catch((err) => {});
    });
  });

Generated Interaction:

 "interactions": [
    {
      "description": "a GET request",
      "request": {
        "headers": {
          "Authorization": "Basic YWRtaW46YWRtaW4="
        },
        "method": "GET",
        "path": "/api"
      },
      "response": {
        "headers": {
          "Access-Control-Expose-Headers": "Content-Length, Content-Type, Expires",
          "content-type": "application/json"
        },
        "status": 200
      }
    }
  ]

In Server:

 app.get('/api', (req, res) => {
  res.set('Content-Type', 'application/json');
  res.set(
    'Access-Control-Expose-Headers',
    'Content-Length, Content-Type, Expires'
  );
  res.status(200).send('response');
});

Steps I have used:

Consumer Test (Generates pact)
```
yarn jest -- tests/headerTest.js
```
Provider Test
```
yarn jest -- tests/providerTest.js
```

tonynguyenit18 commented 3 years ago

@saw-jan I can not reproduce the issue, it works all fine. It's much appreciated if you can create simple repo version of the issue and push to Github which I can pull and run to visualise the issue.

saw-jan commented 3 years ago

@tonynguyenit18 I have created the repo https://github.com/saw-jan/pactv3-issues Maybe you could find a bug in my code. Also it would be better if you could provide your repo so that I can test them in my system.

tonynguyenit18 commented 3 years ago

@saw-jan your repo working fine to me. I am sung MacOs Catalina Version 10.15.7.

// I just update test file name to pact.test.js, not a big deal

tests/providerTest.pact.test.js
Verifying a pact between client and server
  a GET request
    returns a response which
      has status code 200 (OK)
      includes headers
        "Access-Control-Expose-Headers" with value "Content-Length, Content-Type, Expires" (OK)
        "content-type" with value "application/json" (OK)
      has a matching body (OK)
 PASS  tests/providerTest.pact.test.js
  Pact Verification
    ✓ verifies the provider (18 ms)

  console.log
    Pact Verification Complete!

      at tests/providerTest.pact.test.js:24:17

  console.log
    Result: true

      at tests/providerTest.pact.test.js:25:17

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.132 s, estimated 2 s
Ran all test suites.
✨  Done in 1.70s.

tonynguyenit18 commented 3 years ago

@saw-jan I can reproduce it now. Actually it works fine, if we use

res.append('Access-Control-Expose-Headers', [
    'Content-Length',
    'Content-Type',
    'Expires',
  ])

It will throw the error if we use any other way to set header. e.g

 res.set('Access-Control-Expose-Headers','Content-Length, Content-Type, Expires');
 // OR
 res.setHeader('Access-Control-Expose-Headers','Content-Length, Content-Type, Expires');

tonynguyenit18 commented 3 years ago

@mefellows I found the issue. Simply, I print in this function https://github.com/pact-foundation/pact-reference/blob/master/rust/pact_matching/src/headers.rs#L109.

I for actual response headers

{
    "content-type": [
        "application/json; charset=utf-8"
    ],
    "connection": [
        "keep-alive"
    ],
    "access-control-expose-headers": [
        "Content-Length",
        "Content-Type",
        "Expires"
    ],
    "date": [
        "Sat, 29 May 2021 08:27:23 GMT"
    ],
    "x-powered-by": [
        "Express"
    ],
    "content-length": [
        "0"
    ]
}

And expected response headers

{
    "content-type": [
        "application/json"
    ],
    "Access-Control-Expose-Headers": [
        "Content-Length",
        "Content-Type",
        "Expires"
    ]
}

Compare expected headers and header in contract

  ....
      "response": {
        "headers": {
          "Access-Control-Expose-Headers": "Content-Length, Content-Type, Expires",
          "content-type": "application/json"
        },
        "status": 200
      }
  ....

===> headers in contract is split to be expected (https://github.com/pact-foundation/pact-reference/blob/master/rust/pact_matching/src/models/mod.rs#L261) response headers. BUT actual response header is not

When we use

res.append('Access-Control-Expose-Headers', [
    'Content-Length',
    'Content-Type',
    'Expires',
  ])

==> We got response headers is ['Content-Length', 'Content-Type', 'Expires'] (3 string items) => WORK FINE

When we use

 res.set('Access-Control-Expose-Headers','Content-Length, Content-Type, Expires');
 // OR
 res.setHeader('Access-Control-Expose-Headers','Content-Length, Content-Type, Expires');

==> We got response headers is ['Content-Length, Content-Type, Expires] (1 String item) and this is not split => FAILED

My approach is split headers of actual response too.

mefellows commented 3 years ago

Awesome work Tony! I'll look into releasing the update into Pact JS next week.

individual-it commented 3 years ago

some unit tests added in https://github.com/pact-foundation/pact-reference/pull/111

saw-jan commented 2 years ago

Issue has been fixed, Closing here

pact-foundation / pact-js