search

socketry / protocol-http2

MIT License
9 stars 10 forks source link

maximum concurrent stream is defined only by the local settings #9

Closed Maaarcocr closed 2 years ago

Maaarcocr commented 2 years ago

Types of Changes

Bug fix. Resolves #8

ioquatix commented 2 years ago

It would be good to add a spec to this to prevent a regression.

Maaarcocr commented 2 years ago

It would be good to add a spec to this to prevent a regression.

I'm looking at the connection spec and I was thinking of reusing some of the test logic here: https://github.com/socketry/protocol-http2/blob/014f415df03b7058488126bd899ce90f893d986e/spec/protocol/http2/connection_spec.rb#L82 while also setting the maximum concurrent connection setting. I'm just not sure how to set it. Is it possible? If so, how?

ioquatix commented 2 years ago

You should be able to access the settings directly:

https://github.com/socketry/protocol-http2/blob/main/lib/protocol/http2/connection.rb#L88-L90

Then you can set it yourself:

https://github.com/socketry/protocol-http2/blob/main/lib/protocol/http2/settings_frame.rb#L52-L53

Would be good to test both positive and negative cases to prevent regressions.

Maaarcocr commented 2 years ago

@ioquatix tests added!