Compute hash of OpenApi document#985
Compute hash of OpenApi document#985MaggieKimani1 merged 16 commits intovnextmicrosoft/OpenAPI.NET:vnextfrom mk/enhancement-create-hash-codemicrosoft/OpenAPI.NET:mk/enhancement-create-hash-codeCopy head branch name to clipboard
Conversation
…Api document and its property values
baywet
left a comment
There was a problem hiding this comment.
I don't think we should be using the GetHashCode override for multiple reasons:
- it's not its intended use, especially if caller want to store the hash somewhere for later comparison
- it can yield equality between unequal objects
- it will vary depending on chip instruction set (x86, 64, arm, arm64)
- it requires also overriding the Equals method
I'd instead recommend using a proper hashing algorithm (configurable?) like sha256 or up.
see the documentation of get hash code.
https://docs.microsoft.com/en-us/dotnet/api/system.object.gethashcode?view=net-6.0#remarks
| { | ||
| // select two random prime numbers e.g 1 and 3 and use them to compute hash codes | ||
| int hash = 1; | ||
| hash = hash * 3 + (Workspace == null ? 0 : Workspace.GetHashCode()); |
There was a problem hiding this comment.
I think you're supposed to select different prime numbers.
I leaned more towards overriding
|
Nitpicking: hashing and encryption are two completely different things :) I'm not suggesting to rely on any encryption algorithm, simply on standard hashing. The nice additional thing with using industry standard hashing is that if we document the process correctly, other libs (from other languages) could interop with it and generate the exact same hash for the same document. Serialize and convert: well that's the beauty of it, we already have the infrastructure to serialize the document to JSON/YAML, let's use that to produce a terse representation (so white-spacing doesn't impact the result), convert to binary, hash, and voilà! You don't even need to implement changes on the whole object model anymore, just on the document or root object. What do you think? Also @darrelmiller, any objection with this approach? |
baywet
left a comment
There was a problem hiding this comment.
Thanks for making the changes, a few minor comments.
We should also make sure the serialization is terse when we hash
baywet
left a comment
There was a problem hiding this comment.
thanks for making the changes, here are a few final remarks
Fixes #799