Skip to content
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.

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

Add support for parsing BER #42

Merged
merged 1 commit into from
Oct 2, 2023
Merged

Add support for parsing BER #42

merged 1 commit into from
Oct 2, 2023

Conversation

davidzech
Copy link
Contributor

@davidzech davidzech commented Sep 28, 2023

Adds support for parsing BER encoded structures.

  • Refactor common ASN.1 structures to ASN1.swift, and move DER related items to DER.swift.
  • Add BER.swift with full Parsing support
  • Encoding to BER that is not also valid DER is not supported. Only DER will continue to be the only serialization strategy.

@dnadoba
Copy link
Member

dnadoba commented Sep 29, 2023

@swift-server-bot add to allowlist

@dnadoba
Copy link
Member

dnadoba commented Sep 29, 2023

Can you please run swift-format format --parallel --recursive --in-place Sources Tests? We use version 509.0.0.

We currently compile swift-format from source by cloning https://github.com/apple/swift-format and building it with swift build -c=release. Afterwards the binary is at .build/release/swift-format.


In addition you will need to run ./scripts/update_cmakelists.sh to add the new files to the cmake files list.

@davidzech
Copy link
Contributor Author

Can you please run swift-format format --parallel --recursive --in-place Sources Tests? We use version 509.0.0.

We currently compile swift-format from source by cloning https://github.com/apple/swift-format and building it with swift build -c=release. Afterwards the binary is at .build/release/swift-format.

In addition you will need to run ./scripts/update_cmakelists.sh to add the new files to the cmake files list.

Fixed in 7a66b54

@davidzech
Copy link
Contributor Author

davidzech commented Sep 29, 2023

Is this actually an API breakage?

14:27:58 5 breaking changes detected in SwiftASN1:
14:27:58   💔 API breakage: protocol ASN1IntegerRepresentable has generic signature change from <Self : SwiftASN1.DERImplicitlyTaggable, Self.IntegerBytes : Swift.RandomAccessCollection, Self.IntegerBytes.Element == Swift.UInt8> to <Self : SwiftASN1.BERImplicitlyTaggable, Self.IntegerBytes : Swift.RandomAccessCollection, Self.IntegerBytes.Element == Swift.UInt8>
14:27:58   💔 API breakage: protocol ASN1IntegerRepresentable has added inherited protocol BERImplicitlyTaggable
14:27:58   💔 API breakage: protocol ASN1IntegerRepresentable has added inherited protocol BERParseable
14:27:58   💔 API breakage: protocol ASN1IntegerRepresentable has added inherited protocol BERSerializable
14:27:58   💔 API breakage: constructor ASN1IntegerRepresentable.init(berIntegerBytes:) has been added as a protocol requirement

Am I wrong that this should be a transparent change to existing consuming code bases, since the new methods should have been satisfied with extensions.

@Lukasa
Copy link
Collaborator

Lukasa commented Oct 2, 2023

You're correct, these are not breaking changes. The API breakage checker is just a bit pessimistic.

@Lukasa Lukasa added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Oct 2, 2023
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

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

I'm happy with this, I'd like @dnadoba to confirm he is too.

Copy link
Member

@dnadoba dnadoba left a comment

Choose a reason for hiding this comment

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

Great addition! Thanks a lot for implementing this!

@Lukasa Lukasa merged commit 28d0e07 into apple:main Oct 2, 2023
5 of 6 checks passed
@davidzech davidzech deleted the ber-support branch October 2, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants