FirebirdSQL / firebird

Firebird server, client and tools
https://www.firebirdsql.org/
1.19k stars 204 forks source link

Makes GEN_UUID return a compliant RFC-4122 UUID. [CORE3238] #3609

Closed firebird-automations closed 11 years ago

firebird-automations commented 13 years ago

Submitted by: Christoph Theuring (ctheuring)

Relate to DOC74

Attachments: rfc4122.txt

In compliance with rfc4122 every UUID gives his (build)version at the 1st digit of the 3rd block and ALL generated UUIDs with the same code MUST have the SAME digit at this place xxxxxxxx-xxxx-4xxx-xxxx-xxxxxxxxxxxx (this is an example for a version 4 UUID) The FireBird UUID is a version 4 UUID (?) and must have the digit 4 at this place. But - every time you generate an UUID THIS digit is different - THIS digit is mostly NOT a '4' examples of created UUIDs with FireBird 2.5.0 with SELECT uuid_to_char(gen_uuid()) FROM RDB$DATABASE; A2A1ED80-1824-101D-C7A6-382DB19A57D1 -> seems to be version 1 AE5E1BCF-E6F2-4D93-5517-B23DDCE6F579 -> seems to be version 4 FF1259D0-87D6-8787-D926-1F7E162A4FE1 -> no legal version 8B0E0580-ED77-360C-7D89-37B69FE1E3C5 -> seems to be a version 3 Include in my FreeAdhocUDF there are functions to create UUIDs of version 1 and 4 - and a UDF to read the version from a generated UUID - see http://freeadhocudf.org/documentation_english/dok_eng_uuid.html also with a description

Commits: FirebirdSQL/firebird@b31f4d9b949023c998196c57f313d1d90dce9c3c FirebirdSQL/firebird@87d19fb0c899a25cf6bfc99c3e48c8de46f58239 FirebirdSQL/firebird@e6169577fb2c1753d39e4488e1b9453636d8851c FirebirdSQL/firebird@6b364d8cb344b31e8eeab3da2476d878ecbc1edb FirebirdSQL/firebird@1bb24e6c082ee57c31677d3f11390f117ba322b4

firebird-automations commented 13 years ago
Modified by: Christoph Theuring (ctheuring) Attachment: rfc4122\.txt \[ 11830 \]
firebird-automations commented 13 years ago

Commented by: @hvlad

On POSIX we really generate not UUID's but just a sequence of 16 random bytes reading it from /dev/urandom. So i can confirm we generate wrong (non compliance to RFC) UUID's

On Windows we call CoCreateGuid() which generates correct UUID's (i hope :) The issue on Windows is that UUID_TO_CHAR converts binary UUID's into text represenatation not in correct way as "version" octect is at 17th place instead of 15th as Christoph said.

I don't know if it is really bug or just "implementation details" :)

firebird-automations commented 13 years ago

Commented by: @hvlad

Simple test

execute block returns (uuid_txt char(36), verid1 char(1), verid2 char(1)) as declare uuid_bin char(16) character set octets; declare i int = 100; begin while (i > 0) do begin i = i - 1; uuid_bin = GEN_UUID(); uuid_txt = UUID_TO_CHAR(uuid_bin); verid1 = SUBSTRING(uuid_txt FROM 15 FOR 1); -- here we should have version number verid2 = SUBSTRING(uuid_txt FROM 17 FOR 1); -- here we have it actually suspend; end end

firebird-automations commented 13 years ago

Commented by: @asfernandes

The POSIX code could be changed to generate compliant numbers.

About conversion to/from string, I'm sorry, but it's over. It's very sad that people waits for final version of software to report such type of bugs.

Changing these functions now will have more trouble that benefits, as they're already in use.

firebird-automations commented 13 years ago

Commented by: Christoph Theuring (ctheuring)

Adriano, I don't believe that's over. Users generste UUIDs instead of a int primary-key. They are not interested in the content of the number - they ONLY want to have an UNIQUE primary key. There's good reasons to use the original created UUID-octet instead of the "convertet" text-string at this pk - and I think, they do it - so no problem with the "readable" conversation of this - if this is changed now. Second I think if someone uses the text instead of the octet he is MOSTLY interested in having a UNIQUE ID - and I can't conjure that THIS is allways a unique ID. So we MUST change. And as a allowed comment: I didn't wait until the final release to report this bug - I stumple by coincidence about this (because my own FreeAdhocUDF reports me: this is not a valid rfc4122 UUID). And second command: I don't hope this is the FINAL version of Firebird :-)

firebird-automations commented 13 years ago

Commented by: @asfernandes

This is over because such functions are in use. If one converted a generated UUID to something and store at any place (database, disk, email link) the contrary conversion should generate the original UUID.

Changing this is a major break than don't have a compliant UUID.

firebird-automations commented 13 years ago

