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

Failed to convert header keys to lower case due to field name conflict: accept-encoding #126

Closed
oyvindeh opened this issue Aug 30, 2016 · 6 comments
Labels

Comments

@oyvindeh
Copy link

I get this error when I use express-http-proxy >= 0.9.1:

Failed to convert header keys to lower case due to field name conflict: accept-encoding
Error: Failed to convert header keys to lower case due to field name conflict: accept-encoding
    at foo/node_modules/nock/lib/common.js:210:13
    at foo/node_modules/lodash/lodash.js:4332:15
    at baseForOwn (foo/node_modules/lodash/lodash.js:2731:24)
    at Function.forOwn (foo/node_modules/lodash/lodash.js:12042:24)
    at Object.headersFieldNamesToLowerCase (foo/node_modules/nock/lib/common.js:207:5)
    at RequestOverrider (foo/node_modules/nock/lib/request_overrider.js:111:30)
    at new OverriddenClientRequest (foo/node_modules/nock/lib/intercept.js:240:23)
    at foo/node_modules/nock/lib/intercept.js:363:13
    at Object.module.request (foo/node_modules/nock/lib/common.js:137:14)
    at runProxy (foo/node_modules/express-http-proxy/index.js:87:43)
    at invokeCallback (foo/node_modules/raw-body/index.js:262:16)
    at done (foo/node_modules/raw-body/index.js:251:7)
    at IncomingMessage.onEnd (foo/node_modules/raw-body/index.js:307:7)
    at emitNone (events.js:86:13)
    at IncomingMessage.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:975:12)

I see the release notes for 0.9.1 says "Set 'Accept-Encoding' header to match bodyEncoding". Perhaps something goes wrong in relation to this?

@monkpow monkpow added the bug label Aug 30, 2016
@monkpow
Copy link
Collaborator

monkpow commented Aug 30, 2016

Sounds like it. Can you give me a code snippet that triggers this issue, and I'll fix the bug. Thanks!

@btmurrell
Copy link

btmurrell commented Sep 4, 2016

@monkpow , @oyvindeh I'm hitting this issue too. I cannot tell you how to reproduce it -- express (or possibly helmet which I'm using) is loading up my req.headers by some default behavior... however, i do see where the problem in express-http-proxy is happening.

This may be a suitable fix for express-http-proxy index.js line ~83:

      if (bodyEncoding(options)) {
        var acceptEncodingHeader = reqOpt.headers[ 'Accept-Encoding'.toLowerCase() ];
        if (acceptEncodingHeader) {
          reqOpt.headers[ 'Accept-Encoding'.toLowerCase() ] = acceptEncodingHeader + ', ' + bodyEncoding(options);
        } else {
          reqOpt.headers[ 'Accept-Encoding' ] = bodyEncoding(options);
        }
      }

Edit: That is short-sighted; does not take into account unique values in this header. Various ways to do this; lodash would be my toolkit, but it does not look like you are including it.

@monkpow
Copy link
Collaborator

monkpow commented Sep 7, 2016

@oyvindeh @btmurrell I'm tentative about fixing without being able to see the problem in action. If your hunch is right, you should be able to skip this chunk of code by adding

reqBodyEnconding: null

to your options.

@oyvindeh
Copy link
Author

oyvindeh commented Sep 8, 2016

I'm sorry, I haven't had time to write a snippet that triggers the issue. Hopefully, I'll be able to do so next week.

@oyvindeh
Copy link
Author

oyvindeh commented Sep 16, 2016

I have problems reproducing this outside of the environment where I get the error (and I cannot share the full code). But I can confirm that reqBodyEncoding: null does indeed work around the issue.

@monkpow
Copy link
Collaborator

monkpow commented Oct 26, 2016

@oyvindeh I did find a relevant bug in the code, which is patch here 861b620. This fix is in the most recent version (0.10.1) published to npm.

Let me know if you continue to have issues. Thanks for the report and clarifications so far.

@monkpow monkpow closed this as completed Oct 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants