XMLHttpRequest getResponseHeader case sensitivity
#89
Conversation
| @@ -422,13 +422,7 @@ export const createXMLHttpRequestOverride = ( | |||
| const headerValue = Object.entries(this.responseHeaders).reduce< | |||
| string | null | |||
| >((_, [headerName, headerValue]) => { | |||
| // Ignore header name casing while still allowing to set response headers | |||
| // with an arbitrary casing (no normalization). | |||
There was a problem hiding this comment.
This gives me pause - it looks like it was a conscious decision not to normalise case so I might be missing something here. The comment mentions "set"ing response headers but this function is read only, so I don't see the harm in forcing all header names to lowercase for comparison?
There was a problem hiding this comment.
I think you're correct: header names must be case-insensitive when retrieving a header. We forgot to normalize the input name to ensure that.
There was a problem hiding this comment.
Thank you for spotting and fixing this! Tremendous appreciation for adding a test covering the regression ❤️
Welcome to contributors, @kalopilato! 🎉
kettanaito
force-pushed
the
xhr-get-response-header-case-sensitivity
branch
from
48ce657
to
e48e391
Compare
|
@kalopilato could you please set the right email for your commits? They don't seem to be recognized as yours on GitHub. This often happens if your email in Git doesn't match the email on GitHub. You deserve your contribution to be associated with you. |
kalopilato
force-pushed
the
xhr-get-response-header-case-sensitivity
branch
from
e48e391
to
6ed70f7
Compare
|
Thanks @kettanaito!! And thanks for the heads up about my git config, have updated and amended those commits. Pleased to have finally made my first open source contribution 😄 |
|
We are happy to see you contributing, especially with this superb finding. Thank you! If you're interested in request interception in Node, take a look through the code of this package, I'm sure it may need improvement. We also have a few issues we need help with in the MSW repository. |
kettanaito
force-pushed
the
xhr-get-response-header-case-sensitivity
branch
from
6ed70f7
to
d62b3ed
Compare
I came across a problem when using MSW in testing: the component under test has a dependency that makes API calls (authentication) and inspects the "Content-Type" response header (note the casing), but the value of this header was always null. I traced this to the XMLHttpRequestOverride's
getResponseHeaderfunction which evaluated "Content-Type" (the passed in header name) against "content-type" (the actual header name in the response), thereby failing and returning null.This PR updates the XHR
getResponseHeaderfunction to normalise the passed in header name and the response headers to lowercase when evaluating equality. This looks to be similar behaviour to jsdom's implementation and aligns with HTTP spec.