Skip to content
This repository has been archived by the owner on Aug 29, 2024. It is now read-only.

Add functionality to be able to peek n bytes off the wire #72

Merged
merged 5 commits into from
Nov 9, 2023

Conversation

maruth-stripe
Copy link
Contributor

@maruth-stripe maruth-stripe commented Nov 6, 2023

To fix socketry/protocol-http2#14 , one approach is to maintain the invariant that the read buffer / wire is always frame-aligned. We propose doing this by always reading a full frame at a time. To do this, 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

  • New feature.

Contribution

@maruth-stripe maruth-stripe marked this pull request as draft November 6, 2023 19:25
@ioquatix
Copy link
Member

ioquatix commented Nov 6, 2023

This is a great idea, I like it.

@ioquatix
Copy link
Member

ioquatix commented Nov 6, 2023

Do you mind adding some tests?

@maruth-stripe
Copy link
Contributor Author

Yup I'll get on it! thanks for the feedback

@ioquatix
Copy link
Member

ioquatix commented Nov 7, 2023

I'll also fix CI. Looks like there are some issues with Async v1 / Ruby 2.7.

@maruth-stripe maruth-stripe marked this pull request as ready for review November 7, 2023 01:38
@maruth-stripe
Copy link
Contributor Author

@ioquatix added a couple of tests!

@ioquatix
Copy link
Member

ioquatix commented Nov 7, 2023

Are you able to agree to the developer certificate of origin?

@maruth-stripe
Copy link
Contributor Author

@ioquatix yup did it! Sorry for multiple pushes, unable to get tests run locally due to environment issues

@maruth-stripe
Copy link
Contributor Author

ah managed to get tests running locally, they pass for me now!

@maruth-stripe
Copy link
Contributor Author

@ioquatix seems like the same (unrelated?) failures for 2.7/v1, any action needed from me?

@ioquatix
Copy link
Member

ioquatix commented Nov 9, 2023

No I will sort this out on the weekend, LGTM.

@ioquatix ioquatix merged commit 81c6728 into socketry:main Nov 9, 2023
dentarg added a commit to dentarg/sinatra that referenced this pull request Nov 11, 2023
Due to socketry/async-io#72 released in async-io 1.37.0

We will need this fix as long as we want to test Falcon in Ruby 2.6, as
the "fix" in async-io will be to raise the min Ruby version:
socketry/async-io#74
dentarg added a commit to sinatra/sinatra that referenced this pull request Nov 11, 2023
Due to socketry/async-io#72 released in async-io
1.37.0

We will need this fix as long as we want to test Falcon in Ruby 2.6, as
the "fix" in async-io will be to raise the min Ruby version:
socketry/async-io#74

Fail can be seen at
https://github.com/sinatra/sinatra/actions/runs/6836408460/job/18591270458#step:5:19
ioquatix added a commit that referenced this pull request Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants