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

Behavior of header matching should treat field names as case-insensitive, but not values #1360

Closed
scottashipp opened this issue Nov 3, 2020 · 13 comments

Comments

@scottashipp
Copy link

Overview

First, the default behavior of Karate is incorrect in how it handles the matching of header field names. It treats them in a case-sensitive manner, which is incorrect, as will be explained below.

Second, the documentation is (perhaps inadvertently) misleading in that it suggests that both header field names and header field values are treated case-insensitive according to the HTTP spec.

Problem 1 Detailed

For Karate to correctly follow the HTTP Spec for headers, it should have default behavior as follows:

  • When using match header <field-name> == // etc.. the field name in question should be found no matter what case it is supplied in.

For example, whether or not the user enters vary or Vary here, and whether or not it is returned in the response as vary or Vary, or even as vArY, it should match.

Problem 1 Explanation

The HTTP spec specifies that:

Field names are case-insensitive.

It explicitly does not specify the same for field values, thus indicating that field values should be treated in a case-sensitive manner.

For further proof of this, consider that the curl command lowercases all header field names by default, but not the field values.

In issue 504, the same issue was raised, but the decision appears to have been to introduce a secondary feature that can be toggled on to configure lower-casing of both header field names and header field values prior to when matching occurs. That is fine as an additional feature since it is documented, but that solution also does not meet the spec because it lowercases both the header field names and header field values.

The bottom line is that Karate does not adhere to the HTTP spec in either the default configuration or when lowerCaseResponseHeaders has been set to true.

Problem 1 Proposed solution

By default, Karate should match header field names in a case-insensitive way.

Problem 2 Detailed

The README documentation incorrectly states the following:

Since as per the HTTP spec, headers are case-insensitive you may want to switch on this setting: * configure lowerCaseResponseHeaders = true - if you need to perform a lot of assertions on the responseHeaders.

Problem 2 Explanation

The README statement is incorrect. Headers are not case-insenstive according to the spec. Field names are.

Problem 2 Proposed Solution

The README should be reworded to something like the following:

Since as per the HTTP spec, header field names are case-insensitive . . .

Zip repro

The attached zip file contains a minimal, complete, and verifiable example. The scenario therein should succeed if Karate's default behavior correctly reflected the HTTP spec because the value in the response from the fake JSON service (at least at the time of this writing) is as follows:

Vary: Origin, Accept-Encoding

Which means that the And match header vary == 'Origin, Accept-Encoding' assertion should succeed. Currently it fails.

@scottashipp
Copy link
Author

myproject.zip

@ptrthomas
Copy link
Member

ptrthomas commented Nov 4, 2020

@scottashipp thanks for the details:

  • would be much easier if you provide a PR with what exactly the docs should be (docs are hard ;)
  • initially yes we lowercased the header values as well, but not in the latest version
  • this test shows how you can switch on the lower-casing of header names and it works as expected: https://github.com/intuit/karate/blob/master/karate-demo/src/test/java/demo/headers/content-type.feature
  • again, would be super convenient if you provide a PR (ideally with a new test in that same folder) to show the issues
  • maybe it is good to have that the lower-casing is "on by default" not a deal-breaker IMO
  • I'll keep this ticket open, just that without a PR it may take time to look at

@ptrthomas
Copy link
Member

@scottashipp it would be great if you can try the rewrite branch and confirm that this is fixed: https://github.com/intuit/karate/wiki/1.0-upgrade-guide

@scottashipp
Copy link
Author

Great I will verify today!

@scottashipp
Copy link
Author

I have confirmed that the rewrite branch fixes this issue. Thank you @ptrthomas !

@ilamn
Copy link

ilamn commented Feb 8, 2021

@ptrthomas will this fix be part of next release?

@ptrthomas
Copy link
Member

@ilamn yes, you can try this now in 0.9.9.RC3 - please read https://github.com/intuit/karate/wiki/1.0-upgrade-guide

@ilamn
Copy link

ilamn commented Feb 8, 2021

Thanks. I tried in 0.9.9.RC3 . still fails for me.
have modified the above test slightly.

myproject-header.zip

And def cookie = responseHeaders['Set-Cookie'][0]
works

And def cookie = responseHeaders['set-cookie'][0]
fails

org.opentest4j.AssertionFailedError:
>>>> js failed:
01: responseHeaders['set-cookie'][0]
<<<<
org.graalvm.polyglot.PolyglotException: TypeError: Cannot read property "0" from undefined
- <js>.:program(Unknown)

classpath:examples/users/users.feature:12

@ptrthomas
Copy link
Member

ptrthomas commented Feb 8, 2021

@ilamn before I look at the zip file may I mention:

so please re-confirm

@ilamn
Copy link

ilamn commented Feb 8, 2021

yes @ptrthomas I confirm

  • I am not using match
  • i tried * configure lowerCaseResponseHeaders = true: did not help.
  • my use case at work is, i need to get a customer header and assign it to a variable. so that i can pass it in request header to next api call. and not cookies. I just used cookie as an header example to demonstrate. The problem for me is, the header case keeps changing for some reason (yet to deep dive, initial suspect is some infa layers before the api server). so I want the tests to ignore the case of the header name

Thanks a lot for your time.

@ptrthomas
Copy link
Member

@ilamn awesome, indeed our miss. would be great if you can build from source and confirm: https://github.com/intuit/karate/wiki/Developer-Guide

@ilamn
Copy link

ilamn commented Feb 8, 2021

Thanks a lot for the quick turnaround @ptrthomas . can confirm it works now. is there any further RC planned ? would be nice if there is one, so that I can use the fix at my work.

@ptrthomas
Copy link
Member

@ilamn yes I can confirm the last RC4 will go out this week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants