Skip to content

feat: expose asn1 parse options - #141

Merged
microshine merged 3 commits into
PeculiarVentures:masterfrom
hacdias:feat/asn1-parse-options
Aug 19, 2026
Merged

feat: expose asn1 parse options#141
microshine merged 3 commits into
PeculiarVentures:masterfrom
hacdias:feat/asn1-parse-options

Conversation

@hacdias

@hacdias hacdias commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

This exposes the ASN.1 parse options for the reasons described in PeculiarVentures/asn1-schema#134.

The options are exposed on the larger parsing entry points such as certificates, CRLs, CSRs, public keys, and certificate collections. They are also propagated through nested/lazy ASN.1 parsing (including extensions and attributes), so caller-provided resource limits remain consistent when inspecting nested values.

@simonlerpard

Copy link
Copy Markdown

A CRL I consume grew past asn1js’s 10,000-node default, causing new X509Crl(raw) to throw and revocation checking to fail closed. I don't think there's any good workaround through the current public API, so this change would be very useful. 😃

@hacdias

hacdias commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@microshine could you take a look at this PR and #140, please?

@microshine microshine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for working on this — exposing the ASN.1 resource limits is useful, especially for large CRLs and for callers parsing untrusted input.

I’d like to request one change around how the options are propagated. The top-level objects keep and reuse ParseOptions, but some nested ASN.1 payloads are parsed later without those options. For example, certificate extensions go through ExtensionFactory.create(...), and extension implementations such

@hacdias

hacdias commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@microshine done. My initial argument (is in the PR above) was that those functions only deal with relatively small parts and that it wouldn't be likely needed, but they're added now.

@microshine microshine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks — the nested ParseOptions propagation looks much better now, and the new regression tests cover the important lazy parsing paths.

There’s one remaining issue in X509Certificates.import(): it assigns this.#options = options before the incoming CMS has been parsed successfully. If parsing throws, the existing certificate collection remains intact, but its stored parse options have already been replaced by the options from the failed import. That leaves the object in a partially mutated/inconsistent state and can make later calls such as toString("asn") or toTextObject() fail under limits that were never successfully imported.

Could we make the import transactional here — parse/build the new certificates first, then update the collection and #options only after everything succeeds? A regression test where a failed import leaves the previous collection/options unchanged would be useful as well.

@hacdias

hacdias commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@microshine done

@microshine
microshine merged commit 33b5d6a into PeculiarVentures:master Aug 19, 2026
5 checks passed
@hacdias
hacdias deleted the feat/asn1-parse-options branch August 19, 2026 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants