Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix decoding of packet numbers in different packet number spaces #2903

Merged
merged 1 commit into from
Dec 4, 2020

Conversation

marten-seemann
Copy link
Member

Fixes #2902.

This would also justify a new release.

@codecov
Copy link

codecov bot commented Nov 23, 2020

Codecov Report

Merging #2903 (9533420) into master (d161d7e) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2903      +/-   ##
==========================================
+ Coverage   85.77%   85.82%   +0.04%     
==========================================
  Files         133      133              
  Lines        9187     9216      +29     
==========================================
+ Hits         7880     7909      +29     
  Misses        959      959              
  Partials      348      348              
Impacted Files Coverage Δ
internal/flowcontrol/base_flow_controller.go 100.00% <ø> (ø)
internal/flowcontrol/connection_flow_controller.go 100.00% <100.00%> (ø)
internal/flowcontrol/stream_flow_controller.go 100.00% <100.00%> (ø)
internal/handshake/aead.go 97.92% <100.00%> (+0.14%) ⬆️
internal/handshake/updatable_aead.go 96.32% <100.00%> (+0.08%) ⬆️
packet_unpacker.go 89.15% <100.00%> (+3.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed1956f...9533420. Read the comment docs.

When QUIC abandoned the "stream 0" design, it introduced separate
packet number spaces for packets with different encryption levels.
Packet number compression now also works per packet number space.
The current code doesn't lead to any problems if the peer starts sending
with packet number 0, as we only exchange a few packets in the Initial
and the Handshake packet number space and there's nothing to compress.
It might lead to problems if the peer starts with a large packet number
in one space (which is allowed by the spec), and then starts with a
small packet number in another packet number space.
Copy link

@mvdan mvdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand the issue and the idea behind the fix, so SGTM. Thanks for the commit message. Though this change is relatively large and I'm not familiar enough with the codebase to fully say that I think it's correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unpacker is unaware of packet number spaces
2 participants