Skip to content

San 4260 headers#121

Merged
anandkumarpatel merged 16 commits intomasterfrom
SAN-4260-headers
May 18, 2016
Merged

San 4260 headers#121
anandkumarpatel merged 16 commits intomasterfrom
SAN-4260-headers

Conversation

@anandkumarpatel
Copy link
Copy Markdown
Contributor

@anandkumarpatel anandkumarpatel commented May 18, 2016 •
edited by bkendall
Loading

  • moved http => https hack to data-fetch
  • added headers
  • update reqUrl and reqParsedUrl if we are doing https=> http

dep

tests

  • master -> master
  • master -> branch
  • branch -> master
  • branch -> branch
  • isolation master -> isolation dep
  • isolation dependency -> isolation master
  • perf tested with blaze meter
  • https on port 443
  • https on url with only 80 exposed routes to 80
  • http on port 80 or 8080

review

@runnabot
Copy link
Copy Markdown

runnabot commented May 18, 2016 •
edited
Loading

The latest push to PR-121 has stopped. Check out the logs SAN-4260-headers-navi

@Myztiq
Copy link
Copy Markdown
Contributor

Myztiq commented May 18, 2016

Code LGTM!

bkendall
bkendall reviewed May 18, 2016
View reviewed changes
Comment thread lib/models/proxy.js
req.headers['x-forwarded-proto'] = proto;
req.headers['x-forwarded-port'] = req.isHttps ? '443' : req.parsedReqUrl.port;
log.trace({ headers: req.headers }, 'add x-forwarded headers');
});
Copy link
Copy Markdown
Contributor

@bkendall bkendall May 18, 2016

Choose a reason for hiding this comment

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

I think you should add one more: x-forwarded-host. then you should be good.

here's node-http-proxy's implementation of the same: https://github.com/nodejitsu/node-http-proxy/blob/master/lib/http-proxy/passes/web-incoming.js#L65

@bkendall
Copy link
Copy Markdown
Contributor

bkendall commented May 18, 2016

looking pretty good!

@bkendall
Copy link
Copy Markdown
Contributor

bkendall commented May 18, 2016

nice

@anandkumarpatel anandkumarpatel merged commit aef7c8a into master May 18, 2016
@anandkumarpatel anandkumarpatel deleted the SAN-4260-headers branch May 18, 2016 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@anandkumarpatel @runnabot @Myztiq @bkendall