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(ext/net): implement a graceful error on an invalid SSL certificate #20157

Merged
merged 10 commits into from
Aug 15, 2023

Conversation

0xIchigo
Copy link
Contributor

The goal of this PR is to address issue #19520 where Deno panics when encountering an invalid SSL certificate.

This PR achieves that goal by removing an .expect() statement and implementing a match statement on tsl_config (found in /ext/net/ops_tsl.rs) to check whether the desired configuration is valid

@CLAassistant
Copy link

CLAassistant commented Aug 14, 2023

CLA assistant check
All committers have signed the CLA.

@0xIchigo 0xIchigo changed the title Implement Graceful Error on Invalid SSL Cert Instead of Panic fix(ext/net): implement a graceful error on an invalid SSL certificate Aug 14, 2023
ext/net/ops_tls.rs Outdated Show resolved Hide resolved
ext/net/ops_tls.rs Outdated Show resolved Hide resolved
@mmastrac
Copy link
Contributor

@0xIchigo Looks like the lint failed -- run tools/format.ts and that should fix it.

@0xIchigo
Copy link
Contributor Author

@mmastrac My apologies, fixed with this commit !

@0xIchigo 0xIchigo requested a review from mmastrac August 14, 2023 23:27
@mmastrac
Copy link
Contributor

@0xIchigo Looks like the tests failed, but it's not clear why. I'll take a quick look to figure out what's broken.

Copy link
Contributor

@mmastrac mmastrac left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your contribution. The tests should pass and it'll merge automatically once it's done testing.

@mmastrac mmastrac enabled auto-merge (squash) August 14, 2023 23:35
@0xIchigo
Copy link
Contributor Author

Thank you so much @mmastrac for all your help! I'll be on the lookout for any other code I can contribute to in the future 🔥

@mmastrac mmastrac merged commit ece2a3d into denoland:main Aug 15, 2023
13 checks passed
littledivy pushed a commit to littledivy/deno that referenced this pull request Aug 21, 2023
denoland#20157)

The goal of this PR is to address issue denoland#19520 where Deno panics when
encountering an invalid SSL certificate.

This PR achieves that goal by removing an `.expect()` statement and
implementing a match statement on `tsl_config` (found in
[/ext/net/ops_tsl.rs](https://github.com/denoland/deno/blob/e071382768fa57b5288a6a5ba90e73bf5870b169/ext/net/ops_tls.rs#L1058))
to check whether the desired configuration is valid

---------

Co-authored-by: Matt Mastracci <[email protected]>
littledivy pushed a commit that referenced this pull request Aug 21, 2023
#20157)

The goal of this PR is to address issue #19520 where Deno panics when
encountering an invalid SSL certificate.

This PR achieves that goal by removing an `.expect()` statement and
implementing a match statement on `tsl_config` (found in
[/ext/net/ops_tsl.rs](https://github.com/denoland/deno/blob/e071382768fa57b5288a6a5ba90e73bf5870b169/ext/net/ops_tls.rs#L1058))
to check whether the desired configuration is valid

---------

Co-authored-by: Matt Mastracci <[email protected]>
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.

3 participants