google / uuid

Go package for UUIDs based on RFC 4122 and DCE 1.1: Authentication and Security Services.
BSD 3-Clause "New" or "Revised" License
5.24k stars 363 forks source link

JSON parsing of empty strings as uuid.Nil #74

Closed maxekman closed 3 years ago

maxekman commented 3 years ago

Would it be possible/desirable to support parsing empty strings in JSON as uuid.Nil?

Currently it generates "invalid UUID length: 0": https://play.golang.org/p/K4y-DFj44Z6

maxekman commented 3 years ago

I would also be interested in any general suggestions on how this can be worked around. In our case we are trying to support JSON from an other UUID library which encodes nil as "".

pborman commented 3 years ago

Probably not ideal, but you could do something like https://play.golang.org/p/om22wkP5OAj

You could use github.com/pborman/uuid instead: https://play.golang.org/p/aZVmu5z0_31.

My concern here is that the uuid.Nil is actually a UUID while "" or JSON's nil implies there is no UUID at all. The UUID type has no way to express that, unless you use a pointer to UUID. This type of change may also break existing code as it would now treat "" and "00000000-0000-0000-0000-000000000000" as the same where before it did not. It is unfortunate that the standard JSON encoder/decoder does not enable the registration of customers encoding/decoding for existing types. (Apparently bson in mongodb does.)

The github.com/pborman/uuid does have the notion of a nonexistent UUID as a UUID is []byte instead of [16]byte. The google package originally was a fork of the pborman package to make uuid.UUID comparable and hence the ability to directly use as a key. The pborman package now is basically a wrapper around this package. (Both packages are by the same author.)

maxekman commented 3 years ago

Thanks for the explanation!

The library I'm authoring (https://github.com/looplab/eventhorizon) just switched away from using our own UUID type in favour of this more widely adopted package, so I would rather not go back to a custom UUID type if possible.

Could it be an alternative (or even possible) to support parsing of "" as nil when the a UUID struct member is a pointer?

type S struct {
    ID *uuid.UUID
}

If that would be possible, client code will be forced to update all structs where UUIDs are optional using this convention.

pborman commented 3 years ago

Did you look at using github.com/pborman/uuid (as I mentioned, it is a wrapper around the google UUID package)? It appears your example works just fine when using the pborman version. https://play.golang.org/p/aZVmu5z0_31

I have no idea how prevalent this issue is in your code base. Is there a limited number of places this impacts you or is it external code that gets impacted?

maxekman commented 3 years ago

Yes, I looked at github.com/pborman/uuid and it looks like it could work. However I would prefer to use github.com/google/uuid for now.

My main reason for supporting parsing "" is to be backward compatible with a relatively large MongoDB database in an external project. I could migrate all empty IDs to uuid.Nil and not be backward compatible in that regard, but due to the nature of the document store with some deeply nested documents it's very hard to guarantee that all IDs are properly migrated. I have to think some more on the trade offs here. Another option is to add support for empty UUID values in the existing custom BSON support: https://github.com/looplab/eventhorizon/blob/master/types/mongodb/uuid.go Really cool that they have this support for encoding of external types, but quiet tricky to work with I have to say!

Thanks for the advice so far!

Also, feel free to close the issue if you wish.

pborman commented 3 years ago

Best of luck. If you have more questions, feel free to re-open or open a new issue.

skagget77 commented 3 years ago

I have a similar problem, I like this UUID library (due to UUIDs being comparable) but need to work with a misbehaving JSON based API. As a workaround I created a small wrapper to allow for unmarshaling of empty and malformed UUIDs:

https://github.com/skagget77/uuidnil

Maybe this can help you out as well.