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

[UPDATED] Compression library for Websocket #1259

Merged
merged 4 commits into from
May 3, 2023
Merged

[UPDATED] Compression library for Websocket #1259

merged 4 commits into from
May 3, 2023

Conversation

kozlovic
Copy link
Member

Signed-off-by: Derek Collison derek@nats.io
Signed-off-by: Ivan Kozlovic ivan@synadia.com

derekcollison and others added 4 commits April 28, 2023 13:28
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
With compress/flate, it seems that after calling Close() it was
fine dropping the last 4 bytes before doing the framing, but
with the different flate, it fails.

Instead, use Flush() (like in the server) and then dropping the
last 4 bytes works as before.

I have added a compression.Close() on connection close, after
sending the CLOSE frame.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@piotrpio piotrpio left a comment

Choose a reason for hiding this comment

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

LGTM!

@derekcollison
Copy link
Member

A couple of failing tests, might be flappers or might be due to upgrade of underlying server. I doubt related to WS compression.

@coveralls
Copy link

Coverage Status

Coverage: 85.11% (-0.04%) from 85.152% when pulling 1b25a76 on flate into 2857164 on main.

@derekcollison
Copy link
Member

Can someone take a look at TestNoRaceObjectContextOpt?

@piotrpio
Copy link
Collaborator

piotrpio commented May 1, 2023

I'll take a look at it

@piotrpio
Copy link
Collaborator

piotrpio commented May 3, 2023

I found some issues in object store and fixed them here: #1260. They had nothing to do with this PR, so I'll merge it now.

@piotrpio piotrpio merged commit d963c7d into main May 3, 2023
2 of 3 checks passed
@piotrpio piotrpio deleted the flate branch May 3, 2023 15:31
@piotrpio piotrpio mentioned this pull request May 23, 2023
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

4 participants