Repository navigation
Improve handling of null and missing elements in OCI JSON files - #6704
Merged
Merged
Conversation
swick
approved these changes
Jun 16, 2026
swick
left a comment
swick
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, just a few details we should clean up before merging.
| } | ||
|
|
||
| if (signature->critical.identity.reference != NULL) | ||
| if (signature->critical.identity.reference != NULL) /* defensive: cannot be demarshaled as NULL */ |
Collaborator
There was a problem hiding this comment.
If we expect this to be non-NULL, an assert is the right tool.
Comment on lines
+62
to
+64
| /* We treat <property>: null the same as missing. While you could | ||
| * have JSON data structures where the distinction is significant, we | ||
| * don't need to handle any such. |
Collaborator
There was a problem hiding this comment.
This deserves to be in its own commit.
| for (i = 0; self->manifests[i] != NULL; i++) | ||
| { | ||
| if (strcmp (self->manifests[i]->platform.architecture, oci_arch) == 0) | ||
| if (g_strcmp0 (self->manifests[i]->platform.architecture, oci_arch) == 0) |
Collaborator
There was a problem hiding this comment.
This also seems different enough from making properties mandatory that it doesn't belong in the same commit.
Mark all properties required by the OCI specification as required; this eliminates a bunch of cases where we were assuming that descriptor->digest was non-NULL, and potentially generating critical errors from g_return_if_fail().
It's legitimate to have manifests listed in an image index that have no platform object, and hence no architecture - avoid crashing if we encounter such a manifest.
We were handling null properties the same as missing properties *except* that the MANDATORY flag allowed null properties but not missing properties. Fix this, so null is disallowed by MANDATORY. When checking signatures, the image identity could only have been NULL if it was null in the input file - so replace a conditional check on it being non-null with an assertion.
owtaylor
force-pushed
the
oci-json-null-missing
branch
from
June 16, 2026 20:04
30f6328 to
fcae93b
Compare
Contributor
Author
|
|
Collaborator
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
<property>: nullin the input the same as a missing property.