From 24ab66d15002649bd60b64db7fa2e7ef1901643a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 7 Jun 2026 01:19:03 +0000 Subject: [PATCH 1/3] Resolve value set decode performance TODO with a value -> member map Replaces the linear scan over enum members in value set decoding with an O(1) lookup in a value -> member map, built once when the class is registered and stored in Type::ValueSet. This is now possible because the ASN.1 wrapper types implement __hash__ (#14963). Member values that implement __eq__ but not __hash__ (e.g. Null) can't be used as dict keys; for those the map is None and decoding falls back to the previous linear scan, preserving behavior. https://claude.ai/code/session_01FrUdGjs6fKGSQ6E6WQPGB1 --- src/cryptography/hazmat/asn1/asn1.py | 12 ++++++++- src/rust/src/declarative_asn1/decode.rs | 34 ++++++++++++++++--------- src/rust/src/declarative_asn1/encode.rs | 2 +- src/rust/src/declarative_asn1/types.rs | 13 +++++++--- tests/hazmat/asn1/test_api.py | 6 ++++- tests/hazmat/asn1/test_serialization.py | 14 ++++++++++ 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/src/cryptography/hazmat/asn1/asn1.py b/src/cryptography/hazmat/asn1/asn1.py index 8c01f74985c6..98a7b1d7614d 100644 --- a/src/cryptography/hazmat/asn1/asn1.py +++ b/src/cryptography/hazmat/asn1/asn1.py @@ -463,7 +463,17 @@ def decorator(cls: type[U]) -> type[U]: inner = declarative_asn1.AnnotatedType( rust_type, declarative_asn1.Annotation() ) - root = declarative_asn1.Type.ValueSet(cls, inner) + # Map from member value to member, used for O(1) lookups when + # decoding. + value_map: dict | None + try: + value_map = {member.value: member for member in members} + except TypeError: + # Member values that implement `__eq__` but not `__hash__` + # (e.g. `Null`) can't be used as dict keys; decoding falls + # back to a linear scan over the members. + value_map = None + root = declarative_asn1.Type.ValueSet(cls, inner, value_map) setattr(cls, "__asn1_root__", root) return cls diff --git a/src/rust/src/declarative_asn1/decode.rs b/src/rust/src/declarative_asn1/decode.rs index a328045faa2d..baa56845f034 100644 --- a/src/rust/src/declarative_asn1/decode.rs +++ b/src/rust/src/declarative_asn1/decode.rs @@ -3,7 +3,7 @@ // for complete details. use asn1::Parser; -use pyo3::types::{PyAnyMethods, PyListMethods, PyTypeMethods}; +use pyo3::types::{PyAnyMethods, PyDictMethods, PyListMethods, PyTypeMethods}; use crate::asn1::big_byte_slice_to_py_int; use crate::declarative_asn1::types::{ @@ -262,19 +262,29 @@ fn decode_value_set<'a>( parser: &mut Parser<'a>, cls: &pyo3::Py, inner_type: &AnnotatedType, + value_map: &Option>, annotation: &Annotation, ) -> ParseResult> { let inner_ann_type = value_set_inner_type(py, inner_type, annotation)?; let decoded = decode_annotated_type(py, parser, &inner_ann_type)?; - // NOTE: This is a linear scan over the members of the enum. If this - // ever becomes a performance problem, it could be replaced with a - // value -> member map stored in `Type::ValueSet` (keeping in mind - // that hash-based lookups won't work for the asn1 wrapper types, - // which implement `__eq__` but not `__hash__`). - for member in cls.bind(py).try_iter()? { - let member = member?; - if member.getattr(pyo3::intern!(py, "value"))?.eq(&decoded)? { - return Ok(member); + match value_map { + // Common case: look the decoded value up in the value -> member + // map. + Some(value_map) => { + if let Some(member) = value_map.bind(py).get_item(&decoded)? { + return Ok(member); + } + } + // The map is `None` when the member values implement `__eq__` + // but not `__hash__` (e.g. `Null`), so fall back to a linear + // scan over the members. + None => { + for member in cls.bind(py).try_iter()? { + let member = member?; + if member.getattr(pyo3::intern!(py, "value"))?.eq(&decoded)? { + return Ok(member); + } + } } } Err(CryptographyError::Py( @@ -452,8 +462,8 @@ pub(crate) fn decode_annotated_type<'a>( ))? } }, - Type::ValueSet(cls, inner_type) => { - decode_value_set(py, parser, cls, inner_type.get(), annotation)? + Type::ValueSet(cls, inner_type, value_map) => { + decode_value_set(py, parser, cls, inner_type.get(), value_map, annotation)? } Type::PyBool() => decode_pybool(py, parser, encoding)?.into_any(), Type::PyInt() => decode_pyint(py, parser, encoding)?.into_any(), diff --git a/src/rust/src/declarative_asn1/encode.rs b/src/rust/src/declarative_asn1/encode.rs index 139978fc173b..669189b00a6e 100644 --- a/src/rust/src/declarative_asn1/encode.rs +++ b/src/rust/src/declarative_asn1/encode.rs @@ -181,7 +181,7 @@ impl asn1::Asn1Writable for AnnotatedTypeObject<'_> { ), )) } - Type::ValueSet(cls, inner_type) => { + Type::ValueSet(cls, inner_type, _) => { if !value.is_instance(cls.bind(py))? { return Err(CryptographyError::Py( pyo3::exceptions::PyTypeError::new_err(format!( diff --git a/src/rust/src/declarative_asn1/types.rs b/src/rust/src/declarative_asn1/types.rs index 9eb6f470c36e..eb737a5a4921 100644 --- a/src/rust/src/declarative_asn1/types.rs +++ b/src/rust/src/declarative_asn1/types.rs @@ -38,8 +38,15 @@ pub enum Type { /// a single underlying ASN.1 type). /// The first element is the Python enum class, the second /// element is the (already converted) underlying type of the - /// member values. - ValueSet(pyo3::Py, pyo3::Py), + /// member values, and the third element is a map from member + /// value to enum member, used when decoding. The map is `None` + /// when the member values are not hashable, in which case + /// decoding falls back to a linear scan over the members. + ValueSet( + pyo3::Py, + pyo3::Py, + Option>, + ), // Python types that we map to canonical ASN.1 types // @@ -686,7 +693,7 @@ pub(crate) fn is_tag_valid_for_type( Type::Choice(variants) => variants.bind(py).into_iter().any(|v| { is_tag_valid_for_variant(py, tag, v.cast::().unwrap().get(), encoding) }), - Type::ValueSet(_, t) => is_tag_valid_for_type(py, tag, t.get().inner.get(), encoding), + Type::ValueSet(_, t, _) => is_tag_valid_for_type(py, tag, t.get().inner.get(), encoding), Type::PyBool() => check_tag_with_encoding(bool::TAG, encoding, tag), Type::PyInt() => check_tag_with_encoding(asn1::BigInt::TAG, encoding, tag), Type::PyBytes() => { diff --git a/tests/hazmat/asn1/test_api.py b/tests/hazmat/asn1/test_api.py index b9fe418d6931..aa4878c10c12 100644 --- a/tests/hazmat/asn1/test_api.py +++ b/tests/hazmat/asn1/test_api.py @@ -428,9 +428,13 @@ def test_fields_of_variant_type(self) -> None: choice = declarative_asn1.Type.Choice(my_list) assert choice._0 is my_list - value_set = declarative_asn1.Type.ValueSet(type(None), ann_type) + my_value_map: dict = dict() + value_set = declarative_asn1.Type.ValueSet( + type(None), ann_type, my_value_map + ) assert value_set._0 is type(None) assert value_set._1 is ann_type + assert value_set._2 is my_value_map def test_fields_of_variant_encoding(self) -> None: from cryptography.hazmat.bindings._rust import declarative_asn1 diff --git a/tests/hazmat/asn1/test_serialization.py b/tests/hazmat/asn1/test_serialization.py index d88828f04623..44ad1e8c4bf2 100644 --- a/tests/hazmat/asn1/test_serialization.py +++ b/tests/hazmat/asn1/test_serialization.py @@ -2230,6 +2230,20 @@ class Example: ] ) + def test_ok_null_value_set(self) -> None: + # `Null` implements `__eq__` but not `__hash__`, so decoding + # exercises the linear scan fallback (instead of the value -> + # member map lookup). + @asn1.value_set(asn1.Null) + class Marker(enum.Enum): + PRESENT = asn1.Null() + + assert_roundtrips( + [ + (Marker.PRESENT, b"\x05\x00"), + ] + ) + def test_fail_decode_non_member_value(self) -> None: @asn1.sequence @_comparable_dataclass From c07f34f75ed7d0e500821893286a428eaecb801c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 7 Jun 2026 04:03:28 +0000 Subject: [PATCH 2/3] Remove the linear scan fallback for unhashable value set members Per review: value set member values must be hashable. The value -> member map is now built unconditionally at registration time, so an enum with unhashable member values (e.g. Null) raises TypeError when decorated, and decoding is always an O(1) map lookup. https://claude.ai/code/session_01FrUdGjs6fKGSQ6E6WQPGB1 --- src/cryptography/hazmat/asn1/asn1.py | 11 ++--------- src/rust/src/declarative_asn1/decode.rs | 23 +++-------------------- src/rust/src/declarative_asn1/types.rs | 6 ++---- tests/hazmat/asn1/test_api.py | 10 ++++++++++ tests/hazmat/asn1/test_serialization.py | 14 -------------- 5 files changed, 17 insertions(+), 47 deletions(-) diff --git a/src/cryptography/hazmat/asn1/asn1.py b/src/cryptography/hazmat/asn1/asn1.py index 98a7b1d7614d..d77581fcc600 100644 --- a/src/cryptography/hazmat/asn1/asn1.py +++ b/src/cryptography/hazmat/asn1/asn1.py @@ -464,15 +464,8 @@ def decorator(cls: type[U]) -> type[U]: rust_type, declarative_asn1.Annotation() ) # Map from member value to member, used for O(1) lookups when - # decoding. - value_map: dict | None - try: - value_map = {member.value: member for member in members} - except TypeError: - # Member values that implement `__eq__` but not `__hash__` - # (e.g. `Null`) can't be used as dict keys; decoding falls - # back to a linear scan over the members. - value_map = None + # decoding. This requires the member values to be hashable. + value_map = {member.value: member for member in members} root = declarative_asn1.Type.ValueSet(cls, inner, value_map) setattr(cls, "__asn1_root__", root) diff --git a/src/rust/src/declarative_asn1/decode.rs b/src/rust/src/declarative_asn1/decode.rs index baa56845f034..a20c83fc4ed7 100644 --- a/src/rust/src/declarative_asn1/decode.rs +++ b/src/rust/src/declarative_asn1/decode.rs @@ -262,30 +262,13 @@ fn decode_value_set<'a>( parser: &mut Parser<'a>, cls: &pyo3::Py, inner_type: &AnnotatedType, - value_map: &Option>, + value_map: &pyo3::Py, annotation: &Annotation, ) -> ParseResult> { let inner_ann_type = value_set_inner_type(py, inner_type, annotation)?; let decoded = decode_annotated_type(py, parser, &inner_ann_type)?; - match value_map { - // Common case: look the decoded value up in the value -> member - // map. - Some(value_map) => { - if let Some(member) = value_map.bind(py).get_item(&decoded)? { - return Ok(member); - } - } - // The map is `None` when the member values implement `__eq__` - // but not `__hash__` (e.g. `Null`), so fall back to a linear - // scan over the members. - None => { - for member in cls.bind(py).try_iter()? { - let member = member?; - if member.getattr(pyo3::intern!(py, "value"))?.eq(&decoded)? { - return Ok(member); - } - } - } + if let Some(member) = value_map.bind(py).get_item(&decoded)? { + return Ok(member); } Err(CryptographyError::Py( pyo3::exceptions::PyValueError::new_err(format!( diff --git a/src/rust/src/declarative_asn1/types.rs b/src/rust/src/declarative_asn1/types.rs index eb737a5a4921..d296e493e941 100644 --- a/src/rust/src/declarative_asn1/types.rs +++ b/src/rust/src/declarative_asn1/types.rs @@ -39,13 +39,11 @@ pub enum Type { /// The first element is the Python enum class, the second /// element is the (already converted) underlying type of the /// member values, and the third element is a map from member - /// value to enum member, used when decoding. The map is `None` - /// when the member values are not hashable, in which case - /// decoding falls back to a linear scan over the members. + /// value to enum member, used when decoding. ValueSet( pyo3::Py, pyo3::Py, - Option>, + pyo3::Py, ), // Python types that we map to canonical ASN.1 types diff --git a/tests/hazmat/asn1/test_api.py b/tests/hazmat/asn1/test_api.py index aa4878c10c12..1db636a97f2f 100644 --- a/tests/hazmat/asn1/test_api.py +++ b/tests/hazmat/asn1/test_api.py @@ -628,3 +628,13 @@ def test_fail_unsupported_value_type(self) -> None: match="cannot handle type", ): asn1.value_set(float) + + def test_fail_unhashable_member_values(self) -> None: + # Member values must be hashable, since they are used as keys + # in the value -> member map used when decoding. `Null` + # implements `__eq__` but not `__hash__`. + with pytest.raises(TypeError, match="unhashable type"): + + @asn1.value_set(asn1.Null) + class Example(enum.Enum): + A = asn1.Null() diff --git a/tests/hazmat/asn1/test_serialization.py b/tests/hazmat/asn1/test_serialization.py index 44ad1e8c4bf2..d88828f04623 100644 --- a/tests/hazmat/asn1/test_serialization.py +++ b/tests/hazmat/asn1/test_serialization.py @@ -2230,20 +2230,6 @@ class Example: ] ) - def test_ok_null_value_set(self) -> None: - # `Null` implements `__eq__` but not `__hash__`, so decoding - # exercises the linear scan fallback (instead of the value -> - # member map lookup). - @asn1.value_set(asn1.Null) - class Marker(enum.Enum): - PRESENT = asn1.Null() - - assert_roundtrips( - [ - (Marker.PRESENT, b"\x05\x00"), - ] - ) - def test_fail_decode_non_member_value(self) -> None: @asn1.sequence @_comparable_dataclass From 0fdcba3041e747cd00b6daa8a8bbd8c301a250c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 7 Jun 2026 04:06:01 +0000 Subject: [PATCH 3/3] Address review feedback - Use a {} literal for the value map in test_fields_of_variant_type (the annotation stays: mypy can't infer the type of an empty literal) - Drop test_fail_unhashable_member_values https://claude.ai/code/session_01FrUdGjs6fKGSQ6E6WQPGB1 --- tests/hazmat/asn1/test_api.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/tests/hazmat/asn1/test_api.py b/tests/hazmat/asn1/test_api.py index 1db636a97f2f..46023518c83e 100644 --- a/tests/hazmat/asn1/test_api.py +++ b/tests/hazmat/asn1/test_api.py @@ -428,7 +428,7 @@ def test_fields_of_variant_type(self) -> None: choice = declarative_asn1.Type.Choice(my_list) assert choice._0 is my_list - my_value_map: dict = dict() + my_value_map: dict = {} value_set = declarative_asn1.Type.ValueSet( type(None), ann_type, my_value_map ) @@ -628,13 +628,3 @@ def test_fail_unsupported_value_type(self) -> None: match="cannot handle type", ): asn1.value_set(float) - - def test_fail_unhashable_member_values(self) -> None: - # Member values must be hashable, since they are used as keys - # in the value -> member map used when decoding. `Null` - # implements `__eq__` but not `__hash__`. - with pytest.raises(TypeError, match="unhashable type"): - - @asn1.value_set(asn1.Null) - class Example(enum.Enum): - A = asn1.Null()