Commented by: @pmakowski

Adriano, the problem was raised many times before the final release see : CORE1682 , in fact this one was closed but shouldn't be, because it don't respect RFC and I remember also a thread on devel before the final release that leaded to CORE2898 but this one was a poor solution we need to find a good solution for our poor implementation maybe a chance using this lib : http://www.ossp.org/pkg/lib/uuid/ and having function like uuid_generate_v1(), uuid_generate_v4(), etc or any suitable syntax and let the actual bad function like they are but deprecate them

firebird-automations commented 13 years ago

Commented by: Christoph Theuring (ctheuring)

Philippe - I 100% agree with you. We have (LPGL) code in C for exactly these implementations of UUID in FreeAdhocUDF. If it is possible (and you want) to use it - do so.

firebird-automations commented 13 years ago

Commented by: @asfernandes

Philippe, the tickets you mentioned mentions RFC4122 always explicitly meaning its string representation.

FWIW, I consider funny a group of people meeting to create a RFC to generate random numbers that's not so random. I was no idea about these peculiarities.

Then in FEB-2010, it was asked (UUID is not RFC 4122 compliant on posix) in fb-devel and I replied: > Maybe you can submit a patch so it be compliant? > Of course, code shouldn't be taken from sources with incompatible license like one above.

At parallel discussion, there were no real objections to maintain non-compliant UUIDs.

And the problem is not with generate function. It could be changed and generate compliant numbers. But functions CHAR_TO_UUID and UUID_TO_CHAR should not change their behaviors.

So if you want to make it right, maybe is better to wait for when we had a GUID data type.

firebird-automations commented 13 years ago

Commented by: @pmakowski

The problem is that even in release note we claim that we use "RFC4122-compliant form", in fact it is wrong, as Vlad proved it so really "Houston, we have a problem" about waiting a GUID data type, why ?, using now CHAR(16) OCTETS string is ok so or the RN have to be changed and CORE1656 and CORE1682 have to be commented or we have to find a new solution I know that in Feb the discussion did not ended well ossp code licence is ok for us

firebird-automations commented 13 years ago

Commented by: Christoph Theuring (ctheuring)

What's facts: - On POSIX we ... generate wrong (non compliance to RFC) UUID's (Vlad) - On Windows we ... generates correct UUID's (i hope :) (Vlad) - The issue on Windows is that UUID_TO_CHAR converts binary UUID's into text represenatation not in correct way (Vlad) - we need to find a good solution for our poor implementation (Philippe) - in release note we claim that we use "RFC4122-compliant form", in fact it is wrong (Philippe) - the users want a "readable" (text-form) UUID - the UUID must be unique and RFC4122 so where is the problem to do, what Philippe suggested: - having new functions like uuid_generate_v1(), uuid_generate_v4() ... or any suitable syntax, generated in CHAR(16) OCTETS (best for to allow better performance when used as primary keys - Dimitry) - having new functions to convert them to "readable" text-representation (FORMAT_UUID - Dimitry) - let the actual bad function like they are but deprecate them (Philippe) - changing the RN All is sayed - let's do it.

firebird-automations commented 13 years ago

Commented by: @hvlad

Another issue of this kind : DNET376

firebird-automations commented 12 years ago

Commented by: @pmakowski

any news on this bug ?

firebird-automations commented 12 years ago

Commented by: @asfernandes

As I said, GEN_UUID can be changed, but it's string conversion functions are over. No way.

Is that GEN_UUID only change that's being requested here?

firebird-automations commented 12 years ago

Commented by: @pmakowski

no

let GEN_UUID and UUID_TO_CHAR and CHAR_TO_UUID like they are correct all the documention and clearly stand that they are not RFC4122 compliant

and

add new implementation RFC4122 compliant such as : uuid_generate_v1(), uuid_generate_v4() and add new functions to convert them to "readable" text-representation and make them RFC4122 compliant

at least V4

today all the uuid stuff firebird compliant only and even give different result on different plateforms

see the simple Vlad test under Linux : UUID_TXT VERID1 VERID2 ==================================== ====== ====== 891CA467-8A23-EC5A-BAFC-C4A5F4385D42 E 5
37197654-9890-5652-1E07-B7C07703FDBF 5 5
257632BB-D472-C546-2E08-C33E795B8A45 C 4
E781A1DA-17DB-F101-776C-1172E07A3B1D F 0
671ED8F0-AC77-F75A-DF17-74AE5AD5DB6B F 5

under windows :

UUID_TXT VERID1 VERID2 ==================================== ====== ====== B0AC6126-E883-F746-BED1-9E1F259F739F F 4
28C0A6BD-3F90-7F48-93D5-0030ECD2CD97 7 4
F7C16C35-0CA7-F345-AF30-80D38C7C3C24 F 4
12817D3F-FEEF-E94B-9F01-92579E2BCDBC E 4
95C8EC0B-5EEC-9D46-8137-EDF49CE0D0E5 9 4

