danielSanchezQ / warp-reverse-proxy

Fully composable warp filter that can be used as a reverse proxy.
https://github.com/danielSanchezQ/warp-reverse-proxy
MIT License
48 stars 18 forks source link

Can not correct process redirect response #51

Closed Jamsa closed 2 years ago

Jamsa commented 2 years ago

Hello The proxy_to_and_forward_response method seems can not process 30X response from backend. It will follow the redirect url and return the content with 200 status code.

I have tried to set

let reqwest_client = ReqwestClient::builder().redirect(RedirectPolicy::none()).build().unwrap();
PROXY_CLIENT.set(reqwest_client).expect("client is set");

But the response code is still 200 without Location header info.

danielSanchezQ commented 2 years ago

Hi @Jamsa

Can you post an example of the backend response? Redirected response is forwarded to the requested. So without the information of what is forwarding I don't know if I can help. Also if you have and endpoint that I can hit with the same behavior would be nice (so I can actually test and debug it myself).

timf commented 2 years ago

Without setting RedirectPolicy:none(), my experience was that the reqwest client was following redirects and not proxying the 30x. I did get it to work the way I wanted (end user experiences 30x) with code similar to the above which I posted in https://github.com/danielSanchezQ/warp-reverse-proxy/discussions/50

httpbin has an open redirect (302's to a 404 page) that you could perhaps use to test: curl -v -L -X GET "https://httpbin.org/status/302"

Jamsa commented 2 years ago

Thank you @timf and @danielSanchezQ . I have tested set RedirectPolicy to a custom policy and call attemp to stop is ok.

let custom_policy = RedirectPolicy::custom(|attempt| {
        attempt.stop()
    });
let reqwest_client = ReqwestClient::builder().redirect(custom_policy).build().unwrap();
PROXY_CLIENT.set(reqwest_client).expect("client is set");
danielSanchezQ commented 2 years ago

@timf , @Jamsa , I will add that policy as default for the next release.

danielSanchezQ commented 2 years ago

Closed in #52