[Tarantool-patches] [PATCH v2] iproto: add an empty body to the unprepare response

[Chris Sosnin]

Absence of the body in the unprepare response forces users to perform
additional checks to avoid errors. Adding an empty body fixes this problem.

Closes #4769
---
branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-4769-unprepare-response-body
issue: https://github.com/tarantool/tarantool/issues/4769

As Nikita suggested, I created box/iproto.test.lua, and basically
inserted wrappers for requests testing from box-py for future usage.

 src/box/iproto.cc        | 17 ++++----
 test/box/iproto.result   | 86 ++++++++++++++++++++++++++++++++++++++++
 test/box/iproto.test.lua | 54 +++++++++++++++++++++++++
 3 files changed, 150 insertions(+), 7 deletions(-)
 create mode 100644 test/box/iproto.result
 create mode 100644 test/box/iproto.test.lua

diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index f313af6ae..d5891391d 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1781,20 +1781,23 @@ tx_process_sql(struct cmsg *m)
 	 */
 	out = msg->connection->tx.p_obuf;
 	struct obuf_svp header_svp;
+	if (is_unprepare) {
+		if (iproto_reply_ok(out, msg->header.sync, schema_version) != 0)
+			goto error;
+		iproto_wpos_create(&msg->wpos, out);
+		return;
+	}
 	/* Prepare memory for the iproto header. */
 	if (iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN) != 0) {
 		port_destroy(&port);
 		goto error;
 	}
-	/* Nothing to dump in case of UNPREPARE request. */
-	if (!is_unprepare) {
-		if (port_dump_msgpack(&port, out) != 0) {
-			port_destroy(&port);
-			obuf_rollback_to_svp(out, &header_svp);
-			goto error;
-		}
+	if (port_dump_msgpack(&port, out) != 0) {
 		port_destroy(&port);
+		obuf_rollback_to_svp(out, &header_svp);
+		goto error;
 	}
+	port_destroy(&port);
 	iproto_reply_sql(out, &header_svp, msg->header.sync, schema_version);
 	iproto_wpos_create(&msg->wpos, out);
 	return;
diff --git a/test/box/iproto.result b/test/box/iproto.result
new file mode 100644
index 000000000..457f88493
--- /dev/null
+++ b/test/box/iproto.result
@@ -0,0 +1,86 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+
+net_box = require('net.box')
+msgpack = require('msgpack')
+urilib = require('uri')
+
+IPROTO_REQUEST_TYPE   = 0x00
+IPROTO_PREPARE        = 13
+IPROTO_SQL_TEXT       = 0x40
+IPROTO_STMT_ID        = 0x43
+
+function recieve_response(socket)
+    local size = socket:read(5)
+    assert(size ~= nil, 'Failed to read response')
+    size = msgpack.decode(size)
+    local response = socket:read(size)
+    local header, header_len = msgpack.decode(response)
+    local body = msgpack.decode(response:sub(header_len))
+    return {
+        ['header'] = header,
+        ['body'] = body
+    }
+end
+
+function test_request(socket, query_header, query_body)
+    local header = msgpack.encode(query_header)
+    local body = msgpack.encode(query_body)
+    local size = msgpack.encode(header:len() + body:len())
+    assert(socket:write(size .. header .. body) ~= nil,
+           'Failed to send request')
+    return recieve_response(socket)
+end
+
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | ...
+
+box.schema.user.grant('guest', 'read, write, execute', 'universe')
+ | ---
+ | ...
+uri = urilib.parse(box.cfg.listen)
+ | ---
+ | ...
+socket = net_box.establish_connection(uri.host, uri.service)
+ | ---
+ | ...
+
+--
+-- gh-4769: Unprepare response must have a body.
+--
+header = { [IPROTO_REQUEST_TYPE] = IPROTO_PREPARE }
+ | ---
+ | ...
+body = { [IPROTO_SQL_TEXT] = 'SELECT 1' }
+ | ---
+ | ...
+response = test_request(socket, header, body)
+ | ---
+ | ...
+
+body = { [IPROTO_STMT_ID] = response['body'][IPROTO_STMT_ID] }
+ | ---
+ | ...
+response = test_request(socket, header, body)
+ | ---
+ | ...
+response.body
+ | ---
+ | - {}
+ | ...
+
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+ | ---
+ | ...
+socket:close()
+ | ---
+ | - true
+ | ...
+
diff --git a/test/box/iproto.test.lua b/test/box/iproto.test.lua
new file mode 100644
index 000000000..5ade33339
--- /dev/null
+++ b/test/box/iproto.test.lua
@@ -0,0 +1,54 @@
+test_run = require('test_run').new()
+test_run:cmd("setopt delimiter ';'")
+
+net_box = require('net.box')
+msgpack = require('msgpack')
+urilib = require('uri')
+
+IPROTO_REQUEST_TYPE   = 0x00
+IPROTO_PREPARE        = 13
+IPROTO_SQL_TEXT       = 0x40
+IPROTO_STMT_ID        = 0x43
+
+function recieve_response(socket)
+    local size = socket:read(5)
+    assert(size ~= nil, 'Failed to read response')
+    size = msgpack.decode(size)
+    local response = socket:read(size)
+    local header, header_len = msgpack.decode(response)
+    local body = msgpack.decode(response:sub(header_len))
+    return {
+        ['header'] = header,
+        ['body'] = body
+    }
+end
+
+function test_request(socket, query_header, query_body)
+    local header = msgpack.encode(query_header)
+    local body = msgpack.encode(query_body)
+    local size = msgpack.encode(header:len() + body:len())
+    assert(socket:write(size .. header .. body) ~= nil,
+           'Failed to send request')
+    return recieve_response(socket)
+end
+
+test_run:cmd("setopt delimiter ''");
+
+box.schema.user.grant('guest', 'read, write, execute', 'universe')
+uri = urilib.parse(box.cfg.listen)
+socket = net_box.establish_connection(uri.host, uri.service)
+
+--
+-- gh-4769: Unprepare response must have a body.
+--
+header = { [IPROTO_REQUEST_TYPE] = IPROTO_PREPARE }
+body = { [IPROTO_SQL_TEXT] = 'SELECT 1' }
+response = test_request(socket, header, body)
+
+body = { [IPROTO_STMT_ID] = response['body'][IPROTO_STMT_ID] }
+response = test_request(socket, header, body)
+response.body
+
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+socket:close()
+
-- 
2.21.1 (Apple Git-122.3)