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

[Migrated] Bug: cocli corim display #17

Open
deeglaze opened this issue Dec 24, 2024 · 4 comments
Open

[Migrated] Bug: cocli corim display #17

deeglaze opened this issue Dec 24, 2024 · 4 comments

Comments

@deeglaze
Copy link
Collaborator

Source: veraison/corim#69, confirmed still an issue

cocli corim display on a signed corim creates a json and prints "Meta:" and "CoRIM:" in https://github.com/veraison/corim/blob/main/cocli/cmd/corimDisplay.go#L88 and later.

This causes a problem when parsing the JSON file output through python or command line, and appears to be a non-conforming JSON.
Can you confirm this is a bug? and perhaps fix it?

Similarly cocli comid display also outputs the file name with >> at the top of the JSON file and makes it difficult to parse.

@ravjot07
Copy link
Contributor

@deeglaze can we provide a --raw (or similarly named) flag to control whether extra labels/headings are displayed, giving users the choice of purely raw JSON vs. human-friendly headings.

@deeglaze
Copy link
Collaborator Author

I think it is difficult to say if this can be resolved fully, as CoRIM does not have a JSON-friendly CDDL definition.

Anything we say is --json would need to be proposed as an enhancement I-D to the CoRIM spec I think, since the CBOR serialization to JSON takes a design decision that isn't standardized (see nlohmann/json#1129), and the CDDL that binds names to constants is not intended to be a basis of JSON serialization. I don't want the reference implementation of the RFC to also prescribe JSON without a spec.

The display tooling is meant purely for human diagnostics. I'm tempted to close as won't fix, or label as enhancement rather than bug.

@ravjot07
Copy link
Contributor

I think it is difficult to say if this can be resolved fully, as CoRIM does not have a JSON-friendly CDDL definition.

Anything we say is --json would need to be proposed as an enhancement I-D to the CoRIM spec I think, since the CBOR serialization to JSON takes a design decision that isn't standardized (see nlohmann/json#1129), and the CDDL that binds names to constants is not intended to be a basis of JSON serialization. I don't want the reference implementation of the RFC to also prescribe JSON without a spec.

The display tooling is meant purely for human diagnostics. I'm tempted to close as won't fix, or label as enhancement rather than bug.

@deeglaze yaa you are right that's the whole point of display tooling is for human diagnostic, however should we consider adding something like - raw tag? As some of the user faces this issue as I have seen one more issue that had mentioned same problem in corim repo

@deeglaze
Copy link
Collaborator Author

deeglaze commented Dec 24, 2024

I wouldn't reject it outright. I would need to see a PR and see if the other maintainers share my stance.

Edit: If we say it's rendering in the template format, I would be okay with it. I just don't know how the template format handles binary strings.

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

No branches or pull requests

2 participants