cloudflare/pingora

Add DerefMut to enable complex uri manipulation for gateway

Closed this issue · 5 comments

What is the problem your feature solves, or the need it fulfills?

I could not find any example of modify request URI
so i implement my own patch, this is the source code gateway_fast.rs

when we re-assign uri directly like this

 // Update request URI using the cached rewritten path and query.
match http::uri::PathAndQuery::from_maybe_shared(rewritten_path_query.clone()) {
    Ok(pq) => {
        let mut parts = session.req_header_mut().uri.clone().into_parts();
        parts.path_and_query = Some(pq);
        match http::Uri::from_parts(parts) {
            // 
            //                                     |  this part  |
            Ok(new_uri) => session.req_header_mut().uri = new_uri,
            //                                     |^^^^^^^^^^^^^|
            Err(e) => {
                error!("Error rebuilding URI from cached parts: {}", e);
                // Fallback on error
                return Ok(DEFAULT_FALLBACK_PEER.clone()); // Clone the precomputed Box<HttpPeer>
            }
        }
    }
    Err(e) => {
        error!(
            "Invalid PathAndQuery in cache: '{}', error: {}",
            rewritten_path_query, e
        );
        // Fallback on error
        return Ok(DEFAULT_FALLBACK_PEER.clone());
    }
}

it will causes this error:

error[E0594]: cannot assign to data in dereference of `RequestHeader`
   --> router-core/src/app/gateway_fast.rs:538:40
    |
538 |                         Ok(new_uri) => session.req_header_mut().uri = new_uri,
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot assign
    |
    = help: trait `DerefMut` is required to modify through a dereference, but it is not implemented for `RequestHeader`

error[E0594]: cannot assign to data in dereference of `RequestHeader`
   --> router-core/src/app/gateway_fast.rs:599:44
    |
599 | ...                   Ok(new_uri) => session.req_header_mut().uri = new_uri,
    |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot assign
    |
    = help: trait `DerefMut` is required to modify through a dereference, but it is not implemented for `RequestHeader`

in which it would not be able to re-assign directly using .uri , which is convinient way to replace the requested URI with target URI which inside the requested URI (not response header)

i saw no uri modification function on the header things

Image

Image

Describe the solution you'd like

my solution was to add DerefMut implementation to the RequestHeader

impl DerefMut for RequestHeader {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.base
    }
}

Describe alternatives you've considered

for now i dont have any alternative. Either to add DerefMut, or to add function with capability to change the uri inside the struct.

Additional context

This could enable easier routing manipulation for multiple backend with their respective path utilizng more complex handling. as tested this method introduce almost zero performance degradation compared to running the gateway with example script.

I have PR for this in #605

@zonblade if you have a mutable reference to RequestHeader, there is a method you can call on it to change the URI: https://docs.rs/pingora/latest/pingora/http/struct.RequestHeader.html#method.set_uri

For example, in upstream_request_filter that runs before proxying the request upstream, there you can change the URI used for the upstream origin request: https://docs.rs/pingora/latest/pingora/prelude/trait.ProxyHttp.html#method.upstream_request_filter

@zonblade if you have a mutable reference to RequestHeader, there is a method you can call on it to change the URI: https://docs.rs/pingora/latest/pingora/http/struct.RequestHeader.html#method.set_uri

For example, in upstream_request_filter that runs before proxying the request upstream, there you can change the URI used for the upstream origin request: https://docs.rs/pingora/latest/pingora/prelude/trait.ProxyHttp.html#method.upstream_request_filter

will check this out and test if it's work thank's for addressing this out, much appreciated. I will go back here to inform the result

well yeah, i can do pass either using header like X-Forward-To inside the peer then i capture that header in request_body_filter then remove it right after. or doing 2nd path processing.

but it fails for this cases

  • direct modification multiple peer target by path

for example

path target peer
/one 127.0.0.1:1234
/two 127.0.0.1:2233
not found 127.0.0.1:404

i will benchmark between those 2, since it is introduce header read and write overhead. if the performance much not difference i will use the header RW method.

i will share the benchmark here

found a way. this cases is closed.