Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start converting src/x509/verify.rs to new pyo3 APIs #10736

Merged
merged 5 commits into from
Apr 5, 2024
Merged

Start converting src/x509/verify.rs to new pyo3 APIs #10736

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fdcd922
Start converting `src/x509/verify.rs` to new pyo3 APIs
facutuesca Apr 5, 2024
595a3b3
Fix errors with temp values being dropped
facutuesca Apr 5, 2024
0f86ab4
Fix error when using `Bound::to_str` in Python<3.10
facutuesca Apr 5, 2024
2dc9f3b
Remove extra clone() call
facutuesca Apr 5, 2024
bdc2152
Add TODO message
facutuesca Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions src/rust/src/x509/verify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use cryptography_x509_verification::{
trust_store::Store,
types::{DNSName, IPAddress},
};
use pyo3::IntoPy;
use pyo3::prelude::{PyAnyMethods, PyListMethods};

use crate::backend::keys;
use crate::error::{CryptographyError, CryptographyResult};
Expand Down Expand Up @@ -75,7 +75,7 @@ impl PolicyBuilder {
fn time(
&self,
py: pyo3::Python<'_>,
new_time: &pyo3::PyAny,
new_time: &pyo3::Bound<'_, pyo3::PyAny>,
) -> CryptographyResult<PolicyBuilder> {
if self.time.is_some() {
return Err(CryptographyError::from(
Expand All @@ -85,7 +85,7 @@ impl PolicyBuilder {
));
}
Ok(PolicyBuilder {
time: Some(py_to_datetime(py, new_time)?),
time: Some(py_to_datetime(py, new_time.clone().into_gil_ref())?),
store: self.store.as_ref().map(|s| s.clone_ref(py)),
max_chain_depth: self.max_chain_depth,
})
Expand Down Expand Up @@ -273,7 +273,7 @@ impl PyClientVerifier {
)
.map_err(|e| VerificationError::new_err(format!("validation failed: {e:?}")))?;

let py_chain = pyo3::types::PyList::empty(py);
let py_chain = pyo3::types::PyList::empty_bound(py);
for c in &chain {
py_chain.append(c.extra())?;
}
Expand All @@ -293,7 +293,7 @@ impl PyClientVerifier {

Ok(PyVerifiedClient {
subjects: py_gns,
chain: py_chain.into_py(py),
chain: py_chain.unbind(),
})
}
}
Expand Down Expand Up @@ -334,7 +334,7 @@ impl PyServerVerifier {
py: pyo3::Python<'p>,
leaf: pyo3::Py<PyCertificate>,
intermediates: Vec<pyo3::Py<PyCertificate>>,
) -> CryptographyResult<&'p pyo3::types::PyList> {
) -> CryptographyResult<pyo3::Bound<'p, pyo3::types::PyList>> {
let policy = self.as_policy();
let store = self.store.get();

Expand All @@ -354,7 +354,7 @@ impl PyServerVerifier {
)
.map_err(|e| VerificationError::new_err(format!("validation failed: {e:?}")))?;

let result = pyo3::types::PyList::empty(py);
let result = pyo3::types::PyList::empty_bound(py);
for c in chain {
result.append(c.extra())?;
}
Expand All @@ -366,21 +366,22 @@ fn build_subject_owner(
py: pyo3::Python<'_>,
subject: &pyo3::Py<pyo3::PyAny>,
) -> pyo3::PyResult<SubjectOwner> {
let subject = subject.as_ref(py);
let subject = subject.bind(py);
alex marked this conversation as resolved.
Show resolved Hide resolved

if subject.is_instance(types::DNS_NAME.get(py)?)? {
if subject.is_instance(&types::DNS_NAME.get_bound(py)?)? {
let value = subject
.getattr(pyo3::intern!(py, "value"))?
.downcast::<pyo3::types::PyString>()?;

Ok(SubjectOwner::DNSName(value.to_str()?.to_owned()))
} else if subject.is_instance(types::IP_ADDRESS.get(py)?)? {
// TODO: switch this to borrowing the string (using Bound::to_str) once our
// minimum Python version is 3.10
.extract::<String>()?;
Ok(SubjectOwner::DNSName(value))
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment reminding us that when our minimum python is 3.10, we can switch to borrowing the str.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

} else if subject.is_instance(&types::IP_ADDRESS.get_bound(py)?)? {
let value = subject
.getattr(pyo3::intern!(py, "_packed"))?
.call0()?
.downcast::<pyo3::types::PyBytes>()?;

Ok(SubjectOwner::IPAddress(value.into()))
.downcast::<pyo3::types::PyBytes>()?
.clone();
Ok(SubjectOwner::IPAddress(value.unbind()))
} else {
Err(pyo3::exceptions::PyTypeError::new_err(
"unsupported subject type",
Expand Down Expand Up @@ -458,7 +459,7 @@ pub(crate) fn add_to_module(module: &pyo3::prelude::PyModule) -> pyo3::PyResult<
module.add_class::<PolicyBuilder>()?;
module.add(
"VerificationError",
module.py().get_type::<VerificationError>(),
module.py().get_type_bound::<VerificationError>(),
)?;

Ok(())
Expand Down