Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
firebird-issue-importer opened this issue Nov 16, 2010 · 29 comments
Closed

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

firebird-issue-importer opened this issue Nov 16, 2010 · 29 comments

Comments

@firebird-issue-importer

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: b31f4d9 87d19fb e616957 6b364d8 1bb24e6

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 16, 2010

Modified by: Christoph Theuring (ctheuring)

Attachment: rfc4122.txt [ 11830 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 17, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 21, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Nov 21, 2010

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Apr 18, 2011

Commented by: @hvlad

Another issue of this kind : DNET376

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 7, 2011

Commented by: @pmakowski

any news on this bug ?

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 7, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 7, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 7, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 7, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 13, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 13, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 13, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 19, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 21, 2011

Modified by: @asfernandes

Link: This issue relate to DOC74 [ DOC74 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Dec 21, 2011

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 10, 2012

Commented by: @asfernandes

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

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 10, 2012

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-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 10, 2012

Modified by: @asfernandes

status: Resolved [ 5 ] => Reopened [ 4 ]

resolution: Fixed [ 1 ] =>

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Jul 11, 2012

Modified by: @asfernandes

status: Reopened [ 4 ] => Resolved [ 5 ]

resolution: Fixed [ 1 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented Mar 29, 2014

Modified by: @pcisar

status: Resolved [ 5 ] => Closed [ 6 ]

@firebird-issue-importer
Copy link
Author

@firebird-issue-importer firebird-issue-importer commented May 29, 2015

Modified by: @pavel-zotov

status: Closed [ 6 ] => Closed [ 6 ]

QA Status: Done successfully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants