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

fix: ignore not throw on invalid response headers #950

Conversation

pke
Copy link
Contributor

@pke pke commented Apr 14, 2021

@JakeChampion I think the fix is fine, however I struggle with the test. It runs into the same problem that the response headers are validated. But I can't figure out which package supplies this code. None of the node_modules has a _http_outgoing.js that I could monkey-patch for this one response to not validate the response headers.

I also can't just test parseHeaders() since its private and not globally exported (rightfully so).

So do you have any idea how to test my fix properly?

Fixes #930

@JakeChampion
Copy link
Owner

_http_outgoing.js is part of the built-in NodeJS http module, it is not possible to set a response header name in an invalid format with the built-in NodeJS http module.

The only way I can think to test this, would be to use another language to create a http server which can set a response header name in an invalid format. From the issue, we know that golang is able to do that.

I'm also happy to accept this pull-request without a test

@pke
Copy link
Contributor Author

pke commented May 1, 2021

I’m working on mocking xhr to Test this.

Repository owner deleted a comment Jun 22, 2021
@pke
Copy link
Contributor Author

pke commented Jun 23, 2021

What is going on here?
I am working on adding a test with mocked xhr and then put it in "Ready for review". So for the time being no more approvals needed.
And please github clean up the spam here, thanks ;)

Repository owner deleted a comment from S69y Jun 26, 2021
Repository owner deleted a comment Mar 22, 2022
Repository owner deleted a comment Mar 22, 2022
Copy link

@PandaSuits PandaSuits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviwed changes, merge commits

@JakeChampion JakeChampion marked this pull request as ready for review April 1, 2023 01:24
@JakeChampion JakeChampion force-pushed the bugfix/issue-930-normalizeName-should-not-throw branch from 9d60b33 to e74fa5b Compare April 1, 2023 01:24
@JakeChampion JakeChampion merged commit 9a6d748 into JakeChampion:master Apr 1, 2023
0 of 3 checks passed
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

Successfully merging this pull request may close these issues.

normalizeName should not throw