[Tarantool-patches] [PATCH] sql: forbid data changes in read-only mode

[Sergey Ostanevich]

Hi!

Thanks for review! Updated, force-pushed, new patch attached.

Sergos

> On 17 Nov 2020, at 00:09, Nikita Pettik <korablev at tarantool.org> wrote:
> 
> On 16 Nov 23:36, Sergey Ostanevich wrote:
>> diff --git a/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
>> new file mode 100755
>> index 000000000..6d95adc2e
>> --- /dev/null
>> +++ b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
>> @@ -0,0 +1,105 @@
>> +#!/usr/bin/env tarantool
>> +
>> +test = require("sqltester")
>> +test:plan(9)
>> +
>> +local expected_err = "Can't modify data because this instance is in read-only mode."
>> +
>> +test:execsql([[
>> +    CREATE TABLE TEST (A INT, B INT, PRIMARY KEY (A));
>> +    INSERT INTO TEST (A, B) VALUES (3, 3);
>> +]])
>> +
>> +box.cfg{read_only = true}
>> +
>> +test:do_catchsql_test(
>> +    "sql-errors-1.1",
>> +    [[
>> +        INSERT INTO TEST (A, B) VALUES (1, 1);
>> +    ]], {
>> +        -- <sql-errors-1.1>
> 
> These comments are artifacts from porting SQLite test suite with
> auto-tools, they are not really needed in new tests :)
> 
=============

From 4804ac485b38ddd280735970f6b9422bad1b30b6 Mon Sep 17 00:00:00 2001
From: Sergey Ostanevich <sergos at tarantool.org>
Date: Tue, 10 Nov 2020 13:52:10 +0300
Subject: [PATCH] sql: forbid data changes in read-only mode

A number of places in sql.c uses direct access to box_process_rw() that
does not check read-only setting. Fixed by use of an intended interface
of box_process1().

Closes #5231
---

Branch: https://github.com/tarantool/tarantool/compare/sergos/gh-5231-box-execute-writes-in-ro
Issue: https://github.com/tarantool/tarantool/issues/5231

@ChangeLog
- Data changes are blocked for SQL if node is read-only (gh-5231).

 src/box/sql.c                                 |  8 +-
 .../gh-5231-box-execute-writes-in-ro.test.lua | 87 +++++++++++++++++++
 2 files changed, 91 insertions(+), 4 deletions(-)
 create mode 100755 test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua

diff --git a/src/box/sql.c b/src/box/sql.c
index a551bffc3..c55d0f4e5 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -453,7 +453,7 @@ insertOrReplace(struct space *space, const char *tuple, const char *tuple_end,
 	request.space_id = space->def->id;
 	request.type = type;
 	mp_tuple_assert(request.tuple, request.tuple_end);
-	return box_process_rw(&request, space, NULL);
+	return box_process1(&request, NULL);
 }
 
 int tarantoolsqlInsert(struct space *space, const char *tuple,
@@ -531,7 +531,7 @@ sql_delete_by_key(struct space *space, uint32_t iid, char *key,
 	request.space_id = space->def->id;
 	request.index_id = iid;
 	assert(space_index(space, iid)->def->opts.is_unique);
-	return box_process_rw(&request, space, &unused);
+	return box_process1(&request, &unused);
 }
 
 /*
@@ -597,7 +597,7 @@ int tarantoolsqlClearTable(struct space *space, uint32_t *tuple_count)
 		request.key = tuple_extract_key(tuple, pk->def->key_def,
 						MULTIKEY_NONE, &key_size);
 		request.key_end = request.key + key_size;
-		rc = box_process_rw(&request, space, &unused);
+		rc = box_process1(&request, &unused);
 		if (rc != 0) {
 			iterator_delete(iter);
 			return -1;
@@ -834,7 +834,7 @@ tarantoolsqlIncrementMaxid(uint64_t *space_max_id)
 	request.key_end = key + sizeof(key);
 	request.type = IPROTO_UPDATE;
 	request.space_id = space_schema->def->id;
-	if (box_process_rw(&request, space_schema, &res) != 0 || res == NULL ||
+	if (box_process1(&request, &res) != 0 || res == NULL ||
 	    tuple_field_u64(res, 1, space_max_id) != 0)
 		return -1;
 	return 0;
diff --git a/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
new file mode 100755
index 000000000..05c23c2f8
--- /dev/null
+++ b/test/sql-tap/gh-5231-box-execute-writes-in-ro.test.lua
@@ -0,0 +1,87 @@
+#!/usr/bin/env tarantool
+
+test = require("sqltester")
+test:plan(9)
+
+local expected_err = "Can't modify data because this instance is in read-only mode."
+
+test:execsql([[
+    CREATE TABLE TEST (A INT, B INT, PRIMARY KEY (A));
+    INSERT INTO TEST (A, B) VALUES (3, 3);
+]])
+
+box.cfg{read_only = true}
+
+test:do_catchsql_test(
+    "gh-5231-1",
+    [[
+        INSERT INTO TEST (A, B) VALUES (1, 1);
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-2",
+    [[
+        DELETE FROM TEST;
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-3",
+    [[
+        REPLACE INTO TEST VALUES (1, 2);
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-4",
+    [[
+        UPDATE TEST SET B=4 WHERE A=3;
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-5",
+    [[
+        TRUNCATE TABLE TEST;
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-6",
+    [[
+        CREATE TABLE TEST2 (A INT, PRIMARY KEY (A));
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-7",
+    [[
+        ALTER TABLE TEST ADD CONSTRAINT UK_CON UNIQUE (B);
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-8",
+    [[
+        ALTER TABLE TEST RENAME TO TEST2;
+    ]], {
+        1, expected_err
+    })
+
+test:do_catchsql_test(
+    "gh-5231-9",
+    [[
+        DROP TABLE TEST;
+    ]], {
+        1, expected_err
+    })
+
+test:finish_test()
-- 
2.24.3 (Apple Git-128)