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

Update parseHeaders to match node http behavior #558

Closed
wants to merge 1 commit into from

Conversation

Jessidhia
Copy link

Node ignores duplicate entries for certain HTTP headers.

It also always converts the set-cookie header into an array.

See https://nodejs.org/api/http.html#http_message_headers

Node ignores duplicate entries for certain HTTP headers.

It also always converts the `set-cookie` header into an array.
'expires', 'from', 'host', 'if-modified-since', 'if-unmodified-since',
'last-modified', 'location', 'max-forwards', 'proxy-authorization',
'referer', 'retry-after', 'user-agent'
];
Copy link
Author

Choose a reason for hiding this comment

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

I'm not entirely sure this is needed, as almost all of these headers are only seen in requests, not responses, but it's probably better to match node's behavior.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 93.516% when pulling 4f54471 on Kovensky:patch-1 into 4976816 on mzabriskie:master.

@Jessidhia
Copy link
Author

Looks like this also would fix #465, but without affecting other headers.

@rubennorte
Copy link
Member

Looks good. Could you please update the tests for this?

@Jessidhia
Copy link
Author

Jessidhia commented Apr 11, 2017

@rubennorte where could I find the appropriate tests? This should only run in a browser...

@tybro0103
Copy link
Contributor

@rubennorte @Kovensky I added the tests: #874

@rubennorte
Copy link
Member

Closing in favor of #874.

@Kovensky thanks for your contribution.

@rubennorte rubennorte closed this Aug 12, 2017
@axios axios locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants