Don't force users to unsafe.

[kixelated]

Fixes #103

Summary by CodeRabbit

• Refactor
  • Updated the API for configuring certificate verification bypass. Certificate verification can now be disabled via client.dangerous().with_no_certificate_verification() instead of the previous direct method.

[coderabbitai]

Walkthrough

The pull request refactors certificate verification bypass in the WebTransport QUINN client. It replaces the unsafe with_no_certificate_verification method on ClientBuilder with a two-step builder pattern: ClientBuilder::dangerous() returns a new DangerousClientBuilder struct, which then provides the with_no_certificate_verification method. The example code is updated to use this new pattern. The underlying functionality remains the same; only the access pattern and safety wrapper change.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Don't force users to unsafe." is concise and directly captures the main objective of the changeset. It clearly summarizes the primary change: replacing an unsafe function-based API with a safer, explicit "dangerous" builder pattern that doesn't force users into unsafe blocks. The title is specific enough for a teammate reviewing the git history to understand the core improvement without requiring additional context.
Linked Issues Check	✅ Passed	The code changes fully satisfy the requirements from linked issue #103. The PR successfully replaces the unsafe with_no_certificate_verification() method on ClientBuilder with a two-step pattern: a new public dangerous() method that returns a DangerousClientBuilder, which then provides the with_no_certificate_verification() method. This implements the suggested rustls-like "dangerous" builder pattern, allowing crates with forbid(unsafe_code) to use the API without unsafe blocks. The example in echo-client.rs is updated to demonstrate the new pattern. All objectives from issue #103 are addressed.
Out of Scope Changes Check	✅ Passed	All changes are directly in scope and aligned with issue #103's objectives. The modifications to client.rs implement the core requirement by removing the unsafe method and introducing the dangerous builder pattern, while the update to echo-client.rs demonstrates the new API usage. No unrelated changes, cleanup tasks, or scope-creeping modifications are present in the changeset. Every alteration serves the primary goal of eliminating forced unsafe usage.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch no-unsafe

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 03066d8 and 6f9191c.

📒 Files selected for processing (2)

• web-transport-quinn/examples/echo-client.rs (1 hunks)
• web-transport-quinn/src/client.rs (2 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build

🔇 Additional comments (3)

web-transport-quinn/src/client.rs (2)

128-135: LGTM! Clean implementation of the dangerous builder pattern.

The method correctly implements the two-step builder pattern for accessing dangerous TLS configurations without requiring unsafe blocks. Documentation clearly communicates the opt-in nature and security implications.

---

171-202: Refactor verified and approved.

The migration from the unsafe ClientBuilder::with_no_certificate_verification to the safe DangerousClientBuilder::with_no_certificate_verification is complete and correct:

• Method at line 190 is properly marked as pub fn (not pub unsafe fn)
• Only accessible through explicit dangerous() builder opt-in
• Example usage at web-transport-quinn/examples/echo-client.rs:37 follows the correct pattern: client.dangerous().with_no_certificate_verification()?
• No remnants of old unsafe API found in codebase

web-transport-quinn/examples/echo-client.rs (1)

37-37: Perfect migration to the new safe API.

The update correctly demonstrates the new pattern: explicitly opting into dangerous configuration via dangerous() without requiring an unsafe block. This achieves the PR's goal of allowing forbid(unsafe_code) crates to disable certificate verification in test/development scenarios.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}