feat: Update reverse proxy example to support both http and https #2696
Conversation
| //? Remove incorrect header host, hyper will add automatically for you. | ||
| req.headers_mut().remove(HOST).unwrap(); |
There was a problem hiding this comment.
Why the question mark in front here? Personally I don't know if this is the right behavior here, @yanns can you comment on it since you approved (in addition to @afifurrohman-id)?
Even if we keep it I don't think there's any reason to .unwrap() there though.
There was a problem hiding this comment.
When proxing a HTTP request, the initial HTTP request contains as HOST the address of the proxy (locahost:4000). If we use that value against another server, that server will refuse the HTTP request as it expects localhost:3000 instead.
There was a problem hiding this comment.
So this was previously broken? Also, should the header value be added as some (de-facto) standard proxied-host header?
There was a problem hiding this comment.
So this was previously broken?
I guess yes. I have not checked. I've built several proxy and this is one of the usual issues.
Also, should the header value be added as some (de-facto) standard proxied-host header?
If we want to go in the direction of "good behaving" proxy, yes they are several headers that should be set:
There was a problem hiding this comment.
I just tried the original example and it works.
The host header would be an issue if the two servers were running externally visible on the same port as virtual servers with nginx or something else in front of them that would forward the requests based on the host header.
axum, hyper, or anything else in the stack does not check the header, because it can't even know what to check it for. The request just arrives either on localhost:4000 or localhost:3000 and then it is processed.
Personally, I think that handling of this and the addition of an external dependency on example.com is a bit sad and it would be better to just add an axum tls server (based on any of the other examples) and call that instead.
But even with example.com, the header change is not really needed as curl -v https://example.com -H 'host: localhost:3000' returns a 404 -- it's not the same page you get if you let curl use the correct host (and it might be returned by a reverse proxy and not the actual server), but it is a response to an https request so it does show that this works.
Generally, I think the examples should be as simple as possible to showcase the thing they mean to (here that's relaying a request, potentially with a change to the protocol), but details such as extension headers can and should be omitted for brevity.
There was a problem hiding this comment.
Why the question mark in front here?
I just for clarify why we remove HOST header, as @yanns say.
There was a problem hiding this comment.
If we want to go in the direction of "good behaving" proxy, yes they are several headers that should be set:
I also think like so, we need add it as well, like some other reverse proxy, it will also make more great example.
There was a problem hiding this comment.
Personally, I think that handling of this and the addition of an external dependency on
example.comis a bit sad and it would be better to just add an axum tls server (based on any of the other examples) and call that instead.
Yes, first because it think to simple as possible use an example.com can add more precisely how we can interact with external service, but will add some external network request as well.
Motivation
Example for reverse proxy currently only using http, but sometimes we want to proxy https server as well.
Solution
Update existing example and added it.