protocol: avoid double buffering #116
Conversation
The mod file contains replace directive to include my fixes to the library [1]. [1] pires/go-proxyproto#116
mmatczuk
force-pushed
the
mmt/dont_use_read_buffer
branch
from
b4098fd to
45b522d
Compare
|
I fixed the linting issues in the patch, and piggybacked patch to fix the ioutil usage. |
|
Hey there! Thank you for your contribution. Would you be so kind as to check the tests failure? https://github.com/pires/go-proxyproto/actions/runs/10919900083/job/30323349143?pr=116#step:5:194 |
mmatczuk
force-pushed
the
mmt/dont_use_read_buffer
branch
from
45b522d to
f7fa186
Compare
|
Sure, it's done. |
|
I am paying attention to this but I need #110 to land first 🙏🏻 thanks for your patience. |
|
Actually, I'm merging this now. Thanks a lot for your contribution. |
This field order reflect usage pattern and avoids mixing exported and unexported fields.
Use buffer only to read the PROXY header. Users may use they own buffers with they own buffer sizes and pools - connection should respect that.
pires
force-pushed
the
mmt/dont_use_read_buffer
branch
from
f7fa186 to
ccf1e75
Compare
| pConn := &Conn{ | ||
| bufReader: bufio.NewReader(conn), | ||
| bufReader: br, | ||
| reader: io.MultiReader(br, conn), |
There was a problem hiding this comment.
I got the idea but this does not look right: io.MultiReader will read br reader until EOF but it is a buffered reader that will read until connection returns EOF.
There was a problem hiding this comment.
I think it should be something like #119
Reverts and re-implemnets incorrect pires#116 which passed all connection data through the buffered reader.
Reverts and re-implements incorrect pires#116 which passed all connection data through the buffered reader.
Reverts and re-implements incorrect pires#116 which passed all connection data through the buffered reader.
Reverts and re-implements incorrect pires#116 which passed all connection data through the buffered reader.
|
@AlexanderYastrebov is correct and there is a bug that causes the I think it would be better to instantiate the buffer in Taking a wider perspective I think that it would be better to have the parsing APIs based on io.Reader and use ReadAll instead of |
Fix bug introduced in pires#116 where io.MultiReader only reads from buffered reader. Move buffer reader management to readHeader(). Remove Conn.bufReader, and make Conn.reader nil until readHeader() is called.
Yes, but then again one needs to keep a copy of read bytes in case there is no PROXY header to support #106 usecase. |
Use smaller buffer size, and use buffer only to read the PROXY header.
Users may use they own buffers with they own buffer sizes and pools - connection should respect that.