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

Adds changeOrigin flag to http-server proxy. #2615

Merged
merged 3 commits into from
Dec 3, 2014

Conversation

typeoneerror
Copy link
Contributor

After some poking around in the ProxyServerAddon and adding back the host param as the third parameter in here:

app.use(function(req, res) {
  return proxy.web(req, res);
});

changed to this:

app.use(function(req, res) {
  return proxy.web(req, res, {
    headers: { host: urlOpts.host }
  });
});

I was able to get the proxy working with a subdomain again.

I started poking around in node-http-proxy and found some earlier issues related to the "Host" header. http-proxy does not seem to change the Host header for outgoing requests, so you have to flag changeOrigin to make this happen.

I added changeOrigin and this immediately fixes my bugs. So, I'm not sure if this is the correct implementation (perhaps this should be an option sent to the proxy via cmd line or a ./server proxy), but I figured I'd push it so you can see where I am at. Hope it helps.

Again, this fixes my issues. Unsure if this will break everyone else's proxies! This is a bit outside of my node knowledge, which is minimal.

References:

After some poking around in the ProxyServerAddon and adding back the `host` param as the third parameter in here:

    app.use(function(req, res) {
      return proxy.web(req, res);
    });

I started poking around in node-http-proxy and found some earlier issues related to the "Host" header. http-proxy does not seem to change the Host header for outgoing requests, so you have to flag `changeOrigin` to make this happen.

I added changeOrigin and this immediately fixes my bugs. So, I'm not sure if this is the correct implementation (perhaps this should be an option sent to the proxy via cmd line or a ./server proxy), but I figured I'd push it so you can see where I am at. Hope it helps.

References:

* [Use changeOrigin for proxyRequest](http-party/node-http-proxy@cee3e2f)
* [Original host not being passed through](http-party/node-http-proxy#621)
* [Host header comments](http-party/node-http-proxy#150 (comment))
@stefanpenner
Copy link
Contributor

can I request a test?

@typeoneerror
Copy link
Contributor Author

@stefanpenner sure. I don't really know testing all that well, but it's something I need to learn, so I'll look into it. Looks like express-server-test.js is a good start?

@typeoneerror
Copy link
Contributor Author

cc @acdx in case you have any work done with testing this or thoughts on how I should do it :)

@abuiles
Copy link
Member

abuiles commented Dec 3, 2014

@stefanpenner I just checked this and this fixed the issue.

@rwjblue do you think it would be worth doing a bug release version soonish to solve this? this is a major blocker and will affect anyone getting started with ember-cli.

@abuiles
Copy link
Member

abuiles commented Dec 3, 2014

@typeoneerror are you on irc? the following on express-server-test https://gist.github.com/abuiles/cad2d1067f07de5eff06 will do :) (it fails without your changes) it might fit well around line 228

@rwjblue
Copy link
Member

rwjblue commented Dec 3, 2014

@abuiles I am absolutely fine releasing 0.1.4 once this lands.

@abuiles
Copy link
Member

abuiles commented Dec 3, 2014

@stefanpenner @rwjblue just added some tests for this I confirm that test will fail without changeOrigin: true that will help us catch regressions. I'll merge once it is green.

@abuiles abuiles force-pushed the ember-cli-http-proxy-2596 branch from 1455f62 to c0840f5 Compare December 3, 2014 22:59
@stefanpenner
Copy link
Contributor

+1

abuiles added a commit that referenced this pull request Dec 3, 2014
Adds changeOrigin flag to http-server proxy.
@abuiles abuiles merged commit bba1315 into ember-cli:master Dec 3, 2014
@abuiles abuiles deleted the ember-cli-http-proxy-2596 branch December 3, 2014 23:13
@shunchu
Copy link

shunchu commented Dec 3, 2014

abuiles added a commit that referenced this pull request Dec 3, 2014
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.

5 participants