Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitespace removed from header include validation #1704

Open
dianaDBB opened this issue Jul 26, 2023 · 9 comments
Open

Whitespace removed from header include validation #1704

dianaDBB opened this issue Jul 26, 2023 · 9 comments

Comments

@dianaDBB
Copy link

Pact JVM version (maven):

  • group-id = au.com.dius.pact.provider
  • artifact-id = junit5
  • version-id = 4.3.11

In consumer (Pact JS) I add an expected result with MatchersV3.includes that has a whitespace:

.willRespondWith({
    status: 200,
    headers: {
        'content-disposition': MatchersV3.includes('attachment; filename='),
    },
});

This generates a contract with the following (it has the whitespace):

"response": {
  "headers": {
    "content-disposition": "attachment; filename=",
  },
  "matchingRules": {
    "header": {
      "content-disposition": {
        "combine": "AND",
        "matchers": [
          {
            "match": "include",
            "value": "attachment; filename="
          }
        ]
      }
    }
  },
  "status": 200
}

However, when provider (Pact JVM) verifies this contract, it fails with the following error - the whitespace was removed from the expected result

"mismatches": [
  {
    "attribute": "header",
    "description": "Expected header 'content-disposition' to have value 'attachment;filename=' but was 'attachment; filename=SomeFileNameHere'",
    "identifier": "content-disposition"
  }
]

This test should pass, the whitespace should not be removed from the validation.

@MZalfred
Copy link

The issue you're facing seems to be related to how the whitespace in the header value is handled during contract verification between the consumer (Pact JS) and the provider (Pact JVM). It looks like the whitespace is being removed from the expected result during the verification process, causing the test to fail.

The problem is likely due to a difference in how the two implementations (Pact JS and Pact JVM) handle the matching of headers, specifically when using the MatchersV3.includes with whitespace.

@MZalfred
Copy link

Option; You can use Different Matcher:
Instead of using MatchersV3.includes, you can use a different matcher that doesn't remove the whitespace. For example, you can use MatchersV3.regex to match the header value using a regular expression. This way, you have more control over how the header value is matched, and the whitespace won't be removed.
Example
.willRespondWith({
status: 200,
headers: {
'content-disposition': MatchersV3.regex('attachment; filename=.*'),
},
});

@rholshausen
Copy link
Contributor

You shouldn't rely on the whitespace, it is optional in these types of headers. Some HTTP clients and servers will include the white space, others will not.

@MZalfred
Copy link

MZalfred commented Aug 8, 2023

It seems like you’re referring to the whitespace in HTTP headers. In HTTP headers, whitespace after the colon (:) that separates the header name from its value is indeed optional. While many implementations include it for readability, it’s not a requirement according to the HTTP/1.1 specification (RFC 7230).
Ex
Host: meyou.com
and
Host:meyou.com
Both are valid

@rholshausen
Copy link
Contributor

Not only that, but headers that follow the ABNF form (like Content-Type, Accept and I assume Content-Disposition), the white space around parameters are also optional. So Content-Type: application/json; charset=UTF-8 and Content-Type: application/json;charset=UTF-8 are both valid

@dianaDBB
Copy link
Author

dianaDBB commented Aug 8, 2023

I agree that space is optional, so in that case, the Provider should NOT validate the space, right?

@rholshausen
Copy link
Contributor

Yes, you can use a regex instead to match any amount of whitespace, i.e. "attachment;\\s*filename="

@MZalfred
Copy link

MZalfred commented Aug 8, 2023

Or Suggested Update to Pact JS Code
'content-disposition': MatchersV3.includes('attachment; filename=')

latest versions of both Pact JS and Pact JVM. Sometimes updating can resolve these inconsistencies.

@avinash10993
Copy link

But we have a header which isnt actually following the ABNF form and when i update pact plugin to latest it is adding an extraspace after semicolon and this shouldnt be the case. Older version of pact 4.5.6 wasnt doing this but with 4.6.14 it was adding an extra whitespace and controller was failing these scenarios from provider verification standpoint.

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

No branches or pull requests

4 participants