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 AEAD stream writer to write a big chunk buffer #30

Merged
merged 1 commit into from
Sep 21, 2018
Merged

Fix AEAD stream writer to write a big chunk buffer #30

merged 1 commit into from
Sep 21, 2018

Conversation

Dreamacro
Copy link
Contributor

Problem

I used riobard/go-shadowsocks2 as an ss-client in my project and I wrapper a net.Conn after StreamConn without WriteTo and ReadFrom.

Then I use io.Copy with two TCP connection. In AEAD cipher, I found out that the connection closes when to upload some images. The Writer is not written correctly when writing a big buffer.

Question

When I discovered this bug, I assumed that shadowsocks/go-shadowsocks2 had the same bug. But when I went to check, I found a big gap between the two implementations. So the two repositories are completely forked?

@riobard riobard merged commit 9979384 into riobard:master Sep 21, 2018
@riobard
Copy link
Owner

riobard commented Sep 21, 2018

Good catch! Thanks for the PR! Could you please send the PR to the stable repo at shadowsocks/go-shadowsocks2 as well?

This repo and the one at shadowsocks/go-shadowsocks2 have diverged since this repo is experimental for my personal use cases, and the stable repo needs to be, well, stable for the public. Some features are not suitable and/or mature enough to be upstreamed to the stable repo. If you want stability and better support, please use that one.

@Dreamacro Dreamacro deleted the fix-aead-stream-writer branch September 21, 2018 17:01
@Dreamacro
Copy link
Contributor Author

shadowsocks/go-shadowsocks2 doesn't have that problem. There is a big difference between the two repositories.

@riobard
Copy link
Owner

riobard commented Sep 22, 2018

Ah, right! There was a refactor that changed the exposed API of this repo…

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.

2 participants