Skip to content

Add decoding and encoding of ObjectIdentifier ASN.1 records #1849

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

Merged
merged 8 commits into from
Jun 19, 2025
Merged

Conversation

seladb
Copy link
Owner

@seladb seladb commented Jun 17, 2025

As part of #1835, X509 certificates require support for ASN.1 records of type ObjectIdentifier

Copy link

codecov bot commented Jun 17, 2025

Codecov Report

Attention: Patch coverage is 93.33333% with 13 lines in your changes missing coverage. Please review.

Project coverage is 83.00%. Comparing base (a87c74a) to head (d690947).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
Packet++/src/Asn1Codec.cpp 94.64% 6 Missing ⚠️
Packet++/header/Asn1Codec.h 76.19% 4 Missing and 1 partial ⚠️
Tests/Packet++Test/Tests/Asn1Tests.cpp 96.72% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1849      +/-   ##
==========================================
+ Coverage   82.96%   83.00%   +0.04%     
==========================================
  Files         285      285              
  Lines       49063    49257     +194     
  Branches    10599    10459     -140     
==========================================
+ Hits        40707    40888     +181     
- Misses       7196     7567     +371     
+ Partials     1160      802     -358     
Flag Coverage Δ
alpine320 75.09% <86.55%> (+0.05%) ⬆️
fedora42 75.20% <87.80%> (+0.08%) ⬆️
macos-13 80.51% <89.47%> (+0.03%) ⬆️
macos-14 80.51% <89.47%> (+0.03%) ⬆️
macos-15 80.49% <89.47%> (+0.03%) ⬆️
mingw32 71.25% <75.60%> (-0.07%) ⬇️
mingw64 71.32% <75.60%> (+0.02%) ⬆️
npcap 84.96% <91.81%> (+0.03%) ⬆️
rhel94 74.97% <88.03%> (+0.05%) ⬆️
ubuntu2004 58.67% <88.78%> (+0.05%) ⬆️
ubuntu2004-zstd 58.79% <88.78%> (+0.05%) ⬆️
ubuntu2204 74.89% <88.03%> (+0.05%) ⬆️
ubuntu2204-icpx 61.50% <66.66%> (+0.03%) ⬆️
ubuntu2404 75.16% <86.55%> (+0.06%) ⬆️
ubuntu2404-arm64 75.13% <86.55%> (+0.05%) ⬆️
unittest 83.00% <93.33%> (+0.04%) ⬆️
windows-2019 84.98% <91.96%> (+0.03%) ⬆️
windows-2022 84.99% <91.81%> (+0.03%) ⬆️
winpcap 85.13% <91.96%> (+0.03%) ⬆️
xdp 50.65% <88.03%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@seladb seladb changed the title [DRAFT] Add decoding and encoding of ObjectIdentifier ASN.1 records Add decoding and encoding of ObjectIdentifier ASN.1 records Jun 17, 2025
@seladb seladb marked this pull request as ready for review June 17, 2025 09:36
Copy link
Collaborator

@tigercosmos tigercosmos left a comment

Choose a reason for hiding this comment

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

Programming-wide LGTM.

std::vector<uint32_t> components;

uint8_t firstByte = data[currentByteIndex++];
components.push_back(static_cast<uint32_t>(firstByte / 40));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider explaining what the 40 magic number is in the comments.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added documentation here: d690947

{
uint8_t byte = data[currentByteIndex++];

currentComponentValue = (currentComponentValue << 7) | (byte & 0x7f);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe leave some comments here? Like doing which step in the spec.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added documentation here: d690947

@tigercosmos tigercosmos merged commit 644e4ff into dev Jun 19, 2025
43 checks passed
@tigercosmos tigercosmos deleted the asn1-oid branch June 19, 2025 08:24
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.

2 participants