cloudflare / pingora

A library for building fast, reliable and evolvable network services.
Apache License 2.0
20.28k stars 1.1k forks source link

Downstream compression does not work for custom response #206

Closed vicanso closed 2 weeks ago

vicanso commented 2 months ago

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

I make a custom response in request_filter like this:

session.downstream_compression.adjust_decompression(true);
session.downstream_compression.adjust_level(9);
session.write_response_header(Box::new(header)).await?;
session.write_response_body(self.body).await?;
return Ok(true);

But the response body is not compressed. Does compression only take effect for response from upstream?

Describe the solution you'd like

I want downstream compression to take effect for all responses to downstream.

eaufavor commented 2 months ago

You are right. The compression module in the proxy logic needs some rework to take care of this case.

palant commented 1 month ago

At the moment this can be achieved by calling session.downstream_compression methods manually along these lines:

session.downstream_compression.adjust_level(3);

session
    .downstream_compression
    .request_filter(session.downstream_session.req_header());

let mut task = HttpTask::Header(Box::new(header), false);
session.downstream_compression.response_filter(&mut task);
if let HttpTask::Header(header, _) = task {
    session.write_response_header(header).await?;
} else {
    panic!("Unexpected: compression response filter replaced header task by {task:?}");
}

let mut task = HttpTask::Body(Some(self.body), true);
session.downstream_compression.response_filter(&mut task);
if let HttpTask::Body(Some(bytes), _) = task {
    session.write_response_body(bytes).await?;
}

return Ok(true);

If the response body consists of multiple chunks, response_filter() has to be called multiple times, probably followed up by a call with HttpTask::Done parameter. It is also necessary to make sure not to write out any empty chunks potentially produced by the compression (these indicate end of stream with chunked encoding).

All in all, not really trivial. But it works.

gumpt commented 2 weeks ago

This should have been fixed with 11863d27a2569ca8238a9b39b25d604635bca642. These should behave as expected in the original issue post if the module is set up.

Please let us know if this did not solve your issue.

palant commented 2 weeks ago

@gumpt: It looks like you forgot to adjust Session::write_response_header_ref method. It will be forwarded to the downstream session without triggering modules.

palant commented 2 weeks ago

@gumpt I’ve created a branch, adjusting my code to these changes. It’s a huge change, but things are mostly working now. As far as I can tell, compression works correctly as well with the bulk of my work-around removed. Well, except for Session::write_response_header_ref as mentioned above – but luckily my code doesn’t actually use it.