Skip to content

Conversation

@hennk
Copy link

@hennk hennk commented Jul 28, 2025

Name of feature:

Fix parsing of indefinite-length but empty structures

Pain or issue this feature alleviates:

Some PKCS7 document may contain indefinite but empty structures like sequences or sets, which currently provoke a "ber2der" error during parsing

This fix solves this by moving the check for the end of the indefinite structure to the top of the recursive loop, avoiding trying to parse the content of an empty structure.

@hslatman
Copy link
Member

hslatman commented Jul 29, 2025

Hey @hennk, thank you for opening this PR 🙂

The change looks OK, but since it's touching on a part of readObject, I'm curious if the changes in #40 also make it work for you. If that is the case, then I think it would be better to go with that PR instead, and then I'll have a good look at that 🙂

@hennk
Copy link
Author

hennk commented Jul 29, 2025

I executed the new TestBer2Der_EmptyIndefinite test with #40 and it also fails. The logic to check for the end of an indefinite-length element on https://github.com/ibukanov/pkcs7/blob/main/ber.go#L178 is also only applied after trying to parse at least one inner element, which will fail if it is an empty indefinite-length element (for example a sequence/set without any items).

I'd say the same fix could be applied to #40, though, by moving the if-block from https://github.com/ibukanov/pkcs7/blob/main/ber.go#L178 to the top of the outer for loop

@hslatman
Copy link
Member

Al right, thank you for testing @hennk 🙂

It probably is best to review and merge #40 before this, and then rebasing on top of that then.

@hslatman hslatman self-assigned this Jul 29, 2025
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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