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.

Sign up for GitHub

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

Jump to bottom

Kuba/ssl/client ocsp/otp 18606 #7543

Merged
merged 2 commits into from
Jan 29, 2024
Merged

Kuba/ssl/client ocsp/otp 18606 #7543

merged 2 commits into from
Jan 29, 2024

Conversation

u3s
Copy link
Contributor

@u3s u3s commented Aug 8, 2023

No description provided.

@u3s u3s added the team:PS Assigned to OTP team PS label Aug 8, 2023
@u3s u3s self-assigned this Aug 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023 •
edited
Loading

CT Test Results

    3 files     73 suites   50m 15s ⏱️
  973 tests   940 ✅  33 💤 0 ❌
3 942 runs  3 167 ✅ 775 💤 0 ❌

Results for commit 8e20f46.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@u3s u3s requested review from dgud and IngelaAndin August 9, 2023 11:03
IngelaAndin
IngelaAndin requested changes Aug 10, 2023
View reviewed changes
lib/public_key/doc/src/public_key.xml Show resolved Hide resolved
lib/public_key/test/public_key_SUITE.erl Outdated Show resolved Hide resolved
lib/ssl/test/openssl_stapling_SUITE.erl Show resolved Hide resolved
lib/ssl/src/ssl.erl Outdated Show resolved Hide resolved
lib/public_key/src/pubkey_ocsp.erl Outdated Show resolved Hide resolved
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from 0ab5a34 to 077abb4 Compare August 14, 2023 17:53
dgud
dgud reviewed Aug 16, 2023
View reviewed changes
Copy link
Contributor

@dgud dgud left a comment

Choose a reason for hiding this comment

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

Should we handle "-type combined_cert() :: #cert{}." in the certificate(s) in the arguments?
To reduce the decode/encode der roundtrips that is done?

lib/public_key/src/pubkey_ocsp.erl Outdated Show resolved Hide resolved
lib/public_key/src/pubkey_ocsp.erl Outdated Show resolved Hide resolved
lib/ssl/src/ssl.erl Outdated Show resolved Hide resolved
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch 4 times, most recently from 4b2e6c9 to 57e0c18 Compare August 21, 2023 14:54
@u3s
Copy link
Contributor Author

u3s commented Aug 21, 2023

Should we handle "-type combined_cert() :: #cert{}." in the certificate(s) in the arguments? To reduce the decode/encode der roundtrips that is done?

hmm, did you mean public_key:pkix_ocsp_validate or some other context?
i've cleaned up code for that function in 57e0c1. I don't think using combined_cert improves anything now ... but maybe I'm missing something?

@IngelaAndin IngelaAndin added the Planned Focus issue added in sprint planning label Aug 25, 2023
@u3s
Copy link
Contributor Author

u3s commented Sep 6, 2023

Should we handle "-type combined_cert() :: #cert{}." in the certificate(s) in the arguments? To reduce the decode/encode der roundtrips that is done?

I'm starting to understand and like your comment :-).
But are you suggesting this PR context or more general?

@u3s u3s added this to the OTP-27.0 milestone Sep 20, 2023
@u3s u3s changed the base branch from maint to master September 25, 2023 12:32
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from 6d588b1 to 30e7203 Compare September 25, 2023 12:32
@u3s u3s added the testing currently being tested, tag is used by OTP internal CI label Sep 25, 2023
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from 121acd6 to 03b422b Compare September 27, 2023 14:36
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from 03b422b to 97c956c Compare November 8, 2023 09:34
@u3s u3s added testing currently being tested, tag is used by OTP internal CI and removed testing currently being tested, tag is used by OTP internal CI labels Nov 8, 2023
@u3s u3s requested review from IngelaAndin and dgud November 8, 2023 11:39
@IngelaAndin
Copy link
Contributor

@u3s you also need to rebase!

@u3s u3s removed the testing currently being tested, tag is used by OTP internal CI label Nov 21, 2023
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from 97c956c to ee1911e Compare November 21, 2023 16:40
dgud
dgud approved these changes Jan 8, 2024
View reviewed changes
lib/public_key/src/public_key.erl Show resolved Hide resolved
lib/ssl/src/ssl_certificate.erl Outdated Show resolved Hide resolved
@IngelaAndin IngelaAndin removed the stalled waiting for input by the Erlang/OTP team label Jan 9, 2024
@IngelaAndin
Copy link
Contributor

Maybe squash some of the commits?

u3s
u3s commented Jan 18, 2024
View reviewed changes
lib/ssl/doc/src/ssl.xml Outdated Show resolved Hide resolved
u3s
u3s commented Jan 18, 2024
View reviewed changes
lib/ssl/src/ssl_handshake.erl Outdated Show resolved Hide resolved
u3s
u3s commented Jan 19, 2024
View reviewed changes
lib/ssl/src/tls_handshake.erl Outdated Show resolved Hide resolved
u3s
u3s commented Jan 19, 2024
View reviewed changes
lib/ssl/src/ssl_handshake.erl Outdated Show resolved Hide resolved
u3s
u3s commented Jan 19, 2024
View reviewed changes
lib/ssl/src/ssl_handshake.erl Outdated Show resolved Hide resolved
u3s
u3s commented Jan 19, 2024
View reviewed changes
lib/ssl/src/ssl_handshake.erl Outdated Show resolved Hide resolved
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from 4d7f2c5 to ca6763f Compare January 25, 2024 12:32
@u3s
Copy link
Contributor Author

u3s commented Jan 25, 2024 •
edited
Loading

I've pushed updated branch. fixup commits contain updates since preview review. I will squash them when we finish discussions.

@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from ca6763f to 7dd44bb Compare January 25, 2024 12:43
@u3s
Copy link
Contributor Author

u3s commented Jan 25, 2024 •
edited
Loading

Should we handle "-type combined_cert() :: #cert{}." in the certificate(s) in the arguments? To reduce the decode/encode der roundtrips that is done?

I created backlog item, to handle this area(optimize encoding/decoding) in separate PR. Plan is to do it soon.

@u3s u3s requested a review from IngelaAndin January 26, 2024 06:24
@IngelaAndin IngelaAndin self-requested a review January 26, 2024 10:37
@u3s u3s force-pushed the kuba/ssl/client_ocsp/OTP-18606 branch from 9945e7d to 8e20f46 Compare January 29, 2024 10:23
@u3s u3s merged commit e6cc808 into erlang:master Jan 29, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IngelaAndin IngelaAndin IngelaAndin approved these changes

@dgud dgud dgud approved these changes

Assignees

@u3s u3s

Labels
Planned Focus issue added in sprint planning team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Milestone
OTP-27.0
Development

Successfully merging this pull request may close these issues.
3 participants
@u3s @IngelaAndin @dgud