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

Fix zlib cert decompression by ignoring EOF #206

Merged
merged 2 commits into from Aug 1, 2023

Conversation

hwh33
Copy link
Contributor

@hwh33 hwh33 commented Aug 1, 2023

An io.Reader is allowed (but not required) to return io.EOF upon reading the
last byte of the stream, even if no subsequent bytes were requested. We later
check that we read the expected number of bytes, so we can safely ignore EOF
errors returned by the decompression readers.

An io.Reader is allowed (but not required) to return io.EOF upon reading the
last byte of the stream, even if no subsequent bytes were requested. We later
check that we read the expected number of bytes, so we can safely ignore EOF
errors returned by the decompression readers.
@hwh33
Copy link
Contributor Author

hwh33 commented Aug 1, 2023

Fixes #205

Thanks for calling attention to the issue @yan-foto and @felipe-linares - also for the fix suggestion, which turned out to be correct.

@hwh33
Copy link
Contributor Author

hwh33 commented Aug 1, 2023

Looks like the same CI failures occur on main right now (and this change should clearly not create any failures as the code path is untested).

go.sum Show resolved Hide resolved
@gaukas
Copy link
Member

gaukas commented Aug 1, 2023

The CI failure is caused by testdata from Go 1.19, since Go 1.20 updated implementations and essentially broke the old per-byte comparison test. The CI for Go 1.20 and 1.21 will pass once we bump up version (perhaps when Go 1.21 becomes stable).

@gaukas gaukas merged commit d73321b into refraction-networking:master Aug 1, 2023
1 of 3 checks passed
@gaukas gaukas mentioned this pull request Aug 2, 2023
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.

None yet

2 participants