Skip to content

[breaking change] Change SecurityContext to an abstract final class #55786

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

Closed
brianquinlan opened this issue May 20, 2024 · 7 comments
Closed
Assignees
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. breaking-change-approved breaking-change-request This tracks requests for feedback on breaking changes library-io

Comments

@brianquinlan
Copy link
Contributor

brianquinlan commented May 20, 2024

Change Intent

The intent is to change the dart:io SecurityContext class, used to control the security aspects of a socket connection, from an interface class to an abstract final class.

Justification

A user-implemented SecurityContext cannot be used in place of the dart:io implementation because the Dart TLS implementation depends on C++ data structures found only in its implementation.

Making SecurityContext final also makes it easier for the Dart team to modify in the future e.g. #55679

Impact

This change will break anyone who implements SecurityContext. On GitHub, I only found two such instances, and both were in tests that just raise if a SecurityContext method is called e.g. https://github.com/platform-platform/universal_io/blob/37e2dfcae32156b28477365e9dcf357d583098e5/packages/universal_io/lib/src/driver/driver_impl_vm.dart#L499

Mitigation

Developers must modify (or remove) all classes that implement SecurityContext.

Change Timeline

Remove in Dart 3.5

Associated CLs

No response

@kevmoo
Copy link
Member

kevmoo commented May 20, 2024

SGTM!

@itsjustkevin
Copy link
Contributor

@vsmenon @Hixie @leonsenft fresh breaking change request

@leonsenft
Copy link

LGTM

@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels May 20, 2024
@Hixie
Copy link
Contributor

Hixie commented May 21, 2024

seems reasonable

@a-siva
Copy link
Contributor

a-siva commented May 29, 2024

@vsmenon can you look at this breaking change and decide.

@brianquinlan
Copy link
Contributor Author

@vsmenon I'm going to assume that we are good with this ;-)

@itsjustkevin
Copy link
Contributor

@brianquinlan i am marking this breaking change as approved.

copybara-service bot pushed a commit that referenced this issue Jul 1, 2024
Bug:#55786
Change-Id: I351ddb7f39d2ac9ee032bdf88ef6d524c97f8402
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370703
Reviewed-by: Sigmund Cherem <[email protected]>
Reviewed-by: Leaf Petersen <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Brian Quinlan <[email protected]>
@github-project-automation github-project-automation bot moved this from In review to Complete in Breaking Changes Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. breaking-change-approved breaking-change-request This tracks requests for feedback on breaking changes library-io
Projects
Status: Complete
Development

No branches or pull requests

7 participants