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

Don't compress response if the x-no-compression header is present #1376

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

sygem
Copy link
Member

@sygem sygem commented Jul 23, 2024

Several FluxOS endpoint use server-sent events to show progress to the user. For example '/apps/testappinstall/' details the install process as it goes.

I have been unable to get Flutter to read these streams, and managed to track the problem down to the express compression plugin.

This solution allows a client to send a "x-no-compression" header with the request to disable the response compression. If no such header is sent, compression will take place as normal. I am then able to read the SSE streams in Flux Cloud.

@sygem sygem requested review from TheTrunk and Cabecinha84 July 23, 2024 11:01
Copy link
Member

@Cabecinha84 Cabecinha84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack

@Cabecinha84 Cabecinha84 merged commit 790ac0b into development Jul 23, 2024
4 checks passed
@MorningLightMountain713
Copy link
Contributor

MorningLightMountain713 commented Jul 23, 2024

@sygem
@Cabecinha84

We don't need to do this...

The compression middleware allows for this already:

This middleware will never compress responses that include a Cache-Control header with the [no-transform directive](https://tools.ietf.org/html/rfc7234#section-5.2.2.4), as compressing will transform the body.

@Cabecinha84
Copy link
Member

Cabecinha84 commented Jul 23, 2024

I have merged and tested on development node and I don't see any difference sending the x-no-compression header

@Cabecinha84
Copy link
Member

@sygem can you check against node http://94.16.104.218:16127/ (dev node with the changes already running) if it makes any difference? If not I will revert it. Please let me know ASAP

@sygem
Copy link
Member Author

sygem commented Jul 23, 2024

@sygem @Cabecinha84

We don't need to do this...

The compression middleware allows for this already:

This middleware will never compress responses that include a Cache-Control header with the [no-transform directive](https://tools.ietf.org/html/rfc7234#section-5.2.2.4), as compressing will transform the body.

I tried that, but couldn't get it to work with my Flutter tests.

Cabecinha84 added a commit that referenced this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants