Skip to content

Add support for rustls-platform-verifier#528

Open
FrankenApps wants to merge 1 commit intosnapview:masterfrom
FrankenApps:rustls-platform-verifier
Open

Add support for rustls-platform-verifier#528
FrankenApps wants to merge 1 commit intosnapview:masterfrom
FrankenApps:rustls-platform-verifier

Conversation

@FrankenApps
Copy link
Copy Markdown

Adds support for rustls-platform-verifier as mentioned in #522.

However contrary to the plan outlined in that issue I left the rustls-native-certs feature untouched for now.

I also did not implement support for additional root certificates (new_with_extra_roots), because it works differently than the current approach based on RootCertStore.

@FrankenApps FrankenApps force-pushed the rustls-platform-verifier branch from 22d57a0 to b94dfb0 Compare February 25, 2026 18:05
@FrankenApps
Copy link
Copy Markdown
Author

@daniel-abramov Is this something you would consider?

@FrankenApps
Copy link
Copy Markdown
Author

CI failure seems unrelated:

error: package `syn v2.0.117` cannot be built because it requires rustc 1.71 or newer, while the currently active rustc version is 1.68.2
Either upgrade to rustc 1.71 or newer, or use
cargo update -p syn@2.0.117 --precise ver
where `ver` is the latest version of `syn` supporting rustc 1.68.2
Error: Process completed with exit code 101.

Copy link
Copy Markdown
Member

@daniel-abramov daniel-abramov left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@daniel-abramov Is this something you would consider?

Yes. I think it's the right direction in the long term.

The only concern I have with these particular changes is that cargo features are meant to be additive, and I think it could be seen as an unexpected behavior if one feature "silently overwrites" the other.

CI failure seems unrelated:

Oh right. I've just updated the master to incorporate the fix.

@FrankenApps FrankenApps force-pushed the rustls-platform-verifier branch from b94dfb0 to 54020fb Compare March 7, 2026 08:31
@FrankenApps
Copy link
Copy Markdown
Author

The only concern I have with these particular changes is that cargo features are meant to be additive, and I think it could be seen as an unexpected behavior if one feature "silently overwrites" the other.

I agree that this is somewhat unfortunate, albeit also expected behavior for these rustls-tls-* features.
Most other projects do this in a very similar manner as far as I can tell: quinn, ureq, etc.

A notable exception is the latest version of reqwest which offers a rustls-no-provider feature that at least allows for selecting either aws-lc-rs or the ring crypto provider, but it still does not allow for additively mixing different TLS verifiers as far as I can tell.
Because of the way these behave I would even go as far and doubt that it is possible to mix those in any meaningful / logical way.

Therefore while I think that a rustls-no-provider feature would likely make sense for tungstenite-rs too (might start a separate PR if desired), I personally do not really see how to resolve your concern mentioned above.

If you see it as a blocker for merging and nobody else can give me guidance for how to improve the PR in that regard, feel free to close it.

@FrankenApps FrankenApps force-pushed the rustls-platform-verifier branch from 54020fb to 3f9e5cf Compare March 22, 2026 07:36
@FrankenApps
Copy link
Copy Markdown
Author

@daniel-abramov Rebased on top of master in order to fix the unrelated clippy issue in CI.

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.

2 participants