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

fixed inflating uncompressed messages in deflate extension #12

Merged
merged 1 commit into from
May 9, 2024

Conversation

GenrikhFetischev
Copy link
Contributor

@GenrikhFetischev GenrikhFetischev commented Jan 19, 2024

This PR created as a quick fix of the decompression issue

@@ -33,7 +33,7 @@ use tokio::io::AsyncRead;
use tokio_util::codec::Decoder;
use url::Url;

pub use client::{subscribe, subscribe_with, HandshakeResult, UpgradedClient};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was made to satisfy linter, according to rules here - https://github.com/swimos/ratchet/blob/main/ratchet_core/src/lib.rs#L25

@SirCipher
Copy link
Member

@GenrikhFetischev, apologies for the very late response and thank you for raising an issue and submitting a fix. While it appears that the logic in the block where you applied your fix is incorrect, your fix does not include support for fragmented messages. For a fragmented message, only the first frame will have rsv1 set high and the subsequent frames will be low. See 7.2.3.1:

Note that the RSV1 bit is set only on the first frame.

For this to work, the decode implementation needs to check if the rsv1 flag has been set high and if the fin flag is low:

        match header.opcode {
            OpCode::Binary | OpCode::Text if header.rsv1 => {
                if !header.fin {
                    *compressed = true;
                    return Ok(());
                }
            }
            OpCode::Continuation if header.fin && *compressed => {}
            _ => return Ok(()),
        }

@GenrikhFetischev
Copy link
Contributor Author

GenrikhFetischev commented May 7, 2024

@SirCipher Yep, good catch!
Would you like me to update PR or your intent is to fix by yourself?
I could do it if you have time to review in the nearest future :)

@SirCipher
Copy link
Member

@GenrikhFetischev, you're welcome to submit a PR for it! 😄

@GenrikhFetischev
Copy link
Contributor Author

@SirCipher ok, will work on it soon 🤝

@GenrikhFetischev
Copy link
Contributor Author

@SirCipher I have updated PR, could you take a look please?

@SirCipher SirCipher merged commit 2d5406b into swimos:main May 9, 2024
6 checks passed
@SirCipher
Copy link
Member

@GenrikhFetischev merged. Thanks for your contributions!

@SirCipher
Copy link
Member

@GenrikhFetischev released in https://github.com/swimos/ratchet/releases/tag/v0.4.2

@GenrikhFetischev
Copy link
Contributor Author

@GenrikhFetischev merged. Thanks for your contributions!

@SirCipher it was a pleasure considering you have provided solution 🙂

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.

None yet

2 participants