Conversation
📝 WalkthroughWalkthroughModified SSL context password argument parsing to explicitly handle None password values by returning early with (None, None). Preserved existing branches for string, bytes-like, and callable passwords. Simplified error message for invalid password types from three types to two supported types. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
This reverts commit fa8fb38.
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [x] lib: cpython/Lib/http dependencies:
dependent tests: (23 tests)
Legend:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/stdlib/src/ssl.rs`:
- Around line 2024-2035: The password-handling branch currently accepts UTF-8
strings, bytes-like via ArgBytesLike::try_from_object, or a callable
(p.is_callable), but the error raised in the final Err uses "password should be
a string or callable"; update that error text to list all accepted types (e.g.,
"password should be str, bytes, bytearray, or callable") so callers see the
correct allowed types; locate the matching match/if chain around
ArgBytesLike::try_from_object and the p.is_callable branch and change the
vm.new_type_error message accordingly (refer to ArgBytesLike::try_from_object,
the String::from_utf8 branch, and p.is_callable).
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
Lib/http/__init__.pyis excluded by!Lib/**Lib/http/client.pyis excluded by!Lib/**Lib/http/cookiejar.pyis excluded by!Lib/**Lib/http/cookies.pyis excluded by!Lib/**Lib/http/server.pyis excluded by!Lib/**Lib/test/test_http_cookiejar.pyis excluded by!Lib/**Lib/test/test_http_cookies.pyis excluded by!Lib/**Lib/test/test_httplib.pyis excluded by!Lib/**Lib/test/test_httpservers.pyis excluded by!Lib/**
📒 Files selected for processing (1)
crates/stdlib/src/ssl.rs
| else if let Ok(pwd_bytes_like) = ArgBytesLike::try_from_object(vm, p.clone()) | ||
| { | ||
| let pwd = String::from_utf8(pwd_bytes_like.borrow_buf().to_vec()).map_err( | ||
| |_| vm.new_type_error("password bytes must be valid UTF-8".to_owned()), | ||
| )?; | ||
| let pwd = String::from_utf8(pwd_bytes_like.borrow_buf().to_vec()) | ||
| .map_err(|_| vm.new_type_error("password bytes must be valid UTF-8"))?; | ||
| Ok((Some(pwd), None)) | ||
| } | ||
| // Try callable | ||
| else if p.is_callable() { | ||
| Ok((None, Some(p.clone()))) | ||
| } else { | ||
| Err(vm.new_type_error( | ||
| "password should be a string, bytes, or callable".to_owned(), | ||
| )) | ||
| Err(vm.new_type_error("password should be a string or callable")) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "password should be a string or callable|password should be a string, bytes|load_cert_chain|password callback" crates/stdlib/src/ssl.rsRepository: RustPython/RustPython
Length of output: 714
🌐 Web query:
CPython 3.14 ssl.SSLContext.load_cert_chain password parameter accepted types TypeError message
💡 Result:
In CPython 3.14, ssl.SSLContext.load_cert_chain(certfile, keyfile=None, password=None) accepts password as: str, bytes, bytearray, or a callable (called with no arguments) that returns str, bytes, or bytearray; str is UTF-8 encoded before use. [1]
If you pass an invalid password object (e.g. True), CPython raises TypeError with an error message matching "should be a string" (per the stdlib tests). [2] Historically / in source, this error is phrased like **"password should be a string or callable" (wording may vary by version). [3]
If you pass a callable but it returns a wrong type (e.g. 9), CPython raises TypeError with an error message matching "must return a string" (per the stdlib tests). [2]
Sources: Python 3.14.2 ssl docs [1], CPython test_ssl.py expectations [2], CPython _ssl source example wording [3].
[1] (docs.python.org)
[2] (fossies.org)
[3] (gist.github.com)
Citations:
- 1: https://docs.python.org/es/3.14/library/ssl.html?utm_source=openai
- 2: https://fossies.org/linux/Python/Lib/test/test_ssl.py?utm_source=openai
- 3: https://gist.github.com/Lh4cKg/fc6f1b6d6e6d285f51d6?utm_source=openai
Update error message to reflect accepted password types.
Line 2024-2028 accepts bytes-like passwords via ArgBytesLike::try_from_object, but the error message at line 2034 only mentions "string or callable". The message should be updated to accurately reflect all accepted types:
Fix
- Err(vm.new_type_error("password should be a string or callable"))
+ Err(vm.new_type_error("password should be a string, bytes, or callable"))This aligns with CPython 3.14's ssl.SSLContext.load_cert_chain(), which also accepts str, bytes, bytearray, or a callable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| else if let Ok(pwd_bytes_like) = ArgBytesLike::try_from_object(vm, p.clone()) | |
| { | |
| let pwd = String::from_utf8(pwd_bytes_like.borrow_buf().to_vec()).map_err( | |
| |_| vm.new_type_error("password bytes must be valid UTF-8".to_owned()), | |
| )?; | |
| let pwd = String::from_utf8(pwd_bytes_like.borrow_buf().to_vec()) | |
| .map_err(|_| vm.new_type_error("password bytes must be valid UTF-8"))?; | |
| Ok((Some(pwd), None)) | |
| } | |
| // Try callable | |
| else if p.is_callable() { | |
| Ok((None, Some(p.clone()))) | |
| } else { | |
| Err(vm.new_type_error( | |
| "password should be a string, bytes, or callable".to_owned(), | |
| )) | |
| Err(vm.new_type_error("password should be a string or callable")) | |
| } | |
| else if let Ok(pwd_bytes_like) = ArgBytesLike::try_from_object(vm, p.clone()) | |
| { | |
| let pwd = String::from_utf8(pwd_bytes_like.borrow_buf().to_vec()) | |
| .map_err(|_| vm.new_type_error("password bytes must be valid UTF-8"))?; | |
| Ok((Some(pwd), None)) | |
| } | |
| // Try callable | |
| else if p.is_callable() { | |
| Ok((None, Some(p.clone()))) | |
| } else { | |
| Err(vm.new_type_error("password should be a string, bytes, or callable")) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/stdlib/src/ssl.rs` around lines 2024 - 2035, The password-handling
branch currently accepts UTF-8 strings, bytes-like via
ArgBytesLike::try_from_object, or a callable (p.is_callable), but the error
raised in the final Err uses "password should be a string or callable"; update
that error text to list all accepted types (e.g., "password should be str,
bytes, bytearray, or callable") so callers see the correct allowed types; locate
the matching match/if chain around ArgBytesLike::try_from_object and the
p.is_callable branch and change the vm.new_type_error message accordingly (refer
to ArgBytesLike::try_from_object, the String::from_utf8 branch, and
p.is_callable).
1 participant
Summary by CodeRabbit