Skip to content

Conversation

hexdigest
Copy link

@hexdigest hexdigest commented Aug 12, 2025

This is related and fixes #443

@hexdigest
Copy link
Author

Hey guys these ☝️ are failing due to thirdparty dependency.

@nhooyr the third party dependency I introduce here is a well known drop in replacement for compress/flate. It also has constructors that allows you to specify the size of the window.

@mafredri
Copy link
Member

Hey @hexdigest. Thanks for the contribution! I'll need to read up on this a bit to give a proper review, so it might take a while.

In the meantime, I'm thinking an implementation that allows providing a custom flate implementation via options would be preferable. This way we don't need to add any external dependencies to this library, yet we support the use-case. It'd be fine to even include an example of how to do it as part of the go docs.

@mafredri
Copy link
Member

mafredri commented Sep 1, 2025

@hexdigest is this something you still want to work on?

@hexdigest
Copy link
Author

@hexdigest is this something you still want to work on?

Yep, I can do it. My only concern about the options approach is that depending on the provided flate implementation we should enable or disable certain compression options. But I guess this could be a part of flate abstraction interface (that can return a list of supported options) and manage pools.

Does it make sense to you?

@mafredri
Copy link
Member

mafredri commented Sep 1, 2025

What kind of use-cases are you considering?

Off the top of my head I could see us having something like this:

// Match stdlib flate.Writer, add release.
type FlateWriter interface {
	io.WriteCloser
	Flush() error
	Reset(io.Writer)
	Release() // Allow backing implementation to restore to pool.
}

type Options struct {
	FlateWriter func(w io.Writer, bits int) FlateWriter
}

But open to other suggestions, especially if this doesn't cover some use-case.

@hexdigest
Copy link
Author

What kind of use-cases are you considering?

Off the top of my head I could see us having something like this:

// Match stdlib flate.Writer, add release.
type FlateWriter interface {
	io.WriteCloser
	Flush() error
	Reset(io.Writer)
	Release() // Allow backing implementation to restore to pool.
}

type Options struct {
	FlateWriter func(w io.Writer, bits int) FlateWriter
}

I was thinking about something like this ☝️ maybe with the one minor difference which is putting Release() logic into Close() method of WriteCloser.

But the thing that I was talking about in the above message is that depending on the FlateWriter implementation the websocket implementation (e.g. *_max_window_bits) might or might not accept certain compression options passed by the client. So maybe we can add a method to this FlateWriter interface that returns compression options that it supports.

WDYT?

@mafredri
Copy link
Member

mafredri commented Sep 5, 2025

I was thinking about something like this ☝️ maybe with the one minor difference which is putting Release() logic into Close() method of WriteCloser.

Yeah, that's honestly better 👍🏻

But the thing that I was talking about in the above message is that depending on the FlateWriter implementation the websocket implementation (e.g. *_max_window_bits) might or might not accept certain compression options passed by the client. So maybe we can add a method to this FlateWriter interface that returns compression options that it supports.

WDYT?

That could overcomplicate the interface. If I'm understanding you correctly, I think we could cover the use-case by adding CompressionServerWindowMaxBits and CompressionServerWindowMinBits to AcceptOptions? This way negotiation is driven by server policy, not by querying a particular writer. The writer just accepts the bits we already negotiated.

Usage would then look something like:

c, err := websocket.Accept(w, r, &websocket.AcceptOptions{
	CompressionServerWindowMinBits: 9,
	CompressionServerWindowMaxBits: 15,
	CompressionFlateWriter:         kpFlateWriter(kpflate.DefaultCompression),
})

func kpFlateWriter(level int) func(io.Writer, int) FlateWriter {
    return func(dst io.Writer, bits int) FlateWriter {
        // ...
    }
}

Does that make sense?

Edit: Perhaps best to add Compression prefix to flate writer as well so all compression options look the same.

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.

unsupported permessage-deflate parameter: "client_max_window_bits=15" from client
3 participants