fix: Accept or reject incoming frames based on self.local_max_path_id instead of Connection::max_path_id
#183
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a previously failing test case that best illustrates the problem.
Here's a picture anyways:

When the server receives a
MAX_PATH_IDframe and agrees to increase it, it'll increase it after sending aMAX_PATH_IDframe itself.Now, from the server's perspective the maximum path ID might be set to 14 (a value I saw in practice).
Then it starts issuing NEW_CONNECTION_ID frames with path IDs up to what it thinks is the max path id.
However, these two datagrams might get reordered. This is what we synthetically make happen in the test
issue_max_path_id_reordered.Then, the client will receive newer NEW_CONNECTION_ID frames while it still thinks the max path ID might be something like 12 and promptly close the connection with a protocol violation.
The fix I propose is to be more lenient when reading incoming packets. The
NEW_CONNECTION_IDframe we got IMO is fine because we've already sent out aMAX_PATH_IDframe with 14. The fact that the other side hasn't sent us aMAX_PATH_IDframe is kind of secondary.The other possible fix would be to delay the remote using any higher path IDs until it made sure the
MAX_PATH_IDframe was acknowledged. I think this adds unnecessary delays.The specification doesn't really seem to agree with me on this.
Section 4 says:
I interpret the
MAX_PATH_IDframes as the ones that are received, due to it referring to having received them or not in the next sentence.This means the situation from the picture above would be the connection's flow according to the specification. (But I think that's a specification bug.)