Bug when parsing a SequentialGuid.
Closed this issue · 4 comments
FromSequentialGuid
incorrectly swaps the bytes of the sequence counter, this means that parsing SequentialGuid creates incorrect NewIds. Tests won't detect this because the first NewId generated has the sequence number 0x0000.
This bug was introduced in #20 affecting FromSequentialGuid
and ToNewIdFromSequential
.
Expected: 9bfb0100-e3f8-6a00-06a1-08daed27ef21
But was: 9bfb0001-e3f8-6a00-06a1-08daed27ef21
Example failing test
// Round trip, to SequentialGuid and back with FromSequentialGuid
[Test]
public void Should_parse_sequential_guid_2_as_newid()
{
NewId n = NewId.Next(2)[1];
var nn = n.ToGuid();
var g = n.ToSequentialGuid();
var ng = NewId.FromSequentialGuid(g);
Assert.AreEqual(n, ng);
// Also checks to see if this would throw
Assert.IsTrue(ng.Timestamp != default);
}
Fix
static void FromSequentialByteArray(in byte[] bytes, out Int32 a, out Int32 b, out Int32 c, out Int32 d)
{
a = bytes[3] << 24 | bytes[2] << 16 | bytes[1] << 8 | bytes[0];
b = bytes[5] << 24 | bytes[4] << 16 | bytes[7] << 8 | bytes[6];
c = bytes[8] << 24 | bytes[9] << 16 | bytes[10] << 8 | bytes[11];
d = bytes[12] << 24 | bytes[13] << 16 | bytes[14] << 8 | bytes[15];
}
Any chance a PR is incoming?
Sorry I should have been clearer. I wasn't 100% sure if this was a bug or not. I'm not familiar with SequentialGuid
.
I'll make a PR now.
Oh, no worries. Yeah, if we are unable to round trip an identifier there and back, that's a bug :)
Oh, no worries. Yeah, if we are unable to round trip an identifier there and back, that's a bug :)
It was midnight when I found it, I wasn't 100% there 😄. Wasn't sure what the intended layout/behaviour of ToGuid
and ToSequentialGuid
is. I spent ages looking at the generated guid table from #18 and couldn't figure out why my sequence layout was different. If I had just read the thread I would have understood that it changed the sequence layout 🤷
Original | Current |
---|---|
xxxx0001-xxxx-xxxx-15ad-08d89a158ab8 | xxxx0100-xxxx-xxxx-15ad-08d89a158ab8 |