Split up modules, support no built-in providers, small tidying #505
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the `cipher` mod was very large. This commit splits it into three modules: * `certificate` for the certificate, certified key, and root store code. * `verifier` for the client/server verifier code. * `cipher` for the remaining ciphersuite/protocol code. References are updated throughout.
Updates imports to match the Rustls style: https://github.com/rustls/rustls/blob/main/CONTRIBUTING.md#imports
* Move user data related code to `userdata` mod * Move FFI macro related code to `ffi` mod * Move version related code to `version` mod
We almost always keep the `rustls_result` enum in-scope because we return many variants from it. Importing some specific variants is confusing and we're not doing it consistently. Instead, always use the `rustls_results::` prefix for referencing variants.
ctz
approved these changes
Dec 17, 2024
Also tack a `cargo test --no-default-features` into CI to make sure we don't regress. The integration tests that use client.c/server.c can't support this build type: they require a built-in provider.
When running under `cargo ctest` the `rustls-ffi` import doesn't work for this small integration test. It's not worth debugging atm. Let's just ignore it for now and only run the test from the normal 'cargo test' invocation where it works.
The `make_acceptor()` test helper is only used in 2 places, and just directly invokes a simple constructor. Let's do that in the call-sites directly instead of adding unhelpful indirection.
|
Since this is just moving existing code around, making cosmetic tweaks, or build-gating tests I'm going to merge w/o waiting for further review. Always happy to iterate further. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Splits up
cipher.rsandlib.rs. The former was quite large, and the latter should ideally be just mod definitions and not substantial code.Adds support for building rustls-ffi without a built-in provider. Mechanically this was always possible, but
cargo testwouldn't work without some effort. Theclient/serverexamples continue to enforce that a provider is supplied. This build configuration only makes sense for downstream consumers that want to bring their own FFI-ready crypto provider.Tidies up Rust imports to match the Rustls convention.
Removes single variant
rustls_resultenum imports.Fixes
cargo capi testby ignoring the version test in this configuration.Removes a small acceptor test helper that wasn't carrying its weight.
Resolves #231