issue with uuidv5 and/or parse function - Invalid UUID

[afterthought]

Describe the bug

A GUID generated in dotnet is failing to parse. We get an error: Invalid UUID. We encountered the issue with v5, but it's the same error if you pass to the new parse function. I ran git bisect using this test and it fails starting on commit: 0e6c10b.

    describe('v5', () => {
       test('should parse', () => {
       const id1 = v5('test', 'de42f6ff-210c-da9c-a30e-1a4ac2743656');
  
       });
    });

How to reproduce

Run this test:

    describe('v5', () => {
       test('should parse', () => {
       const id1 = v5('test', 'de42f6ff-210c-da9c-a30e-1a4ac2743656');
  
       });
    });

Expected behavior

Should produce a v5 uuid.

Runtime

• OS: [macOS, Linux]
• Runtime: [e.g. Node.js, AWS Lambda]
• Runtime Version: [e.g. 12x]

Additional information

[Any other information or comments that you think will help]

[ctavan]

@afterthought can you provide more details on how you generated the "GUID" de42f6ff-210c-da9c-a30e-1a4ac2743656?

It does not appear to be an RFC4122 compliant UUID: The first character after the second - in your UUID is a d. However valid UUIDs have the version number at this position which must be one of 1, 3, 4, 5.

We did include more thorough checks in uuid@8.3.0 to verify that the passed UUID namespaces are actually valid UUIDs which is why you now see an error.

[broofa]

FWIW, I think there's some misconceptions around the synonymity of Microsoft GUIDs and RFC4122. Lots of articles out there conflating these two terms, often claiming they're identical. However, Microsoft is pretty clear that GUIDs are allowed to contain any hex digit.

As @ctavan notes, RFC UUIDs have specific requirements for the version and variant fields, whereas GUIDS can have any hex digts there. I.e. The current behavior - where this library considers Microsoft GUIDs that don't comply with the RFC spec to be invalid - is the intended behavior.

[afterthought]

Ok. So I was wrong about it coming from dotnet. It actually originates from an old titanium based mobile app with a handwritten random number generator that just orders the chars/digits into a guid format. Pretty bad. (I did do some testing with the MS Guid.newGuid() method and it seems compliant).

I definitely see that the ID is non-compliant and now we know why. I'm now in the pinch of figuring out how to move forward. Our design/architecture relies heavily on uuidv5 and now I've found that I have a mountain of legacy data that won't be compatible. For now I can pin the library at 8.2. Not a great long-term solution though. On my end I'm going to see if I can convince the product team to fix the uuid generation and ideally have a conversion strategy for older data.

[ctavan]

@broofa since this seems to be popping up a few times and since legacy data may be hard or impossible to change: How do you feel about introducing an optional lax option to uuid.v5/3 that restores the pre-uuid@8.3 lax checking?

[broofa]

@afterthought If you pass the namespace as a byte array, it circumvents the parse() (and subsequent validation check). Would that work for you ?

E.g.

const {v5} = require('.')

const NAMESPACE = Uint8Array.from([
      0xDE, 0x42, 0xF6, 0xFF,
      0x21, 0x0C,
      0xDA, 0x9C,
      0xA3, 0x0E,
      0x1A, 0x4A, 0xC2, 0x74, 0x36, 0x56,
    ]);

v5('test', NAMESPACE); // => 'bd6be041-a2ef-552b-98e2-7c6a78301b1c'```

[afterthought]

Yes, elegant backdoor. Thanks for the help.

[ctavan]

@broofa is this workaround a behavior we want to have/encourage? Or rather make this an explicit option to allow skipping the namespace validation?

[broofa]

@ctavan Good question. Short answer: No, but I'm not sure there's a great solution here.

That byte-array-based namespaces aren't validated but string ones are is an inconsistency that should probably get addressed at some point so... yeah... @afterthought probably shouldn't count on this continuing to work in the future. But I don't see a compelling reason to close that particular loophole at the moment.

The alternative - adding an option to allow non-standard namespaces - flies in the face of the "this module adheres to the RFC" philosophy, so I'm a bit reticent to go that route.

'Kind of feels like we have a passable solution for the moment so maybe we can get away with looking the other way on this one until there's a reason to do otherwise. 🤷

[pbleuse-orange]

Hello,

I'm not an expert but If I resume, variant microsoft is GUID and not RFC compliant with UUID, but appear in RFC 4.1.1 well maybe just an interpretation, but I understand.
In all case, if this lib not handling GUID, why when I have search "GUID" in npm search, first result is this library with GUID tag ?
Why in documentation you not add a simply alert "GUID is not handling" with little explanation ?
Ok you will said "it's RFC", but seriously, you think all people know RFC in detail ? ;)
And when I go in more oneline tools, there is no differences, like here: https://www.uuidtools.com/

This lib is a very very good job, but my opinion is in this point think to what user/developper need and what you provide

Thank :)

[ctavan]

@pbleuse-orange did you see this comment: #511 (comment)

The GUID which caused this issue was not generated using any standards compliant UUID/GUID generator.

If you manually generate things, that look like UUIDS/GUIDS but don’t follow the standard, then please don’t expect this library to accept these things as being valid UUIDS/GUIDS.

And to emphasize again: the original issue described here was caused by faulty manual
GUID generation, not by using a well tested standard library.

[pbleuse-orange]

@ctavan sorry I would'nt relauch all discussion, but for my part I have problem with this ID for example:

6023a8a5-d9fe-5142-d062-5f38fe5142d0

It was decoded here https://www.uuidtools.com/api/decode/6023a8a5-d9fe-5142-d062-5f38fe5142d0 with this response:

{
    "encode": {
        "STR": "6023a8a5-d9fe-5142-d062-5f38fe5142d0",
        "SIV": "127791038570326569133610857067145413328"
    },
    "decode": {
        "variant": "reserved (Microsoft GUID)",
        "version": "5 (name based, SHA-1)",
        "contents": "60:23:A8:A5:D9:FE:01:42:10:62:5F:38:FE:51:42:D0\n(not decipherable: truncated SHA-1 message digest only)"
    }
}

You can see variant microsoft. But uuid.validate() said this ID is invalid, that's it.
But maybe my reply is not in good discussion because it's valid GUID, I apologize for that.

[broofa]

First line of RFC4122 [emphasis mine]:

This specification defines a Uniform Resource Name namespace for UUIDs (Universally Unique IDentifier), also known as GUIDs (Globally Unique IDentifier).

[Edit to add: While "GUID" and "UUID" are technically different things, they are closely linked in appearance, ancestry, and everyday usage. Hence, why "guid" is one of the keywords named in this module. That the RFC mentions GUID as synonymous with UUID doesn't help matters.]