Issue Details (XML | Word | Printable)

Key: CORE-3238
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Adriano dos Santos Fernandes
Reporter: Christoph Theuring
Votes: 0
Watchers: 4
Operations

If you were logged in you would be able to see more operations.
Firebird Core

Makes GEN_UUID return a compliant RFC-4122 UUID.

Created: 16/Nov/10 12:46 PM   Updated: 29/Mar/14 06:06 PM
Component/s: Engine
Affects Version/s: 2.5.0, 2.5.1
Fix Version/s: 2.5.2, 3.0 Alpha 1

Time Tracking:
Not Specified

File Attachments: 1. Text File rfc4122.txt (58 kB)

Environment: WindowsXP and Linux ubuntu64 10.04 LTS ... and I think all
Issue Links:
Relate
 

Planning Status: Unspecified


 Description  « Hide
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

 All   Comments   Work Log   Change History   Version Control   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Vlad Khorsun added a comment - 17/Nov/10 02:14 PM
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" :)

Vlad Khorsun added a comment - 17/Nov/10 02:19 PM
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

Adriano dos Santos Fernandes added a comment - 17/Nov/10 02:29 PM
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.

Christoph Theuring added a comment - 17/Nov/10 02:51 PM
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 :-)

Adriano dos Santos Fernandes added a comment - 17/Nov/10 02:56 PM
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.

Philippe Makowski added a comment - 17/Nov/10 03:40 PM
Adriano, the problem was raised many times before the final release
see : http://tracker.firebirdsql.org/browse/CORE-1682 ,
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 http://tracker.firebirdsql.org/browse/CORE-2898
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
   


Christoph Theuring added a comment - 17/Nov/10 03:55 PM
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.

Adriano dos Santos Fernandes added a comment - 17/Nov/10 04:42 PM
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.

Philippe Makowski added a comment - 21/Nov/10 09:42 AM
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 CORE-1656 and CORE-1682 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


Christoph Theuring added a comment - 21/Nov/10 12:52 PM
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.

Vlad Khorsun added a comment - 18/Apr/11 11:02 AM
Another issue of this kind : DNET-376

Philippe Makowski added a comment - 07/Dec/11 07:55 AM
any news on this bug ?


Adriano dos Santos Fernandes added a comment - 07/Dec/11 01:45 PM
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?

Philippe Makowski added a comment - 07/Dec/11 02:28 PM
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





Christoph Theuring added a comment - 07/Dec/11 02:41 PM
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

Philippe Makowski added a comment - 07/Dec/11 04:16 PM
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

Adriano dos Santos Fernandes added a comment - 13/Dec/11 02:27 PM
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 <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.
----------------------------------

Adriano dos Santos Fernandes added a comment - 13/Dec/11 02:30 PM
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.

Philippe Makowski added a comment - 13/Dec/11 02:46 PM
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.


Adriano dos Santos Fernandes added a comment - 10/Jul/12 02:58 PM
Renaming ticket and reworking on the solution without CHAR_TO_UUID2 and UUID_TO_CHAR2.