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

Downstream compression does not work for custom response #206

Closed
vicanso opened this issue Apr 13, 2024 · 5 comments
Closed

Downstream compression does not work for custom response #206

vicanso opened this issue Apr 13, 2024 · 5 comments
Assignees
Labels
bug Something isn't working WIP We are working on this feature internally

Comments

@vicanso
Copy link

vicanso commented Apr 13, 2024

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.

@johnhurt johnhurt added the bug Something isn't working label Apr 26, 2024
@eaufavor
Copy link
Member

eaufavor commented May 1, 2024

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

@palant
Copy link
Contributor

palant commented May 6, 2024

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 gumpt added the WIP We are working on this feature internally label May 10, 2024
@gumpt
Copy link
Contributor

gumpt commented Jun 14, 2024

This should have been fixed with 11863d2. 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.

@gumpt gumpt closed this as completed Jun 14, 2024
@palant
Copy link
Contributor

palant commented Jun 14, 2024

@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
Copy link
Contributor

palant commented Jun 15, 2024

@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.

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

No branches or pull requests

5 participants