fix: Don't allow sending path status frames on non-multipath connections#191
fix: Don't allow sending path status frames on non-multipath connections#191
Conversation
d3126e7 to
ca1b4ff
Compare
|
needs update/rebase |
flub
left a comment
There was a problem hiding this comment.
-1 on all the renames to be fair. But it seems I'm alone on this 🤷
quinn-proto/src/connection/mod.rs
Outdated
| path_id: PathId, | ||
| timeout: Option<Duration>, | ||
| ) -> Result<Option<Duration>, ClosedPath> { | ||
| ) -> Result<Option<Duration>, NotOpen> { |
There was a problem hiding this comment.
Hum, should this error if multipath is not negotiated? I guess it's not wrong right now, but a little weird. Let's leave it for now maybe.
There was a problem hiding this comment.
Is this something that causes something to be sent over the wire? If so, then yes this should return an error if multipath is not negotiated.
If not, then I think that's optional.
Let me expand on this. I think the state of a path is not very clearly defined in the multipath draft, and therefore folks (including me) do not have a clear view of the states. Essentially at any point it is valid to send a packet for a path_id as long as the path_id is below the current max path ID and not yet abandoned. That means paths don't have a clearly defined "now it's open" state. A non-abandoned path below the current max path ID is always kinda open. And you can get path errors on paths you didn't know had been "active" yet. |
|
The main goal of this PR is to get the bug fix in. The renames just pollute the PR (I introduced them originally because I ended up creating a conflicting Let's get this reviewed for the bug fix not for the renames. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main-iroh #191 +/- ##
=============================================
- Coverage 76.38% 76.38% -0.01%
=============================================
Files 83 83
Lines 22281 22286 +5
=============================================
+ Hits 17020 17023 +3
- Misses 5261 5263 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.