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

pingora-http incorrectly handling URI with host #476

Open
jeroenvervaeke opened this issue Nov 19, 2024 · 2 comments · May be fixed by #477
Open

pingora-http incorrectly handling URI with host #476

jeroenvervaeke opened this issue Nov 19, 2024 · 2 comments · May be fixed by #477
Assignees
Labels
bug Something isn't working

Comments

@jeroenvervaeke
Copy link

jeroenvervaeke commented Nov 19, 2024

Describe the bug

I am trying to build a proxy service which proxies a http request and modifies certain requests.

I set up some easy code, just to confirm that I can make a transparent http proxy, which basically just proxies the request without modifying it.

This is the basic example I came up with.
This example crashes because uri.host() returns None.

use async_trait::async_trait;
use pingora::prelude::HttpPeer;
use pingora::proxy::{http_proxy_service, ProxyHttp, Session};
use pingora::server::Server;
use pingora::Result;

fn main() {
    // Create server instance
    let mut my_server = Server::new(None).unwrap();
    my_server.bootstrap();

    // Create proxy service
    let mut proxy_service = http_proxy_service(&my_server.configuration, MyProxy);

    // Add listening address
    proxy_service.add_tcp("0.0.0.0:8080");

    // Register service with server
    my_server.add_service(proxy_service);

    // Start server
    my_server.run_forever();
}

// Create your proxy struct
pub struct MyProxy;

#[async_trait]
impl ProxyHttp for MyProxy {
    // Define context type - use () if you don't need context
    type CTX = ();

    fn new_ctx(&self) -> Self::CTX {
        ()
    }

    // This is the only required method
    async fn upstream_peer(
        &self,
        session: &mut Session,
        _ctx: &mut Self::CTX,
    ) -> Result<Box<HttpPeer>> {
        // Get the request header
        let uri = &session.req_header().uri;

        // Print the uri and it's parts for debugging purposes
        let parts = uri.clone().into_parts();
        println!("uri: {uri}, parts {parts:?}");

        let host = uri.host().unwrap();
        let port = uri.port_u16().unwrap_or(443);

        Ok(Box::new(HttpPeer::new(
            (host, port),
            true,
            host.to_string(),
        )))
    }
}

Upon further inspection of my debug logs it shows that the uri is parsed incorrectly.

uri: http://example.com/, parts Parts { scheme: None, authority: None, path_and_query: Some(/http://example.com/), _priv: () }
uri: http://example.com/favicon.ico, parts Parts { scheme: None, authority: None, path_and_query: Some(/http://example.com/favicon.ico), _priv: () }

The entire uri is in path_and_query, while example.com should be in Authority. (uri.host() calls map on authority).

I made a fix which parses the url correctly (see PR here).
To be clear, my code is not working yet (I probably need to do a dns lookup and return the ip instead of the domain name), but that's not the point of this issue.

Now the logs of my simple test program are:

uri: http://example.com/, parts Parts { scheme: Some("http"), authority: Some(example.com), path_and_query: Some(/), _priv: () }
uri: http://example.com/favicon.ico, parts Parts { scheme: Some("http"), authority: Some(example.com), path_and_query: Some(/favicon.ico), _priv: () }

Pingora info

Please include the following information about your environment:

Pingora version: latest version on crates.io 0.4.0 and also latest version on master: sha bdb13a7e40b19280d703ad1f64abc29ee5baf480
Rust version: cargo 1.81.0 (2dbb1af80 2024-08-20)
Operating system version: Arch Linux 6.11.5-arch1-1

Steps to reproduce

See bug description.

Expected results

What were you expecting to happen?

When proxying http://example.com/favicon.ico.

Inside
async fn upstream_peer( &self, session: &mut Session, _ctx: &mut Self::CTX, ) -> Result<Box<HttpPeer>> {

I expect:

  • session.req_header().uri.host() to be Some("example.com")
  • session.req_header().url path_and_query to be equal to: /favicon.ico

Observed results

What actually happened?

  • session.req_header().uri.host() is None
  • session.req_header().url path_and_query is /favicon.ico

Additional context

I've created a PR to solve this issue.
See #477

@lebanggit
Copy link

I was faced this issue with TLS connection (http2).

@eaufavor eaufavor added the bug Something isn't working label Dec 14, 2024
@hominsu
Copy link

hominsu commented Dec 24, 2024

same issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants