Hi,
On 8/26/22 19:21, Tom Lane wrote:
Nazir Bilal Yavuz <byavu...@gmail.com> writes:
Based on these discussions, I attached a patch.
I think the right fix is to call uuid_create and then actually check
the version field of the result. This avoids breaking what need not
be broken, and it'd also guard against comparable problems on other
platforms (so don't blame NetBSD specifically in the message, either).
I updated my patch. I checked version field in 'uuid_generate_internal'
function instead of checking it in 'uuid_generate_v1' and
'uuid_generate_v1mc' functions, but I have some questions:
1 - Should it be checked only for '--with-uuid=bsd' option?
1.1 - If it is needed to be checked only for '--with-uuid=bsd',
should just NetBSD be checked?
2 - Should it error out without including current UUID version in the
error message? General error message could mask if the 'uuid_create'
function starts to produce UUIDs other than version-4.
Regards,
Nazir Bilal Yavuz
diff --git a/contrib/uuid-ossp/expected/uuid_ossp_1.out b/contrib/uuid-ossp/expected/uuid_ossp_1.out
new file mode 100644
index 0000000000..e9db9d9b1b
--- /dev/null
+++ b/contrib/uuid-ossp/expected/uuid_ossp_1.out
@@ -0,0 +1,111 @@
+CREATE EXTENSION "uuid-ossp";
+SELECT uuid_nil();
+ uuid_nil
+--------------------------------------
+ 00000000-0000-0000-0000-000000000000
+(1 row)
+
+SELECT uuid_ns_dns();
+ uuid_ns_dns
+--------------------------------------
+ 6ba7b810-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+SELECT uuid_ns_url();
+ uuid_ns_url
+--------------------------------------
+ 6ba7b811-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+SELECT uuid_ns_oid();
+ uuid_ns_oid
+--------------------------------------
+ 6ba7b812-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+SELECT uuid_ns_x500();
+ uuid_ns_x500
+--------------------------------------
+ 6ba7b814-9dad-11d1-80b4-00c04fd430c8
+(1 row)
+
+-- some quick and dirty field extraction functions
+-- this is actually timestamp concatenated with clock sequence, per RFC 4122
+CREATE FUNCTION uuid_timestamp_bits(uuid) RETURNS varbit AS
+$$ SELECT ('x' || substr($1::text, 15, 4) || substr($1::text, 10, 4) ||
+ substr($1::text, 1, 8) || substr($1::text, 20, 4))::bit(80)
+ & x'0FFFFFFFFFFFFFFF3FFF' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_version_bits(uuid) RETURNS varbit AS
+$$ SELECT ('x' || substr($1::text, 15, 2))::bit(8) & '11110000' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_reserved_bits(uuid) RETURNS varbit AS
+$$ SELECT ('x' || substr($1::text, 20, 2))::bit(8) & '11000000' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_multicast_bit(uuid) RETURNS bool AS
+$$ SELECT (('x' || substr($1::text, 25, 2))::bit(8) & '00000001') != '00000000' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_local_admin_bit(uuid) RETURNS bool AS
+$$ SELECT (('x' || substr($1::text, 25, 2))::bit(8) & '00000010') != '00000000' $$
+LANGUAGE SQL STRICT IMMUTABLE;
+CREATE FUNCTION uuid_node(uuid) RETURNS text AS
+$$ SELECT substr($1::text, 25) $$
+LANGUAGE SQL STRICT IMMUTABLE;
+-- Ideally, the multicast bit would never be set in V1 output, but the
+-- UUID library may fall back to MC if it can't get the system MAC address.
+-- Also, the local-admin bit might be set (if so, we're probably inside a VM).
+-- So we can't test either bit here.
+SELECT uuid_version_bits(uuid_generate_v1()),
+ uuid_reserved_bits(uuid_generate_v1());
+ERROR: uuid_create function is not generating version-1 UUIDs
+-- Although RFC 4122 only requires the multicast bit to be set in V1MC style
+-- UUIDs, our implementation always sets the local-admin bit as well.
+SELECT uuid_version_bits(uuid_generate_v1mc()),
+ uuid_reserved_bits(uuid_generate_v1mc()),
+ uuid_multicast_bit(uuid_generate_v1mc()),
+ uuid_local_admin_bit(uuid_generate_v1mc());
+ERROR: uuid_create function is not generating version-1 UUIDs
+-- timestamp+clock sequence should be monotonic increasing in v1
+SELECT uuid_timestamp_bits(uuid_generate_v1()) < uuid_timestamp_bits(uuid_generate_v1());
+ERROR: uuid_create function is not generating version-1 UUIDs
+SELECT uuid_timestamp_bits(uuid_generate_v1mc()) < uuid_timestamp_bits(uuid_generate_v1mc());
+ERROR: uuid_create function is not generating version-1 UUIDs
+-- Ideally, the node value is stable in V1 addresses, but OSSP UUID
+-- falls back to V1MC behavior if it can't get the system MAC address.
+SELECT CASE WHEN uuid_multicast_bit(uuid_generate_v1()) AND
+ uuid_local_admin_bit(uuid_generate_v1()) THEN
+ true -- punt, no test
+ ELSE
+ uuid_node(uuid_generate_v1()) = uuid_node(uuid_generate_v1())
+ END;
+ERROR: uuid_create function is not generating version-1 UUIDs
+-- In any case, V1MC node addresses should be random.
+SELECT uuid_node(uuid_generate_v1()) <> uuid_node(uuid_generate_v1mc());
+ERROR: uuid_create function is not generating version-1 UUIDs
+SELECT uuid_node(uuid_generate_v1mc()) <> uuid_node(uuid_generate_v1mc());
+ERROR: uuid_create function is not generating version-1 UUIDs
+SELECT uuid_generate_v3(uuid_ns_dns(), 'www.widgets.com');
+ uuid_generate_v3
+--------------------------------------
+ 3d813cbb-47fb-32ba-91df-831e1593ac29
+(1 row)
+
+SELECT uuid_generate_v5(uuid_ns_dns(), 'www.widgets.com');
+ uuid_generate_v5
+--------------------------------------
+ 21f7f8de-8051-5b89-8680-0195ef798b6a
+(1 row)
+
+SELECT uuid_version_bits(uuid_generate_v4()),
+ uuid_reserved_bits(uuid_generate_v4());
+ uuid_version_bits | uuid_reserved_bits
+-------------------+--------------------
+ 01000000 | 10000000
+(1 row)
+
+SELECT uuid_generate_v4() <> uuid_generate_v4();
+ ?column?
+----------
+ t
+(1 row)
+
diff --git a/contrib/uuid-ossp/uuid-ossp.c b/contrib/uuid-ossp/uuid-ossp.c
index b868812358..ece46a31d8 100644
--- a/contrib/uuid-ossp/uuid-ossp.c
+++ b/contrib/uuid-ossp/uuid-ossp.c
@@ -282,6 +282,16 @@ uuid_generate_internal(int v, unsigned char *ns, const char *ptr, int len)
uuid_to_string(&uu, &str, &status);
if (status == uuid_s_ok)
{
+ /* At newer versions of BSD, uuid_create function
+ * could produce other UUID versions than version-1.
+ * Check version field(14) of generated UUID and
+ * error out if required.
+ */
+ if(str[14] != '1')
+ ereport(ERROR,
+ errmsg("uuid_create function is not "
+ "generating version-1 UUIDs"));
+
strlcpy(strbuf, str, 37);
/*
