Skip to content

Add type aliases for OCSP and PKCS7 types #14528

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

Open
TaaviE wants to merge 1 commit into
base: main
Choose a base branch
from
+46 −1
Open

Add type aliases for OCSP and PKCS7 types #14528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion src/rust/cryptography-x509/src/ocsp_resp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ use crate::{certificate, common, crl, extensions, name, ocsp_req};

#[derive(asn1::Asn1Read, asn1::Asn1Write)]
pub struct OCSPResponse<'a> {
pub response_status: asn1::Enumerated,
pub response_status: OCSPResponseStatus,
#[explicit(0)]
pub response_bytes: Option<ResponseBytes<'a>>,
}

pub type OCSPResponseStatus = asn1::Enumerated;

#[derive(asn1::Asn1Read, asn1::Asn1Write)]
pub struct ResponseBytes<'a> {
pub response_type: asn1::DefinedByMarker<asn1::ObjectIdentifier>,
Expand Down
43 changes: 43 additions & 0 deletions src/rust/cryptography-x509/src/pkcs7.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,49 @@ pub struct SignedData<'a> {
>,
}

pub type CertificateSet<'a> = common::Asn1ReadableOrWritable<
asn1::SetOf<'a, CertificateChoices<'a>>,
asn1::SetOfWriter<'a, CertificateChoices<'a>, Vec<CertificateChoices<'a>>>,
>;

pub type RevocationInfoChoices<'a> = common::Asn1ReadableOrWritable<
asn1::SetOf<'a, RevocationInfoChoice<'a>>,
asn1::SetOfWriter<'a, RevocationInfoChoice<'a>, Vec<RevocationInfoChoice<'a>>>,
>;

#[allow(clippy::large_enum_variant)]
#[derive(asn1::Asn1Write, asn1::Asn1Read)]
pub enum CertificateChoices<'a> {
Copy link
Copy Markdown
Member

@alex alex Mar 23, 2026

Choose a reason for hiding this comment

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

I did not mean to suggest we should add these entire enums. My suggestion was to split out the type-alises you'd broken out of the existing code.

(Adding these entire untested enums is the opposite of my goal.)

Copy link
Copy Markdown
Contributor Author

@TaaviE TaaviE Mar 24, 2026

Choose a reason for hiding this comment

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

Well, CertificateSet contains CertificateChoices. RevocationInfoChoices contains RevocationInfoChoice. So on and so forth, I can't split out anything besides OCSPResponseStatus on its own.

Copy link
Copy Markdown
Member

@alex alex Mar 24, 2026

Choose a reason for hiding this comment

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

You can split out the type alias using the current tyeps, e.g.

pub type CertificateSet<'a> = common::Asn1ReadableOrWritable<
    asn1::SetOf<'a, certificate::Certificate<'a>>,
    asn1::SetOfWriter<'a,certificate::Certificate<'a>, Vec<certificate::Certificate<'a>>>,
>;

Copy link
Copy Markdown
Contributor Author

@TaaviE TaaviE Mar 24, 2026

Choose a reason for hiding this comment

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

What is the point?

Copy link
Copy Markdown
Member

@alex alex Mar 24, 2026

Choose a reason for hiding this comment

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

It makes the substantiation of /Certificate/CertificateChoices/ a smaller diff.

It's not the biggest thing in the world, but if you want to make the diff easier to review, it does contribute.

Copy link
Copy Markdown
Contributor Author

@TaaviE TaaviE Mar 24, 2026

Choose a reason for hiding this comment

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

It won't even become a noticeably smaller diff because second patch has to update these structures to use the new types? :D

Certificate(certificate::Certificate<'a>),
#[implicit(0)]
ExtendedCertificate(asn1::Sequence<'a>),
#[implicit(1)]
V1AttrCert(asn1::Sequence<'a>),
#[implicit(2)]
V2AttrCert(asn1::Sequence<'a>),
#[implicit(3)]
OtherCertificate(OtherCertificateFormat<'a>),
}

#[derive(asn1::Asn1Write, asn1::Asn1Read)]
pub struct OtherCertificateFormat<'a> {
pub other_cert_format: asn1::ObjectIdentifier,
pub other_cert: asn1::Tlv<'a>,
}

#[derive(asn1::Asn1Write, asn1::Asn1Read)]
pub enum RevocationInfoChoice<'a> {
Crl(asn1::Sequence<'a>),
#[implicit(1)]
Other(OtherRevocationInfoFormat<'a>),
}

#[derive(asn1::Asn1Write, asn1::Asn1Read)]
pub struct OtherRevocationInfoFormat<'a> {
pub other_rev_info_format: asn1::ObjectIdentifier,
pub other_rev_info: asn1::Tlv<'a>,
}

#[derive(asn1::Asn1Write, asn1::Asn1Read)]
pub struct SignerInfo<'a> {
pub version: u8,
Expand Down
Loading