that's bad

firebird-automations commented 12 years ago

Commented by: Christoph Theuring (ctheuring)

Philippe - 100% agree again We have (LPGL) code in C for exactly these implementations of UUID in FreeAdhocUDF for Windows and Linux see ftp://ftp.freeadhocudf.org/FreeAdhocUDF/adhoc20101206/source/adhoc/uuid_functions.c

Christoph Theuring

firebird-automations commented 12 years ago

Commented by: @pmakowski

LGPL is not ok for us but maybe a chance using this lib : http://www.ossp.org/pkg/lib/uuid/ ossp code licence is ok for us

firebird-automations commented 12 years ago

Commented by: @asfernandes

I don't think we should import more sources in our tree, since we already get rid of ICU ones, and distros don't like it.

At least in ubuntu there is libuuid/uuid-dev, which appears to be a different package than the one you mentioned.

License below:

---------------------------------- This is libuuid, previously part of e2fsprogs this is now part of util-linux-ng and has thus moved to the util-linux Debian source package.

Upstream Author: Theodore Ts'o <mailto:tytso@mit.edu>

Copyright:

Copyright (C) 1999, 2000, 2003, 2004 by Theodore Ts'o

Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: 1. Redistributions of source code must retain the above copyright notice, and the entire permission notice in its entirety, including the disclaimer of warranties. 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. 3. The name of the author may not be used to endorse or promote products derived from this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ----------------------------------

firebird-automations commented 12 years ago

Commented by: @asfernandes

Or even much better. Define FB uuid as version 4 (random) only, and fix the version field.

We don't need a new GEN_UUID function. Current one works ok in Windows already, and the fix for POSIX will not cause any problem.

firebird-automations commented 12 years ago

Commented by: @pmakowski

ok with your last proposal (Define FB uuid as version 4 (random) only, and fix the version field. etc...)

Version 4 (random)

Version 4 UUIDs use a scheme relying only on random numbers. This algorithm sets the version number as well as two reserved bits. All other bits are set using a random or pseudorandom data source. Version 4 UUIDs have the form xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx where x is any hexadecimal digit and y is one of 8, 9, A, or B. e.g. f47ac10b-58cc-4372-a567-0e02b2c3d479.

firebird-automations commented 12 years ago
Modified by: @asfernandes Version: 2\.5\.1 \[ 10333 \] Component: Engine \[ 10000 \] assignee: Adriano dos Santos Fernandes \[ asfernandes \] summary: the uuid\_to\_char\(gen\_uuid\(\)\) function/s create \(or convert ?\) not a valid rfc4122 UUID =\> Makes GEN\_UUID return a compliant RFC\-4122 binary UUID and introduce CHAR\_TO\_UUID2 and UUID\_TO\_CHAR2 to convert UUIDs from/to string also complying with the RFC
firebird-automations commented 12 years ago
Modified by: @asfernandes Link: This issue relate to [DOC74](https://github.com/FirebirdSQL/firebird-documentation/issues?q=DOC74+in%3Atitle) \[ [DOC74](https://github.com/FirebirdSQL/firebird-documentation/issues?q=DOC74+in%3Atitle) \]
firebird-automations commented 12 years ago
Modified by: @asfernandes status: Open \[ 1 \] =\> Resolved \[ 5 \] resolution: Fixed \[ 1 \] Fix Version: 3\.0 Alpha 1 \[ 10331 \] Fix Version: 2\.5\.2 \[ 10450 \]
firebird-automations commented 11 years ago

Commented by: @asfernandes

Renaming ticket and reworking on the solution without CHAR_TO_UUID2 and UUID_TO_CHAR2.

firebird-automations commented 11 years ago
Modified by: @asfernandes summary: Makes GEN\_UUID return a compliant RFC\-4122 binary UUID and introduce CHAR\_TO\_UUID2 and UUID\_TO\_CHAR2 to convert UUIDs from/to string also complying with the RFC =\> Makes GEN\_UUID return a compliant RFC\-4122 UUID\.
firebird-automations commented 11 years ago
Modified by: @asfernandes status: Resolved \[ 5 \] =\> Reopened \[ 4 \] resolution: Fixed \[ 1 \] =\>
firebird-automations commented 11 years ago
Modified by: @asfernandes status: Reopened \[ 4 \] =\> Resolved \[ 5 \] resolution: Fixed \[ 1 \]
firebird-automations commented 10 years ago
Modified by: @pcisar status: Resolved \[ 5 \] =\> Closed \[ 6 \]
firebird-automations commented 8 years ago
Modified by: @pavel-zotov status: Closed \[ 6 \] =\> Closed \[ 6 \] QA Status: Done successfully