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

CBOR correctly skips particular values #2873

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

ardune
Copy link
Contributor

@ardune ardune commented Dec 5, 2024

The CBOR "ignoreUnknownKeys" allows for arbitrary content to be skipped.
In the case of an empty map, empty array, or tags on non-map/array values the skip would fail

Included somewhat trimmed down example test.

@sandwwraith
Copy link
Member

Have you filed an issue somewhere already or you've decided to do a PR right away?

@ardune
Copy link
Contributor Author

ardune commented Dec 10, 2024

I just made a PR - Apologies if I skipped a step. I would be happy to open an issue or otherwise add to the PR.

if (header == HEADER_ARRAY || header == HEADER_MAP) {
if (header == HEADER_TAG) {
readNumber()
} else if (header == HEADER_ARRAY || header == HEADER_MAP) {
Copy link
Member

Choose a reason for hiding this comment

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

cc @JesusMcCloud as author of these changes. I do not see any particular problems, so I'll merge this if you don't have other comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Looks good to me!

Copy link
Member

@sandwwraith sandwwraith left a comment

Choose a reason for hiding this comment

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

Thank you!

@sandwwraith sandwwraith merged commit 7204a36 into Kotlin:dev Dec 10, 2024
3 checks passed
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