Ensure wire always contains a full H/2 frame

[maruth-stripe]

Follow-up to socketry/async-io#72 to fix#14
Updates read_header to use Stream#peek instead of `Stream#read. We maintain the invariant that the read buffer / wire is always frame-aligned. Instead of reading data off the wire when reading the header for an H/2 frame, we just peek the header and then read the full frame.

Types of Changes

• Bug fix.

Contribution

• I added tests for my changes.
• I tested my changes locally.
• I agree to the Developer's Certificate of Origin 1.1.

[maruth-stripe]

Verified the fix works with the following script (modification to the repro in #14)

# requiresrequire"stringio"require"async/reactor"classFunkyIOdefinitialize@f=Protocol::HTTP2::DataFrame.new(401,0,Protocol::HTTP2::DataFrame::TYPE,13,"a" * 13)@sio=StringIO.new@f.write_header(@sio)@sio.rewind@state=:read_headerenddefpeek(size,buf=nil)if@state == :read_header || @state == :try_header_againres=@sio.read(size,buf)case@statewhen:read_header@state=:now_timeoutwhen:try_header_again@state=:read_frameend@sio=StringIO.new@f.write_header(@sio)@f.write_payload(@sio)@sio.rewindreturnresendenddefread(size,buf=nil)case@statewhen:now_timeout@state=:try_header_againraiseAsync::TimeoutErrorwhen:read_frameres=@sio.read(size,buf)resendendendf=Protocol::HTTP2::Framer.new(FunkyIO.new)Async::Reactor.rundobeginp(f.read_frame)rescueAsync::TimeoutError# try againendp(f.read_frame)end

outputs
#<Protocol::HTTP2::DataFrame stream_id=401 flags=0 13b>

[ioquatix]

I released async-io v1.37.0 which I assume we can use here?

[maruth-stripe]

Hi @ioquatix really sorry for disappearing on this PR for a bit, got caught up with other stuff!

I've updated it a bit, and tested it with the follwing script:

# requiresrequire"stringio"require"async"require"async/reactor"require"protocol/http2/framer"require"protocol/http2/data_frame"classFunkyIOdefinitialize@f=Protocol::HTTP2::DataFrame.new(401,0,Protocol::HTTP2::DataFrame::TYPE,13,"a" * 13)@sio=StringIO.new@f.write_header(@sio)@sio.rewind@state=:read_headerenddefsync=(x)enddefread_nonblock(size,buf=nil,exception)puts"read_nonblock(#{size}) state: #{@state}"if@state == :read_header || @state == :try_header_againres=@sio.read(size,buf)case@statewhen:read_header@state=:now_timeoutwhen:try_header_again@state=:read_frameend@sio=StringIO.new@f.write_payload(@sio)@sio.rewindreturnreselsif@state == :now_timeout@state=:try_header_againraiseAsync::TimeoutErrorelsif@state == :read_framereturn@sio.read(size,buf)endendendf=Protocol::HTTP2::Framer.new(FunkyIO.new)Async::Reactor.rundobeginp(f.read_frame)rescueAsync::TimeoutError# try againendp(f.read_frame)end

which outputs

read_nonblock(65536) state: read_header
read_nonblock(65536) state: now_timeout
read_nonblock(65536) state: try_header_again
#<Protocol::HTTP2::DataFrame stream_id=401 flags=0 13b>

I'm not sure why but bundle exec bake test seems to be taking quite a while. Unsure if it's something messed up with my local setup.

[maruth-stripe]

One note: this PR now implicitly adds a requirement that the stream passed into Framer must implement sync= and read_nonblock (or be an `Async::IO::Stream)

I believe StringIO, Socket do implement this.

[maruth-stripe]

@ioquatix do you know why the test suite might be timing out here?

[ioquatix]

The next step for this code is to work with standard IO instances. However, I don't think a method like peek exists for IO. Do you have any ideas?

[ioquatix]

After considering this further, I believe we should adopt the peek(n) model in this code. We will have to use an appropriate wrapper.

[ioquatix]

I'm going to merge this into another branch so I can finish it off.