Skip to content

Conversation

@lynzrand
Copy link
Collaborator

  • Related issues: None
  • PR kind: refactor

Summary

This PR removes the dependency to OpenSSL, one of the most notorious library for cross compilation, for moon. This should make this project, as well as downstream executables like the closed-source mooncake, easier to cross-compile and available to more platforms.

cc @myfreess

Metadata

  • Tests added/updated for bug fixes or new features
  • Compatible with Windows/Linux/macOS

@semanticdiff-com
Copy link

Review changes with  SemanticDiff

@peter-jerry-ye-code-review
Copy link

Potential feature compatibility issue with reqwest upgrade

Category
Maintainability
Code Snippet
reqwest = { version = "0.12.24", default-features = false, features = [
"multipart",
"blocking",
"json",
"stream",
"rustls-tls",
"charset",
"http2",
"system-proxy",
] }
Recommendation
Verify that all existing reqwest usage patterns in the codebase are compatible with version 0.12, particularly around error handling and API changes. Consider adding integration tests to ensure HTTP functionality works correctly with the new TLS backend.
Reasoning
Major version upgrades of HTTP clients often introduce breaking changes in APIs, error types, or behavior that could cause runtime issues if not properly tested.

Missing certificate validation configuration

Category
Correctness
Code Snippet
reqwest = { version = "0.12.24", default-features = false, features = [
"rustls-tls",
"system-proxy",
] }
Recommendation
Ensure that certificate validation behavior matches the previous OpenSSL implementation. Consider adding explicit configuration for certificate validation if the default rustls behavior differs from OpenSSL.
Reasoning
Different TLS backends may have different default behaviors for certificate validation, which could affect security or cause connectivity issues with some servers.

Removed OpenSSL build configuration without verifying impact

Category
Performance
Code Snippet
[target.'cfg(not(windows))'.dependencies]
openssl = { version = "0.10.66", features = ["vendored"] }
Recommendation
Benchmark TLS performance between OpenSSL and rustls implementations to ensure no significant performance regression, especially for high-throughput scenarios or in resource-constrained environments.
Reasoning
While rustls is generally efficient, OpenSSL with vendored features might have had specific optimizations that could affect build times or runtime performance in certain scenarios.

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