| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
This ensures we don't fire session events for totally invalid TLS handshakes - fundamental errors, bad SNI/ALPN values, or anything else that our TLS config would reject. Instead, it means servers can access servername & alpnProtocol synchronously as soon as the event is fired - all key session data is available and it's immediately usable. We don't want to defer further to handshake completed, since that'd be an extra RT, and defeat 0RTT benefits entirely. ClientHello processed without errors is sufficient for now. This isn't a security mechanism. Existing structures will defer actually sending & receiving anything that's not marked explicitly as early data until the handshake completes anyway. Signed-off-by: Tim Perry <pimterry@gmail.com>
|
Review requested:
|
Sorry, something went wrong.
| auto emits = std::move(impl_->deferred_emits_); | ||
| for (auto& emit : emits) { | ||
| if (is_destroyed()) return; | ||
| emit(); |
There was a problem hiding this comment.
Since each of these is a C++-to-JavaScript call, I wonder if it would be possible to batch them to a new callback. Essentially, rather than enqueuing a function, enqueue the arguments and type of emit, send them all to JavaScript at once and process each one there instead. Calls from C++-to-JavaScript can be a bit expensive to set up.
Sorry, something went wrong.
There was a problem hiding this comment.
This can be looked at later tho.
Sorry, something went wrong.
There was a problem hiding this comment.
I've left this for now. It looks likely to be a little fiddly, and I think it rarely comes up (just a couple of 0RTT events, or keylog/qlog when they're enabled) but we can explore as an optimization later.
Sorry, something went wrong.
Sorry, something went wrong.
…ssed Cache ALPN & servername once set, update field types and docs.
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #64132 +/- ##
==========================================
- Coverage 92.01% 90.24% -1.78%
==========================================
Files 379 741 +362
Lines 166972 241037 +74065
Branches 25554 45411 +19857
==========================================
+ Hits 153639 217513 +63874
- Misses 13041 15108 +2067
- Partials 292 8416 +8124
... and 543 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
Sorry, something went wrong.
|
@jasnell when you have a sec, I'd love a re-review here to land this. That last commit is just fixes for the points from your review. |
Sorry, something went wrong.
This ensures we don't fire session events for totally invalid TLS handshakes - fundamental errors, bad SNI/ALPN values, or anything else that our TLS config would reject. Instead, it means servers can access servername & alpnProtocol synchronously as soon as the event is fired - all key session data is available and it's immediately usable. We don't want to defer further to handshake completed, since that'd be an extra RT, and defeat 0RTT benefits entirely. ClientHello processed without errors is sufficient for now. This isn't a security mechanism. Existing structures will defer actually sending & receiving anything that's not marked explicitly as early data until the handshake completes anyway. Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: #64132 Reviewed-By: James M Snell <jasnell@gmail.com>
This ensures we don't fire session events for totally invalid TLS handshakes - fundamental errors, bad SNI/ALPN values, or anything else that our TLS config would reject. Instead, it means servers can access servername & alpnProtocol synchronously as soon as the event is fired - all key session data is available and it's immediately usable. We don't want to defer further to handshake completed, since that'd be an extra RT, and defeat 0RTT benefits entirely. ClientHello processed without errors is sufficient for now. This isn't a security mechanism. Existing structures will defer actually sending & receiving anything that's not marked explicitly as early data until the handshake completes anyway. Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: #64132 Reviewed-By: James M Snell <jasnell@gmail.com>
| Back | FazBrowse Home | New Git URL |
Extracted from #63995. It's not identical to the implementation there, I did a little more cleanup and refactoring here, and dropped some parts that aren't relevant without the latest changes in that PR, but it's reviewable standalone.
This change means servers can access servername & alpnProtocol synchronously as soon as the event is fired - all key session data is available and it's immediately usable. New getters are exposed for that. This also ensures we don't fire session events for totally invalid TLS handshakes - fundamental errors, bad SNI/ALPN values, or anything else that our TLS config would reject.
It does so without waiting for any more round trips that before, in any normal flow: it just defers the session event to the end of the client hello processing which should be momentarily after the previous behaviour. In very strange flows (if the first flight from the client doesn't contain the full client hello) then this could introduce a more significant delay, but AFAICT no normal client should ever do that (and we'd have to do some kind of wait regardless eventually if they did - we can't really do anything useful without a complete client hello).
This is just intended to improve UX and smooth the path towards dynamic Application selection (as used in #63995). A few things this doesn't do: