Support client_max_window_bits and server_max_window_bits compression options

[hexdigest]

This is related and fixes #443

[hexdigest]

Hey guys these :point_up: 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]

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]

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

[hexdigest]

@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]

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]

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 :point_up: 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]

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.

[mafredri]

FYI @hexdigest I came across today that this library used to integrate klauspost/compress, removed in #240. The reason seems to have been that it was too big (#220). I see this as validating for the approach we've discussed here.