-
Notifications
You must be signed in to change notification settings - Fork 854
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
Update options.changeOrigin description in docs #141
base: master
Are you sure you want to change the base?
Conversation
@chimurai Sure. Here it is. |
Great thanks! |
Shouldn't the option be renamed to |
Any news on this @chimurai ? This is particularly confusing as consuming projects assume it does change the Origin header, e.g. vitejs/vite#16557 (comment) @nicojs raises a good point but renaming it would be a breaking change and there may be projects that depend on the setting. I vote for merging this doc change so it is at least possible to understand the behaviour once you read the docs. |
Ah, http-proxy docs need updated first. I'll check that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
changeOrigin has nothing to do with
origin
header which is confusing and harmful when working with CORS. As CORS middleware sometimes makes decisions depending onorigin
header.changeOrigin
option handles solelyhost
header.