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.

Sign up for GitHub

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

feat: Update reverse proxy example to support both http and https #2696

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: Update reverse proxy example to support both http and https #2696

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d29cfd7
feat: Update reverse proxy example to support both http and https
afifurrohman-id Apr 2, 2024
37900c6
fix(ci): Format `Cargo.toml` and perform cargo fmt
afifurrohman-id Apr 2, 2024
e01086b
Merge branch 'tokio-rs:main' into reverse-proxy-example-patch
afifurrohman-id May 10, 2024
40971a4
refactor: add some informative header (`X-Forwarded-For`, `X-Forwarde…
afifurrohman-id May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/reverse-proxy/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ edition = "2021"

[dependencies]
axum = { path = "../../axum" }
hyper = { version = "1.0.0", features = ["full"] }
hyper-tls = { version = "0.6.0", features = ["vendored"] }
hyper-util = { version = "0.1.1", features = ["client-legacy"] }
tokio = { version = "1", features = ["full"] }
28 changes: 19 additions & 9 deletions examples/reverse-proxy/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,38 @@
//! Reverse proxy listening in "localhost:4000" will proxy all requests to "localhost:3000"
//! Reverse proxy listening in "localhost:4000" will proxy all `GET` requests to "localhost:3000" except for path /https is example.com
//! endpoint.
//!
//! On unix like OS: make sure `ca-certificates` is installed.
//!
//! Run with
//!
//! ```not_rust
//! cargo run -p example-reverse-proxy
//! ```

use axum::http::{header::HOST, StatusCode};
use axum::{
body::Body,
extract::{Request, State},
http::uri::Uri,
response::{IntoResponse, Response},
routing::get,
Router,
routing, Router,
};
use hyper::StatusCode;
use hyper_tls::HttpsConnector;
use hyper_util::{client::legacy::connect::HttpConnector, rt::TokioExecutor};

type Client = hyper_util::client::legacy::Client<HttpConnector, Body>;
type Client = hyper_util::client::legacy::Client<HttpsConnector<HttpConnector>, Body>;

#[tokio::main]
async fn main() {
tokio::spawn(server());

let client: Client =
hyper_util::client::legacy::Client::<(), ()>::builder(TokioExecutor::new())
.build(HttpConnector::new());
.build(HttpsConnector::new());

let app = Router::new().route("/", get(handler)).with_state(client);
let app = Router::new()
.fallback(routing::get(handler))
.with_state(client);

let listener = tokio::net::TcpListener::bind("127.0.0.1:4000")
.await
Expand All @@ -45,10 +49,16 @@ async fn handler(State(client): State<Client>, mut req: Request) -> Result<Respo
.map(|v| v.as_str())
.unwrap_or(path);

let uri = format!("http://127.0.0.1:3000{}", path_query);
let mut uri = format!("http://127.0.0.1:3000{}", path_query);
if path == "/https" {
uri = String::from("https://example.com");
}

*req.uri_mut() = Uri::try_from(uri).unwrap();

//? Remove incorrect header host, hyper will add automatically for you.
req.headers_mut().remove(HOST).unwrap();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this was previously broken? Also, should the header value be added as some (de-facto) standard proxied-host header?

Copy link
Collaborator

@yanns yanns May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the question mark in front here?

I just for clarify why we remove HOST header, as @yanns say.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@afifurrohman-id afifurrohman-id May 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

However, using other examples like tls-rustls add another complexity because will required to handle self-signed certificate

Ok(client
.request(req)
.await
Expand All @@ -57,7 +67,7 @@ async fn handler(State(client): State<Client>, mut req: Request) -> Result<Respo
}

async fn server() {
let app = Router::new().route("/", get(|| async { "Hello, world!" }));
let app = Router::new().fallback(routing::get(|| async { "Hello, world!" }));

let listener = tokio::net::TcpListener::bind("127.0.0.1:3000")
.await
Expand Down
Loading