fix(proxy): rewrite the origin header to match the target for ws proxy

[johnhunter]

Description

fixes #16557

For ws proxying, this PR updates the request Origin header to match the target Host value. This resolves an issue where proxying to an RFC 6455 compliant server causes a 403 to be returned when request Origin and Host do not match.

I didn't see any existing tests for the proxy module but can look at adding coverage if its a blocker to merging.

Update:

The PR also updates docs to clarify the actual effect of the changeOrigin option. See http-party/node-http-proxy#1130 which was raised in 2017 and has not been merged due to ongoing discussion about whether the option should be removed altogether. I believe clarifying this for Vite users will help them avoid assuming changeOrigin:true means change the origin header.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[johnhunter]

Moving to draft while I investigate why http-proxy isn't doing the chnage.

[sapphi-red]

Thank you for the PR and digging down what changeOrigin option does! I agree it'd great if Vite sets the origin header OOTB. But I'm afraid of breaking existing setups by setting it by default.

So first, I'd like to ask some questions about that.

1. Do you think there's a case where users want to skip setting the origin header?
2. Do you know why http-proxy sets host header instead of origin header?
3. I wonder if we should set origin header for non-ws requests as well. I guess it's useful for servers with origin header checks for CSRF prevention. Do you think there's any concerns about doing it?

[johnhunter]

Thanks @sapphi-red I'll look into it.

[johnhunter]

@sapphi-red excellent points, answers below:

1. Do you think there's a case where users want to skip setting the origin header?

I can't think of a case for skipping - certainly not with vite proxy as it is only used in local dev / preview environments. There may be other proxy/reverse-proxy use cases that would be relevant to the http-proxy project. I think it should be switched with the existing changeOrigin option, which will hopefully become clear...

2. Do you know why http-proxy sets host header instead of origin header?

I looked in to this - they originally set both Host and Origin headers when the changeOrigin option was true. The option was removed entirely in a refactor and then reimplemented but with only the Host change. I've asked if that was an intentional change node-http-proxy#1669 but I suspect it was an unintended regression.

I'll look at raising a PR on http-proxy to re-instate the original (and preferred) behaviour. However I am not confident that maintainers have the bandwidth to review, merge and release such a change.

Therefore what I will do is update this vite PR to honour the original http-proxy behaviour and change the Origin header when changeOrigin is true.

3. I wonder if we should set origin header for non-ws requests as well. I guess it's useful for servers with origin header checks for CSRF prevention. Do you think there's any concerns about doing it?

Given that http-proxy originally applied the change to http requests as well I think it would make sense to do the same.

While updating the changeOrigin behaviour to change Origin as well as Host is a change to vite proxy, I think it is more in line with vite users expectations than the current situation. If changing the Origin header is an issue for vite users then I would expect they already had issues with the current Host header change and so they would have set changeOrigin:false.

I'll set the PR back to draft until I've made the outlined changes Ready for review.

[sapphi-red]

Thanks for checking!

We can introduce a different option like changeOrigin: 'both' to be really safe. But I think we can try this out without that in the next minor beta version. If it breaks, let's try adding an option.

[johnhunter]

We can introduce a different option like changeOrigin: 'both' to be really safe. But I think we can try this out without that in the next minor beta version. If it breaks, let's try adding an option.

Thats a great idea! Happy to revisit if we get issues.