fix: check push id before decoding a blocked push promise#3819
Open
uwezkhan wants to merge 1 commit into
Open
Conversation
uwezkhan
requested review from
KershawChang,
larseggert,
martinthomson and
mxinden
as code owners
July 17, 2026 17:40
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3819 +/- ##
==========================================
- Coverage 96.93% 96.80% -0.13%
==========================================
Files 115 120 +5
Lines 40516 40873 +357
Branches 40516 40873 +357
==========================================
+ Hits 39275 39568 +293
- Misses 1226 1280 +54
- Partials 15 25 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 7.59%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | simulated/10-streams/each-1048576-bytes |
246.9 ms | 229.5 ms | +7.59% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing uwezkhan:push-id-before-decode (1335a8e) with main (9ec0cc6)
Contributor
Benchmark resultsNo significant performance differences relative to 9ec0cc6. All resultsstreams/walltime/1-streams/each-1000-bytes: No change in performance detected. time: [543.12 µs 544.91 µs 546.98 µs]
thrpt: [1.7435 MiB/s 1.7502 MiB/s 1.7559 MiB/s]
change:
time: [-0.5025% -0.0263% +0.3900] (p = 0.91 > 0.05)
thrpt: [-0.3885% +0.0263% +0.5051]
No change in performance detected.
Found 11 outliers among 100 measurements (11.00%)
3 (3.00%) high mild
8 (8.00%) high severestreams/walltime/1000-streams/each-1-bytes: Change within noise threshold. time: [11.004 ms 11.020 ms 11.036 ms]
thrpt: [88.488 KiB/s 88.621 KiB/s 88.749 KiB/s]
change:
time: [-0.7451% -0.5283% -0.3083] (p = 0.00 < 0.05)
thrpt: [+0.3093% +0.5311% +0.7507]
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildstreams/walltime/1000-streams/each-1000-bytes: Change within noise threshold. time: [32.064 ms 32.103 ms 32.143 ms]
thrpt: [29.670 MiB/s 29.707 MiB/s 29.743 MiB/s]
change:
time: [-0.8171% -0.6291% -0.4509] (p = 0.00 < 0.05)
thrpt: [+0.4530% +0.6331% +0.8238]
Change within noise threshold.streams-flow-controlled/walltime/1-streams/each-4194304-bytes: Change within noise threshold. time: [27.380 ms 27.416 ms 27.452 ms]
thrpt: [145.71 MiB/s 145.90 MiB/s 146.09 MiB/s]
change:
time: [+0.1592% +0.3507% +0.5536] (p = 0.00 < 0.05)
thrpt: [-0.5505% -0.3494% -0.1589]
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) low mild
1 (1.00%) high mildstreams-flow-controlled/walltime/10-streams/each-1048576-bytes: No change in performance detected. time: [71.890 ms 72.369 ms 72.870 ms]
thrpt: [137.23 MiB/s 138.18 MiB/s 139.10 MiB/s]
change:
time: [-0.4964% +0.4042% +1.3214] (p = 0.39 > 0.05)
thrpt: [-1.3042% -0.4026% +0.4989]
No change in performance detected.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/walltime/pacing-false/varying-seeds: Change within noise threshold. time: [18.288 ms 18.300 ms 18.311 ms]
thrpt: [218.45 MiB/s 218.58 MiB/s 218.72 MiB/s]
change:
time: [-1.4481% -1.2339% -1.0918] (p = 0.00 < 0.05)
thrpt: [+1.1039% +1.2493% +1.4694]
Change within noise threshold.transfer/walltime/pacing-true/varying-seeds: Change within noise threshold. time: [18.606 ms 18.617 ms 18.628 ms]
thrpt: [214.73 MiB/s 214.86 MiB/s 214.99 MiB/s]
change:
time: [-3.0109% -2.7852% -2.6235] (p = 0.00 < 0.05)
thrpt: [+2.6942% +2.8650% +3.1044]
Change within noise threshold.
Found 17 outliers among 100 measurements (17.00%)
1 (1.00%) low severe
6 (6.00%) low mild
7 (7.00%) high mild
3 (3.00%) high severetransfer/walltime/pacing-false/same-seed: Change within noise threshold. time: [18.193 ms 18.204 ms 18.215 ms]
thrpt: [219.60 MiB/s 219.73 MiB/s 219.86 MiB/s]
change:
time: [-1.4306% -1.3461% -1.2597] (p = 0.00 < 0.05)
thrpt: [+1.2758% +1.3645% +1.4513]
Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) low mild
1 (1.00%) high mild
1 (1.00%) high severetransfer/walltime/pacing-true/same-seed: Change within noise threshold. time: [18.735 ms 18.745 ms 18.755 ms]
thrpt: [213.27 MiB/s 213.39 MiB/s 213.50 MiB/s]
change:
time: [-1.4506% -1.3839% -1.3105] (p = 0.00 < 0.05)
thrpt: [+1.3279% +1.4033% +1.4720]
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildDownload data for |
Contributor
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
|
Contributor
Client/server transfer resultsPerformance differences relative to 9ec0cc6. Transfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Table above only shows statistically significant changes. See all results below. All resultsTransfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Download data for |
larseggert
approved these changes
Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
handle_push_promise checks the Push ID only through new_push_promise, which runs after the QPACK header block decodes. Before this change, a header block that refers to a dynamic table entry the server has not delivered yet blocks the decoder, so the frame lands on blocked_push_promise with no check, and a server that keeps the decoder blocked can push any Push ID unvalidated. RFC 9114 Section 7.2.5 wants a Push ID above the maximum treated as H3_ID_ERROR.
After, check_push_id runs at the top of handle_push_promise, before any decode or buffering, so the error is raised as soon as the frame arrives whatever the decoder state. The check stays in the push controller since that is where the maximum lives; the tradeoff is that a valid Push ID is now checked twice, once here and again in new_push_promise after unblocking. In-range Push IDs behave as before.