this test is also passing with the previous MultiReader approach (although back then we always used the bufferedReader and never directly the conn, but that is also not tested).

If you want to stick with adhering strictly to the reader contract, I would modify this test so that we actually validate that we reach the underlying Conn (I can do that if you let me modify your branch). I.e. so that this test fails on the earlier version of the code. the modification amounts to doing a double-read with a 4KiB buffer and using a payload like 1KiB:

1. we receive the buffered bytes
2. we receive the remaining bytes with the following Read() call.

You should be able to open a PR against this PR. Since I'm working directly on this repo, I would have to give you write permissions to it so you could write to this branch, which is not ideal.

IIUC the test fails with the earlier version of the code but passes now as documented in #148 (comment).

if you really don't want to slightly break the Reader Contract, then I would ask you to increase this bufSize to the default of 4 KiB (i.e. do a bufio.NewReader(conn)). this would mimic the old armon/go-proxyproto behavior, solve ingress nginx issue and potentially other users of this library.

in using this library though I'm expecting to transparently hit the underlying conn. therefore, when making a call to pConn.Read() with a 4KiB buffer, I am not at all expecting that the library will only return 210 bytes.
the alternatives are not compelling either: io.ReadAll would be reading too many bytes and io.ReadFull would return ErrUnexpectedEOF if there's an EOF before my buffer is filled.

if I want to mimic the "transparent" behavior (i.e. providing a 4KiB buffer and receiving 4KiB if those are available on the OS buffers), I'll have to make double Read calls as a user, to first drain the bufferedReader and then reach the actual Conn.
This workaround is both impractical and counter-intuitive, as I would expect from a library that transparently pipes an underlying conn to transparently give me access to the underlying conn, but at least with your fix I would actually reach the underlying Conn and this solution would work (with the previous version that would've been impossible)

Please, see #150 for my preferred ways to deal with headers larger than 256 bytes.

Wrt to the Read(b) contract, the Go documentation is clear that it should return data available and not wait for b to be full. I will sleep over this and come back to you very soon.

io.ReadFull would return ErrUnexpectedEOF if there's an EOF before my buffer is filled.

IIUC EOF would only be returned if the underlying net.Conn.Read(b) returns EOF, right?

That said, I think we're trying to address different challenges:

1. stop buffering the underlying conn after the the header is processed and the buffer is drained (fixed in this PR)

2. strictly follow io.Reader contract or deliver a different undertanding, ie Read(b) will block until b is full or EOF is returned

3. increase the buffer size statically (eg Bump maximum header size to 4KiB, alternative version #155) or dinamically (protocol: safely resize header buffer based on length field #150) Bump maximum header size to 4KiB, alternative version #155 IS NOT about resizing the buffer BUT it does increase the v2 header size limit to 4KB. I have closed protocol: safely resize header buffer based on length field #150

This PR is meant to fix the 1 alone. Meanwhile, I enabled discussions for this repo so we can have a discussion wrt 2. cc @emersion

Created discussion #157 for debating 2.