Fix indeterministic behavior of CodiceFiscaleGenerator#633
Fix indeterministic behavior of CodiceFiscaleGenerator#633Mephistofeles wants to merge 1 commit intobchavez:masterfrom
Conversation
Replace .NET GetHashCode with a deterministic FNV-1a hash for city code in ExtensionsForItaly.CodiceFiscale.cs to ensure cross-runtime stability.
bchavez
left a comment
bchavez
left a comment
There was a problem hiding this comment.
Good find. Thank you for bringing up the non-deterministic .GetHashCode() usage. Most definitely we want to remove non-deterministic calls and replace them with deterministic method calls as much as possible.
| unchecked | ||
| { | ||
| // FNV-1a 32-bit: offset basis and prime from the spec | ||
| const uint offsetBasis = 2166136261u; | ||
| const uint prime = 16777619u; | ||
|
|
||
| var hash = offsetBasis; | ||
|
|
||
| foreach (var c in lastName.ToUpperInvariant()) { hash = (hash ^ c) * prime; } | ||
| foreach (var c in firstName.ToUpperInvariant()) { hash = (hash ^ c) * prime; } | ||
| hash = (hash ^ (uint)birthday.Year) * prime; | ||
| hash = (hash ^ (uint)birthday.Month) * prime; | ||
| hash = (hash ^ (uint)birthday.Day) * prime; | ||
| hash = (hash ^ (male ? 1u : 0u)) * prime; | ||
|
|
||
| return hash; | ||
| } |
There was a problem hiding this comment.
Is it possible to have an implementation where we are not using unchecked? I would generally prefer to have an implementation that doesn't require an unchecked block.
There was a problem hiding this comment.
The FNV-1a is a simple algorithm, but it depends on the overflow wrapping.
I thought about using one of the algorithms in the System.IO.Hashing namespace, but it requires .NET 4.6.2+.
I guess I can use for example SHA256 if you don't want unchecked in the code.
There was a problem hiding this comment.
Assuming there's a comparable implementation in System.IO.Hashing and an
open source version in .NET, I suppose we could also copy the BCL
implementation from that namespace for our usages here.
2 participants
Replace .NET GetHashCode with a deterministic FNV-1a hash for city code in ExtensionsForItaly.CodiceFiscale.cs to ensure cross-runtime stability.
Fixes #632