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

Buffer only proxy header data #119

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AlexanderYastrebov
Copy link

@AlexanderYastrebov AlexanderYastrebov commented Oct 10, 2024

Reverts and re-implements incorrect #116
which passed all connection data through the buffered reader.

@pires
Copy link
Owner

pires commented Oct 11, 2024

@mmatczuk care to take a look at this?

@coveralls
Copy link

coveralls commented Oct 11, 2024

Coverage Status

coverage: 94.67% (+0.5%) from 94.136%
when pulling 0067a88 on AlexanderYastrebov:no-buffer
into bac82fd on pires:main.

Reverts and re-implements incorrect pires#116
which passed all connection data through the buffered reader.
@mmatczuk
Copy link
Contributor

I added my comments in #116 (comment).

@mmatczuk
Copy link
Contributor

Sent a followup patch #120.

@AlexanderYastrebov

This comment was marked as outdated.

@mmatczuk
Copy link
Contributor

@AlexanderYastrebov

It also has issues.

PTAL at my comments in #120.

Besides I fixed WriteTo instead of hand-rolled reads.

Sure, this is also fixed in #120

BTW adding the WriteTo to the interface if the underlying connection does not implement in may trick io.Copy with buffer from a buffer pool to falling back to plain io.Copy that allocates. Also the WriteTo only works on Linux IF the writer is UnixConn.

Cheers

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.

4 participants