[tarantool-patches] [PATCH 1/1] box: raise an error on nil replicaset and instance uuid

[tarantool-patches] [PATCH 1/1] box: raise an error on nil replicaset and instance uuid

[Vladislav Shpilevoy]

Before the patch the nil UUID was ignored and a new random one
was generated. This was because internally box treats nil UUID
as its absense.

Now a user will see an explicit message that nil UUID is a
reserved value.

Closes #4282
---
Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4282-box-cfg-rs-uuid
Issue: https://github.com/tarantool/tarantool/issues/4282

 src/box/box.cc            | 25 +++++++++++++++++--------
 test/app-tap/cfg.test.lua | 12 +++++++++++-
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index ac10c21ad..5efc88d5f 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -480,22 +480,31 @@ box_check_replication_sync_timeout(void)
 	return timeout;
 }
 
+static inline void
+box_check_uuid(struct tt_uuid *uuid, const char* name)
+{
+	*uuid = uuid_nil;
+	const char *uuid_str = cfg_gets(name);
+	if (uuid_str == NULL)
+		return;
+	if (tt_uuid_from_string(uuid_str, uuid) != 0)
+		tnt_raise(ClientError, ER_CFG, name, uuid_str);
+	if (tt_uuid_is_nil(uuid)) {
+		tnt_raise(ClientError, ER_CFG, name,
+			  tt_sprintf("nil UUID is a reserved value"));
+	}
+}
+
 static void
 box_check_instance_uuid(struct tt_uuid *uuid)
 {
-	*uuid = uuid_nil;
-	const char *uuid_str = cfg_gets("instance_uuid");
-	if (uuid_str != NULL && tt_uuid_from_string(uuid_str, uuid) != 0)
-		tnt_raise(ClientError, ER_CFG, "instance_uuid", uuid_str);
+	box_check_uuid(uuid, "instance_uuid");
 }
 
 static void
 box_check_replicaset_uuid(struct tt_uuid *uuid)
 {
-	*uuid = uuid_nil;
-	const char *uuid_str = cfg_gets("replicaset_uuid");
-	if (uuid_str != NULL && tt_uuid_from_string(uuid_str, uuid) != 0)
-		tnt_raise(ClientError, ER_CFG, "replicaset_uuid", uuid_str);
+	box_check_uuid(uuid, "replicaset_uuid");
 }
 
 static enum wal_mode
diff --git a/test/app-tap/cfg.test.lua b/test/app-tap/cfg.test.lua
index 7d8e9a05e..67af00c30 100755
--- a/test/app-tap/cfg.test.lua
+++ b/test/app-tap/cfg.test.lua
@@ -3,7 +3,17 @@ local fiber = require('fiber')
 local tap = require('tap')
 local test = tap.test("cfg")
 
-test:plan(8)
+test:plan(10)
+
+--
+-- gh-4282: box.cfg should not allow nor just ignore nil UUID.
+-- It is a special reserved value.
+--
+local nil_uuid = '00000000-0000-0000-0000-000000000000'
+local ok = pcall(box.cfg, {instance_uuid = nil_uuid})
+test:ok(not ok, 'nil instance UUID is not allowed')
+ok, err = pcall(box.cfg, {replicaset_uuid = nil_uuid})
+test:ok(not ok, 'nil replicaset UUID is not allowed')
 
 test:is(type(box.ctl), "table", "box.ctl is available before box.cfg")
 test:is(type(box.ctl.wait_ro), "function", "box.ctl.wait_ro is available")
-- 
2.20.1 (Apple Git-117)

[tarantool-patches] Re: [PATCH 1/1] box: raise an error on nil replicaset and instance uuid

[Vladislav Shpilevoy]

On 07/09/2019 15:26, Vladislav Shpilevoy wrote:
> Before the patch the nil UUID was ignored and a new random one
> was generated. This was because internally box treats nil UUID
> as its absense.

'absense' -> 'absence'.

> 
> Now a user will see an explicit message that nil UUID is a
> reserved value.
> 
> Closes #4282
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4282-box-cfg-rs-uuid
> Issue: https://github.com/tarantool/tarantool/issues/4282
> 
>  src/box/box.cc            | 25 +++++++++++++++++--------
>  test/app-tap/cfg.test.lua | 12 +++++++++++-
>  2 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/src/box/box.cc b/src/box/box.cc
> index ac10c21ad..5efc88d5f 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -480,22 +480,31 @@ box_check_replication_sync_timeout(void)
>  	return timeout;
>  }
>  
> +static inline void
> +box_check_uuid(struct tt_uuid *uuid, const char* name)

Sorry, '*' should be right before 'name'.
(Messed with Ubisoft code style)

[tarantool-patches] Re: [PATCH 1/1] box: raise an error on nil replicaset and instance uuid

[Vladislav Shpilevoy]

Changed error message as asked by Kostja:

diff --git a/src/box/box.cc b/src/box/box.cc
index 921bc5c42..015994da1 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -491,7 +491,7 @@ box_check_uuid(struct tt_uuid *uuid, const char *name)
 		tnt_raise(ClientError, ER_CFG, name, uuid_str);
 	if (tt_uuid_is_nil(uuid)) {
 		tnt_raise(ClientError, ER_CFG, name,
-			  tt_sprintf("nil UUID is a reserved value"));
+			  tt_sprintf("nil UUID is reserved"));
 	}
 }
 

Re: [Tarantool-patches] [PATCH 1/1] box: raise an error on nil replicaset and instance uuid

[Alexander Turenko]

LGTM.

CCed Kirill.

On Sat, Sep 07, 2019 at 03:26:02PM +0200, Vladislav Shpilevoy wrote:
> Before the patch the nil UUID was ignored and a new random one
> was generated. This was because internally box treats nil UUID
> as its absense.
> 
> Now a user will see an explicit message that nil UUID is a
> reserved value.
> 
> Closes #4282
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4282-box-cfg-rs-uuid
> Issue: https://github.com/tarantool/tarantool/issues/4282
> 
>  src/box/box.cc            | 25 +++++++++++++++++--------
>  test/app-tap/cfg.test.lua | 12 +++++++++++-
>  2 files changed, 28 insertions(+), 9 deletions(-)