Navigation Menu

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

Jump to bottom

compress/flate: Close is not idempotent #27741

Closed
dsnet opened this issue Sep 18, 2018 · 4 comments
Closed

compress/flate: Close is not idempotent #27741

dsnet opened this issue Sep 18, 2018 · 4 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.19

Comments

@dsnet
Copy link
Member

dsnet commented Sep 18, 2018

The flate.Writer.Close should be an idempotent operation. Furthermore, it should disallow further calls to Write:

b := new(bytes.Buffer)
zw, _ := flate.NewWriter(b, 6)
fmt.Println(zw.Write([]byte("hello, world!")))
fmt.Println(zw.Close())
fmt.Println(zw.Write([]byte("hello, world!")))
fmt.Println(zw.Close())
fmt.Printf("%x\n", b.String())

Currently, this prints:

13 <nil>
<nil>
13 <nil>
<nil>
ca48cdc9c9d75128cf2fca495104040000ffff42e100020000ffff

I expect it to print something like:

13 <nil>
<nil>
0 flate: write on closed stream
<nil>
ca48cdc9c9d75128cf2fca495104040000ffff

Currently, allowing a Write after a Close results in a corrupted stream. The trailing 42e100020000ffff is indicative of a backwards reference to the previous block. However, this is invalid since the previous block terminated the stream with the FINAL bit set.

This hasn't been much of a problem in practice since almost all usages of flate are through gzip, which does have this property.
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 19, 2018
@bcmills bcmills added this to the Unplanned milestone Sep 19, 2018
@bcmills
Copy link
Contributor

bcmills commented Sep 19, 2018

Do you have a change pending, or should this be “help wanted”?

@dsnet
Copy link
Member Author

dsnet commented Sep 19, 2018

I have no pending change. Great change for interested contributor. I can review.

@gopherbot
Copy link

Change https://golang.org/cl/136475 mentions this issue: compress/flate: Return error on closed stream write.

@urandom2 urandom2 mentioned this issue Jul 29, 2020
Closed
@teivah teivah mentioned this issue Apr 29, 2022
Closed
@gopherbot
Copy link

Change https://go.dev/cl/403514 mentions this issue: compress/flate: move idempotent close logic to compressor

@dmitshur dmitshur modified the milestones: Unplanned, Go1.19 May 2, 2022
gopherbot pushed a commit that referenced this issue May 2, 2022
@dsnet @gopherbot
compress/flate: move idempotent close logic to compressor
a887579 
The compressor methods already have logic for handling a sticky error.
Merge the logic from CL 136475 into that.

This slightly changes the error message to be more sensible
in the situation where it's returned by Flush.

Updates #27741

Change-Id: Ie34cf3164d0fa6bd0811175ca467dbbcb3be1395
Reviewed-on: https://go-review.googlesource.com/c/go/+/403514
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@golang golang locked and limited conversation to collaborators May 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.19
Development

No branches or pull requests
4 participants
@dmitshur @bcmills @dsnet @gopherbot