Skip to content

fix: protojson non-determinism in ID generation#4967

Merged
cuixq merged 1 commit into
google:masterfrom
michaelkedar:i-hate-protojson
Mar 4, 2026
Merged

fix: protojson non-determinism in ID generation#4967
cuixq merged 1 commit into
google:masterfrom
michaelkedar:i-hate-protojson

Conversation

@michaelkedar

Copy link
Copy Markdown
Member

My tests in #4966 were failing because protojson decided it wanted to add two spaces between the keys and values in its output.
Run the protojson output through the json.Indent to format it consistently.

That said, does the formatting of the output actually matter here? Should we instead rewrite the tests to do a semantic comparison instead of a string comparison?

@michaelkedar michaelkedar requested a review from cuixq March 4, 2026 05:34
@cuixq

cuixq commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

I actually thought about doing the semantic comparison but decided to do string comparison considering the use case is to write the updated vulnerability file. However, maybe the actual ordering of each fields does not matter too much as well as whether the diffs are minimized since the file name is going to be updated too.

@cuixq cuixq merged commit 4fcedbd into google:master Mar 4, 2026
21 checks passed
@michaelkedar michaelkedar deleted the i-hate-protojson branch March 4, 2026 22:35
tymzd pushed a commit to tymzd/osv.dev that referenced this pull request Apr 13, 2026
My tests in google#4966 were failing because `protojson` decided it wanted to
add two spaces between the keys and values in its output.
Run the `protojson` output through the `json.Indent` to format it
consistently.

That said, does the formatting of the output actually matter here?
Should we instead rewrite the tests to do a semantic comparison instead
of a string comparison?
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