Skip to content

Preserve header case for incoming requests.#1108

Open
mixxen wants to merge 1 commit intohttp-party:masterfrom
mixxen:preserve-case-incoming
Open

Preserve header case for incoming requests.#1108
mixxen wants to merge 1 commit intohttp-party:masterfrom
mixxen:preserve-case-incoming

Conversation

@mixxen
Copy link
Copy Markdown

@mixxen mixxen commented Dec 13, 2016

Comment thread lib/http-proxy/common.js
var rawHeaders = {};
for (var i = 0; i < req.rawHeaders.length - 1; i += 2) {
var key = req.rawHeaders[i];
rawHeaders[key] = req.rawHeaders[i + 1];
Copy link
Copy Markdown
Contributor

@pachirel pachirel Dec 16, 2016

Choose a reason for hiding this comment

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

Learning from #1104, I prefer to use req.headers[key] instead.
I didn't check the behavior of incoming, though.

@pachirel
Copy link
Copy Markdown
Contributor

I'm welcome this PR, because I didn't care about incoming request.
But I'm not a maintainer, please take their review ;)

IMO, Existing specs seems not to cover additional behavior. Would you please add specs?

@honzajavorek
Copy link
Copy Markdown

I think we've just bumped into some problems related to this. Is there a way we can help to get this merged and released?

@jarrodek
Copy link
Copy Markdown

It's worth to mention that the http message spec describes headers as a case insensitive. Therefore clients/servers that relays on headers cases sensitivity are not complying with the spec and should not be expected to work properly.
Developers shouldn't get used to bad practices (especially if something is against the spec) and therefore this PR shouldn't be accepted.
That's only my opinion.

@mixxen
Copy link
Copy Markdown
Author

mixxen commented Jul 14, 2017

@jarrodek The issue here is dealing with 3rd party software or legacy software that does not adhere to the spec and we have no control over.

@honzajavorek
Copy link
Copy Markdown

The same with us. I think node-http-proxy preserving case of header names would be something similar to "be benevolent in what you accept and strict about what you produce". I totally agree it's ridiculous some software is HTTP header name case-sensitive, but software is made by humans and humans make mistakes.

This is very simple change node-http-proxy can do and it even actually makes sort of sense - IMHO the proxy should pass things "as is" to be as unnoticeable as possible, unless it really needs to touch them and modify them for some purpose.

@jarrodek
Copy link
Copy Markdown

I totally agree with @honzajavorek The proxy should be totally transparent for passing data and not alter them. In this case, however, it doesn't actually changes the data. If anything, that's the problem of Node's Request object and not the proxy. This PR actually introduce a behavior where data are altered by the proxy. This PR should be sent to Node because that's the source of the problem and not this module.

@mrichmond
Copy link
Copy Markdown

mrichmond commented Sep 13, 2018

Just ran into this problem where for whatever reason the service I'm calling expects a header to be Mixed Case ('X-HTTP-Method-Override'). Even with the preserveHeaderKeyCase: true it looks like the header is being rewritten to ('x-http-method-override'). I'm able to work around this by rewriting the headers... but it's a pretty lame solution since I'd have to do it for each header and I may not now what the original case was. Why wouldn't it just leave the header casing alone in the first place??

const onProxyReq = (proxyReq, req) => {
  if (req.headers['x-http-method-override']) {
    proxyReq.setHeader('X-HTTP-Method-Override', req.headers['x-http-method-override']);
  }
};

@ItsEcholot
Copy link
Copy Markdown

Just ran into this same issue as @mrichmond described... Why are headers rewritten to lower-case?

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.

6 participants