pact-foundation / pact-js

JS version of Pact. Pact is a contract testing framework for HTTP APIs and non-HTTP asynchronous messaging systems.
https://pact.io
Other
1.63k stars 348 forks source link

Multi-value headers are mishandled #964

Closed gruntster closed 1 year ago

gruntster commented 2 years ago

Software versions

Issue Checklist

Please confirm the following:

Expected behaviour

A multi-value header is matched against the multi-value header of the registered Interaction (v2). Substituting the Pact library with version 9.18.1 works as expected.

Actual behaviour

A multi-value header of the actual request appears to be processed as a single value that is a combination of all values. A multi-value header of the expected request is processed as the first value only.

  Mock server failed with the following mismatches:

        0) The following request was incorrect:

                GET /dogs

                         1.0 Mismatch with header 'dog-protocol': Expected 'woofruff' to be equal to 'woof'

Steps to reproduce

package.json

{
  "dependencies": {
    "@pact-foundation/pact": "10.1.4",
    "got": "11.8.5",
    "ts-node": "10.9.1"
  }
}

test.ts

import { Interaction, Pact } from "@pact-foundation/pact";
import got from "got";
import * as path from "node:path";

async function test(): Promise<void> {
    const provider = new Pact({
        dir: path.resolve(process.cwd(), "pacts"),
        consumer: "MyConsumer",
        provider: "MyProvider",
        logLevel: "debug"
    });

    await provider.setup();

    await provider.addInteraction(
        new Interaction()
            .given("I have a list of dogs")
            .uponReceiving("a request for all dogs")
            .withRequest({
                method: "GET",
                path: "/dogs",
                headers: { "dog-protocol": "woof, ruff" }
            })
            .willRespondWith({ status: 200 }));

    try {
        await got.get(
            `${provider.mockService.baseUrl}/dogs`, {
                headers: { "dog-protocol": ["woof", "ruff"] },
                retry: 0
            });
    }
    catch (error) { }

    await provider.verify();
    await provider.finalize();
}

void test();

Steps

  1. npm i
  2. npx ts-node test.ts

Relevant log files

test.txt

mefellows commented 2 years ago

Thanks for this.

Maintainer notes:

This looks like bug on the JS side. The rust core supports multiple values for a header, but currently that is not exposed to the user.

It looks like on the response side of the check, rust is clever enough to split the header value parts and test each of them. But on the consumer side, it doesn't split the header value (e.g. via the presence of a , into its constituent parts) and this might be the cause of the issue.

gruntster commented 1 year ago

@mefellows This issue is still present in pact 10.4.0. Would it be possible for this to be reopened please?

mefellows commented 1 year ago

Thanks. Yes, I only added support to the V3 (PactV3) and (new beta PactV4) V4 interface, but we need to add it to the Pact.

UPDATE: this comment was incorrect, it was added to all types, however the bug is present in the core

github-actions[bot] commented 1 year ago

👋 Hi! The 'smartbear-supported' label has just been added to this issue, which will create an internal tracking ticket in PactFlow's Jira (PACT-848). We will use this to prioritise and assign a team member to this task. All activity will be public on this ticket. For now, sit tight and we'll update this ticket once we have more information on the next steps.

See our documentation for more information.

mefellows commented 1 year ago

After further investigation, the upstream issue is actually https://github.com/pact-foundation/pact-reference/issues/300.

mefellows commented 1 year ago

The latest Pact JS (via pact-core dependency) should address this. Closing.