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

proxy changeOrigin set to true by default #4302

Closed
SavePointSam opened this issue Apr 13, 2018 · 8 comments
Closed

proxy changeOrigin set to true by default #4302

SavePointSam opened this issue Apr 13, 2018 · 8 comments

Comments

@SavePointSam
Copy link

Is this a bug report?

No

Expected Behavior

When configuring the proxy setting in package.json to a string, requests to should not changeOrigin as false is the default setting in node-http-proxy.

Actual Behavior

changeOrigin is enabled by default.

Potential Solutions

  1. Disable changeOrigin by default for simple string proxy configuration.
  2. Add documentation to the User Guide to alert users that this will occur and what it means.
@Timer
Copy link
Contributor

Timer commented Sep 26, 2018

Can you explain this more? What behavior does this turn on? Why is it bad?

@SavePointSam
Copy link
Author

I have a file server running tus-node-server sitting behind my application server that I proxy to for file uploads and downloads. Tus by default reads the origin header to dynamically generate a location to ask the client to send requests to going forward.

I had to spend some time debugging the problem I was having where my file server was sending back locations that were relative to my application server and not the client. While the fix is simple enough, it was not documented, nor is it the default behavior for the packages running the proxy under the hood.

I'm not sure why a system would want to change the default behavior seeing as this is a proxy. Wouldn't all consumers of the proxied request want to know the original origin by default? I don't know what value there is in masking it.

Ultimately, I would like to either have the CRA default match the underlying tooling defaults or at the very least have documentation helping advanced proxy users know that CRA alters the default behavior without them having to read the CRA source code.

@SavePointSam
Copy link
Author

Looks like this is no longer an issue as it appears the proxy key in package.json has been removed in v2 in favor of the more flexible proxySetup.js. If that is the case, this issue is no longer valid. Thanks!

@iansu iansu closed this as completed Sep 26, 2018
@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2018

There's still support for a "simple" proxy key, but indeed advanced configuration is now fully flexible.

@SavePointSam
Copy link
Author

It may be worthwhile to note this in the guide then. 🤷‍♂️

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2018

Added to #5127.

@gaearon
Copy link
Contributor

gaearon commented Sep 27, 2018

You're also welcome to PR :-)

@SavePointSam
Copy link
Author

I probably will! haha

@lock lock bot locked and limited conversation to collaborators Jan 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants