Skip to content

Fix compiling with OpenSSL#7621

Merged
youknowone merged 1 commit intoRustPython:mainfrom
joshuamegnauth54:fix-openssl-compilation
Apr 17, 2026
Merged

Fix compiling with OpenSSL#7621
youknowone merged 1 commit intoRustPython:mainfrom
joshuamegnauth54:fix-openssl-compilation

Conversation

@joshuamegnauth54
Copy link
Copy Markdown
Contributor

@joshuamegnauth54 joshuamegnauth54 commented Apr 17, 2026

This PR fixes compiling with --features=ssl-openssl. Nothing fancy. I fixed some lints.

Summary by CodeRabbit

  • Refactor
    • Internal improvements to OpenSSL/SSL string handling and channel binding operations for better code consistency and maintainability.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 17, 2026

📝 Walkthrough

Walkthrough

OpenSSL integration code refactored to switch from PyStrRef::as_str() to PyStrRef::as_ref() with explicit &str bindings across multiple SSL-related functions, affecting cipher/curve/callback/verification handling and string-to-bytes conversions.

Changes

Cohort / File(s) Summary
OpenSSL String Handling Refactor
crates/stdlib/src/openssl.rs
Replaced PyStrRef::as_str() calls with PyStrRef::as_ref() in SSL context and socket methods (set_ciphers, set_ecdh_curve, set_psk_server_callback, load_verify_locations, get_channel_binding, etc.). Changed PSK callback to use hint.to_string() instead of hint.as_str().to_owned(). Maintains existing behavior for NUL detection, identity hint storage, and channel binding type derivation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A hop through OpenSSL's halls so bright,
Where strings now reference with delight,
From as_str to as_ref we bound,
Consistent patterns all around—
Refactored clean, no logic blight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix compiling with OpenSSL' directly and clearly describes the main objective of the pull request: fixing compilation issues with the OpenSSL feature flag.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
crates/stdlib/src/openssl.rs (1)

1464-1464: Minor: hint.to_string() vs hint.as_ref().to_owned().

hint.to_string() goes through the Display impl, which works, but hint.as_ref().to_owned() would be more consistent with the other sites in this PR (all other conversions now bind &str via as_ref()) and avoids any formatter overhead. Purely stylistic — feel free to ignore.

Optional nit
-                    *self.psk_identity_hint.lock() = Some(hint.to_string());
+                    *self.psk_identity_hint.lock() = Some(hint.as_ref().to_owned());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/stdlib/src/openssl.rs` at line 1464, The assignment to
*self.psk_identity_hint.lock() uses hint.to_string() which goes through Display;
change it to use hint.as_ref().to_owned() to match the other conversions in this
PR and avoid formatter overhead—locate the line that assigns
Some(hint.to_string()) in the block where self.psk_identity_hint.lock() is set
and replace the call with Some(hint.as_ref().to_owned()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/stdlib/src/openssl.rs`:
- Line 1464: The assignment to *self.psk_identity_hint.lock() uses
hint.to_string() which goes through Display; change it to use
hint.as_ref().to_owned() to match the other conversions in this PR and avoid
formatter overhead—locate the line that assigns Some(hint.to_string()) in the
block where self.psk_identity_hint.lock() is set and replace the call with
Some(hint.as_ref().to_owned()).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 2506b2cf-3f56-4dd4-9b55-67ef54d52b94

📥 Commits

Reviewing files that changed from the base of the PR and between 8d61a22 and 5acf9b4.

📒 Files selected for processing (1)
  • crates/stdlib/src/openssl.rs

Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh left a comment

Choose a reason for hiding this comment

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

ty:)

@youknowone youknowone merged commit f82b8d8 into RustPython:main Apr 17, 2026
20 checks passed
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.

3 participants