phatboyg/NewId

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