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

Debate about whether headers are included in proxied request #1276

Closed
the1mills opened this issue May 15, 2018 · 7 comments
Closed

Debate about whether headers are included in proxied request #1276

the1mills opened this issue May 15, 2018 · 7 comments

Comments

@the1mills
Copy link

the1mills commented May 15, 2018

I am on the latest Node.js versions v9, v10, etc. I have a simple server which proxies the npm registry:

import http = require('http');

const s = http.createServer(function (clientRequest, clientResponse) {

  const proxy = http.request({
      hostname: 'registry.npmjs.org',
      port: 80,
      path: clientRequest.url,
      method: clientRequest.method
    },
    function (res) {
      res.pipe(clientResponse);
    });

   // do the clientRequest headers get written to the proxy request??
  clientRequest.pipe(proxy);

});

s.listen(3441);

I have a debate raging here about whether the original request headers are included in the request that gets sent to NPM:

https://stackoverflow.com/questions/50342072/why-doesnt-node-js-proxy-request-need-headers-included?noredirect=1#comment87711085_50342072

@the1mills
Copy link
Author

So yeah my question is - are the original headers included or not?

@bnoordhuis
Copy link
Member

No, they're not.

@ORESoftware
Copy link

ORESoftware commented May 15, 2018

Yeah ok, makes sense, to include them, just do this?

  // first write headers
  Object.keys(headers).forEach(function(k){
      proxy.write(`${k}\t${headers[k]}\r\n`);
  });
  
  // one more newline I think
  proxy.write('\n');

  // then write body
  clientRequest.pipe(proxy);

@bnoordhuis
Copy link
Member

No, that creates an outgoing request with headers in the request body.

Add them as the .headers option to http.request() but scrub them first if you don't want to get pwned. Ditto for .path and other values that you copy from the incoming request.

Take a look at https://github.com/nodejitsu/node-http-proxy.

@ORESoftware
Copy link

Yeah I tried adding the headers to the options arg, but I got some JSON parsing errors which I thought was very strange.

@gireeshpunathil
Copy link
Member

Guessing that this issue is resolved. /cc @ORESoftware and @the1mills (by this time I also have guessed you two are same)

@gireeshpunathil
Copy link
Member

closing as addressed, please let me know if that is not the 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

No branches or pull requests

4 participants