Add WriteError variant for custom errors#562
Conversation
Signed-off-by: Facundo Tuesca <facundo.tuesca@trailofbits.com>
8c6daef to
5ebf95d
Compare
|
Ok
…On Sun, Jun 22, 2025, 8:33 AM Facundo Tuesca ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/writer.rs
<#562 (comment)>:
> @@ -8,12 +8,14 @@ use alloc::{fmt, vec};
#[derive(PartialEq, Eq, Debug)]
pub enum WriteError {
AllocationError,
+ CustomError(String),
It can, with the caveat that (I think) we won't be able to display error
messages with runtime information, e.g: CustomError(format!("Error while
encoding field {name} as INTEGER")).
If that's OK, I'll make the change
—
Reply to this email directly, view it on GitHub
<#562 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBCIOFMKPND775UDNZD3E2PBVAVCNFSM6AAAAAB73F2TYKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDSNBYGE3TIOBVGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Signed-off-by: Facundo Tuesca <facundo.tuesca@trailofbits.com>
|
Hmm, stepping back a bit, this design isn't super consistent with what we have for parsing. Can you show a bit of code that explains why/how this is required, and then we can brainstorm on if there's a better way? |
Sure! This is in the context of the Python ASN.1 API that uses pub struct AnnotatedTypeObject<'a> {
// This contains the Python type information, that determines what type of ASN.1 structure
// this represents (e.g.: a SEQUENCE with two fields, one of them has a DEFAULT value, etc)
pub(crate) annotated_type: &'a AnnotatedType,
// The actual value to encode (e.g.: an object with two attributes, one per field)
pub(crate) value: &'a Bound<'a, PyAny>,
}Then we implement impl asn1::Asn1Writable for AnnotatedTypeObject<'_> {
fn write(&self, writer: &mut Writer<'_>) -> Result<(), asn1::WriteError> {
Python::with_gil(|py| {
let annotated_type = self.annotated_type;
let inner = annotated_type.inner.get();
match &inner {
Type::Sequence(cls) => {
let fields = cls
.getattr(py, "__asn1_fields__")
.map_err(|_| asn1::CustomError("Error accessing SEQUENCE fields")?;
//...etc, encode the values using `asn1::SequenceWriter::new`
// this also recursively calls `write` for each field
}
Type::PyInt() => {
let val: i64 = value
.extract()
.map_err(|_| asn1::CustomError("Error converting Python int to Rust i64")?;
val.write(writer)
}
//etc (other types)Since we traverse the ASN1 structure on the fly while we write/encode the values, there are errors that can happen inside the |
|
Ok, I spent some time thinking, and I wonder if adding an assosciated |
looks like default values for associated types are unstable: |
|
Doh. Well, maybe no default then? It's a bit verbose but possibly fine?
…On Sun, Jun 29, 2025 at 10:33 AM Facundo Tuesca ***@***.***> wrote:
facutuesca left a comment (alex/rust-asn1#562)
Ok, I spent some time thinking, and I wonder if adding an assosciated Error type to Asn1Writable, which defaults to WriteErorr and has a From<WriteError> bound would work?
looks like default values for associated types are unstable:
pub trait Asn1Writable: Sized {
type Error = WriteError;
└─associated type defaults are unstable
see issue #29661 <rust-lang/rust#29661> for more information
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you commented.Message ID: ***@***.***>
--
All that is necessary for evil to succeed is for good people to do nothing.
|
|
How would you like to handle #[derive(asn1::Asn1Write)]
struct S {
field1: MyTypeA,
field2: MyTypeB,
}means that the derived |
|
Hmm, maybe an additional annotation to specify the write error type?
…On Mon, Jun 30, 2025 at 4:58 AM Facundo Tuesca ***@***.***> wrote:
*facutuesca* left a comment (alex/rust-asn1#562)
<#562 (comment)>
How would you like to handle derive(asn1::Asn1Write) in that case? If
each Asn1Writable type has its own associated Error, doing:
#[derive(asn1::Asn1Write)]struct S {
field1: MyTypeA,
field2: MyTypeB,}
means that the derived write function would call write functions that
return Result<(), MyTypeA::Error> and Result<(), MyTypeB::Error>. So how
do we define S::Error?
—
Reply to this email directly, view it on GitHub
<#562 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBBNKT6B7BTEGHTUSST3GD34XAVCNFSM6AAAAAB73F2TYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTAMJYGM2DQOBQGE>
.
You are receiving this because you commented.Message ID:
***@***.***>
--
All that is necessary for evil to succeed is for good people to do nothing.
|
I started implementing this, see #567 |
This PR adds a new
WriteErrorenum variantCustomError, for users that implementAsn1Writable::writefor their own types and have errors that can happen during encoding that are not covered by the current enum variants.