wsflate: handle short compressor EOF blocks - #227
Open
sb123sb123 wants to merge 2 commits into
Open
sb123sb123 wants to merge 2 commits into
sb123sb123 wants to merge 2 commits into
Conversation
cristaloleg
reviewed
Sep 24, 2026
| w.flushed = false | ||
| w.err = w.c.Flush() | ||
| w.checkTail() | ||
| if w.err == nil { |
Collaborator
There was a problem hiding this comment.
w.flushed = w.err == nil ?
cristaloleg
approved these changes
Sep 29, 2026
Author
|
Thanks for the review and approval. The Go CI matrix for this head is green. The separate Autobahn workflow fails while building its test image, before the Autobahn suite starts: installing Failing run: https://github.com/gobwas/ws/actions/runs/36542593276 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #221
Problem
wsflate.Writerassumes that a compressor'sCloseleaves the four-byte stored-block tail (00 00 ff ff) in its buffer. Go 1.27'scompress/flateemits a short fixed-Huffman EOF block instead, so closing after the required PMCE flush can move buffered bytes to the destination and return a falsewsflate: bad compressorerror.Cause
The PMCE sync-flush marker and the compressor's final EOF block are different protocol concerns, but
Closechecked and forwarded them through the same four-byte tail buffer.Fix
Track whether the last successful operation was
Flush. When closing a flushed writer, still call the compressor'sClosemethod, but discard only its close-time output so the PMCE message remains free of the final block. A subsequentWriteclears the state and requires a new flush.The regression test models the short fixed-Huffman EOF block and verifies that the compressor is closed, no error is reported, and the flushed payload is unchanged.
Tests
go test -count=1 -run '^TestWriterCloseAfterFlushWithShortFinalBlock$' ./wsflate(passed with task-local dependency replacements)go test ./...(passed with task-local dependency replacements)go vet ./wsflate(passed)gofmt -d wsflate/writer.go wsflate/writer_test.go(clean)git diff --check(clean)Limitations
The remote Windows host runs Go 1.26.8, so it could not execute the Go 1.27 compressor directly. The Go module proxy and direct module fetches were unavailable; I used temporary local copies of the two small dependencies for testing and removed them before committing. Full-module vet also reports two pre-existing findings outside this change:
wsutil/writer_test.go:135(reflect.SliceHeader) andexample/autobahn/autobahn.go:58(discardedcontext.WithTimeoutcancel function). The race test was not run because the host has nogccC compiler.AI assistance disclosure
This PR was prepared with assistance from OpenAI Codex. Codex performed issue triage, implementation, regression-test authoring, and validation; the final patch and reported results were checked against the repository and command output.