-
Notifications
You must be signed in to change notification settings - Fork 155
TLS Transfer Serialization Improvements #2616
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 10 out of 13. Check the log or trigger a new build to see more.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2616 +/- ##
==========================================
- Coverage 78.75% 78.73% -0.02%
==========================================
Files 663 663
Lines 113346 113622 +276
Branches 15946 15974 +28
==========================================
+ Hits 89264 89464 +200
- Misses 23304 23381 +77
+ Partials 778 777 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
85b387d to
530f170
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
17967b3 to
68f4ff2
Compare
68f4ff2 to
3fbe701
Compare
3fbe701 to
591bcb4
Compare
591bcb4 to
28b0b1a
Compare
| if (data_start_ > end_ptr || data_start_ < start_ptr || | ||
| data_end_ptr > end_ptr || data_end_ptr < start_ptr || size_ > cap_ || | ||
| remaining_ptr > end_ptr || remaining_ptr < start_ptr) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can any of these potentially be equal?
| #define GUARD_SUSPENDED_STATE(ptr,code) \ | ||
| do { \ | ||
| if (ptr->is_suspended_state) { \ | ||
| OPENSSL_PUT_ERROR(SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); \ | ||
| return code; \ | ||
| } \ | ||
| } while (0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NP: Use parentheses: (ptr)->is_suspended_state.
Addresses CryptoAlg-3221, CryptoAlg-3220, CryptoAlg-3219, CryptoAlg-3218, CryptoAlg-3217, CryptoAlg-3216, CryptoAlg-3215, CryptoAlg-3214, CryptoAlg-3212, CryptoAlg-3211 This pull request addresses and improves the experimental TLS transfer serialization feature. Primarily focusing on performance of restored connections for the SSLBuffer by improving the serialization format, and hardens the checking of the data structures on deserialization. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license. (cherry picked from commit 26da017)
Issues:
Addresses CryptoAlg-3221, CryptoAlg-3220, CryptoAlg-3219, CryptoAlg-3218, CryptoAlg-3217, CryptoAlg-3216, CryptoAlg-3215, CryptoAlg-3214, CryptoAlg-3212, CryptoAlg-3211
Description of changes:
This pull request addresses and improves the experimental TLS transfer serialization feature. Primarily focusing on performance of restored connections for the SSLBuffer by improving the serialization format, and hardens the checking of the data structures on deserialization.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.