Skip to content

implement extensions and permessage-deflate#12

Merged
the-mikedavis merged 15 commits intomainfrom
permessage-deflate
Jun 25, 2021
Merged

implement extensions and permessage-deflate#12
the-mikedavis merged 15 commits intomainfrom
permessage-deflate

Conversation

@the-mikedavis
Copy link
Copy Markdown
Collaborator

@the-mikedavis the-mikedavis commented Jun 25, 2021

closes #1

adds a functional interface for extensions (both meanings of "functional") and implements rfc7692 zip-based compression

it's notable that this makes the test suite take ~30min to run 🤦

one passes a list of extension structs to the Mint.WebSocket.upgrade/4 function in the :extensions option, with the syntax sugar of [{extension_module, params}] for short-hand

Mint.WebSocket.upgrade(conn, "/", [], extensions: [Mint.WebSocket.PerMessageDeflate])

(or the even more syntax sugary [extension_module])

not much documentation because I haven't included the whole documentation suite one needs to actually create the documentation

@the-mikedavis the-mikedavis self-assigned this Jun 25, 2021
@the-mikedavis the-mikedavis requested a review from a team June 25, 2021 21:15
Copy link
Copy Markdown

@tonyvanriet tonyvanriet left a comment

Choose a reason for hiding this comment

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

this makes encoding line up with decoding when it comes to
control-flow

I think throw/catch is generally discouraged, but it simplifies
"pipelining" the encoding/decoding process because we don't need
to write any functions that simply carry error tuples to the next
function, which in my opinion is even worse readability

throw/catch allows us to write our encode/decode functions on the
happy path and throw (ha) our hands up in the air when the spec
is violated
@the-mikedavis the-mikedavis merged commit 7ba5dec into main Jun 25, 2021
@the-mikedavis the-mikedavis deleted the permessage-deflate branch June 25, 2021 22:21
@the-mikedavis the-mikedavis mentioned this pull request Jun 26, 2021
@mtrudel
Copy link
Copy Markdown

mtrudel commented Oct 26, 2022

@the-mikedavis wondering if you have a reason for specifically not calling :zlib.close/1 here. I'm in the process of adding WebSocket deflate support to Bandit and I'm trying to figure out if it's necessary or not. The OTP docs aren't clear on the matter, but I'm assuming this is the sort of thing that's automatically closed at process termination (as are many NIF-backed libraries).

Do you have any authoritative info on this one way or the other?

@the-mikedavis
Copy link
Copy Markdown
Collaborator Author

Yep you're correct: zlib_nif.c in the OTP codebase attaches a destructor to the zlib resource that calls the close routine on garbage-collect: https://github.com/erlang/otp/blob/c22b41655830a906515ac395ae6ad77fc0ed817c/erts/emulator/nifs/common/zlib_nif.c#L304-L313

There's a note in the implementation for :zlib.close_nif/1 that calling close is not really necessary: https://github.com/erlang/otp/blob/c22b41655830a906515ac395ae6ad77fc0ed817c/erts/emulator/nifs/common/zlib_nif.c#L577

Gun and Cowboy also do not call :zlib.close/1 explicitly https://github.com/ninenines/cowlib/blob/master/src/cow_ws.erl

@mtrudel
Copy link
Copy Markdown

mtrudel commented Oct 26, 2022

Wonderful. Many thanks!

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.

per-message deflate extension support

3 participants