From 9f02ae54f1643265d707a7677a93498f5c3ef74f Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Wed, 19 Jul 2023 16:56:08 +0300
Subject: [PATCH] iproto: factor out sql request processing to box_process_sql
We are planning to add access checks for EXECUTE and PREPARE requests.
(Currently, everyone, even guest, may execute these requests.)
Checking access in tx_process_sql(), which is defined in IPROTO code,
would violate encapsulation and look inconsistent with other request
handlers. Let's move the code that actually processes an SQL request
to the new function box_process_sql() taking sql_request and returning
the result in a port object.
To unify handling of all SQL requests in box_process_sql(), we add a new
format for port_sql - UNPREPARE. The format works only for dumping port
content to MsgPack buffer - it encodes an empty map then. This way, we
don't need to return the is_unprepare flag from box_process_sql().
Needed for #8803
NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
---
src/box/execute.c | 53 +++++++++++++++++++++++++
src/box/execute.h | 17 ++++++++
src/box/iproto.cc | 63 +-----------------------------
src/box/sql/port.c | 10 +++++
src/box/sql/port.h | 1 +
src/box/xrow.c | 2 +
src/box/xrow.h | 2 +
test/fuzz/xrow_decode_sql_fuzzer.c | 1 +
8 files changed, 88 insertions(+), 61 deletions(-)
diff --git a/src/box/execute.c b/src/box/execute.c
index 06298703aa..44d465602d 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -257,3 +257,56 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
port_destroy(port);
return -1;
}
+
+int
+box_process_sql(const struct sql_request *request, struct port *port)
+{
+ struct region *region = &fiber()->gc;
+ struct sql_bind *bind = NULL;
+ int bind_count = 0;
+ if (request->bind != NULL) {
+ bind_count = sql_bind_list_decode(request->bind, &bind);
+ if (bind_count < 0)
+ return -1;
+ }
+ /*
+ * There are four options:
+ * 1. Prepare SQL query (IPROTO_PREPARE + SQL string);
+ * 2. Unprepare SQL query (IPROTO_PREPARE + stmt id);
+ * 3. Execute SQL query (IPROTO_EXECUTE + SQL string);
+ * 4. Execute prepared query (IPROTO_EXECUTE + stmt id).
+ */
+ if (request->execute) {
+ if (request->sql_text != NULL) {
+ assert(request->stmt_id == NULL);
+ const char *sql = request->sql_text;
+ uint32_t len;
+ sql = mp_decode_str(&sql, &len);
+ return sql_prepare_and_execute(sql, len,
+ bind, bind_count,
+ port, region);
+ } else {
+ assert(request->stmt_id != NULL);
+ const char *data = request->stmt_id;
+ uint32_t stmt_id = mp_decode_uint(&data);
+ return sql_execute_prepared(stmt_id, bind, bind_count,
+ port, region);
+ }
+ } else {
+ if (request->sql_text != NULL) {
+ assert(request->stmt_id == NULL);
+ const char *sql = request->sql_text;
+ uint32_t len;
+ sql = mp_decode_str(&sql, &len);
+ return sql_prepare(sql, len, port);
+ } else {
+ assert(request->stmt_id != NULL);
+ const char *data = request->stmt_id;
+ uint32_t stmt_id = mp_decode_uint(&data);
+ if (sql_unprepare(stmt_id) != 0)
+ return -1;
+ port_sql_create(port, NULL, UNPREPARE, false);
+ return 0;
+ }
+ }
+}
diff --git a/src/box/execute.h b/src/box/execute.h
index 77ca975606..c5f64a26ef 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -51,6 +51,7 @@ extern const char *sql_info_key_strs[];
struct Vdbe;
struct region;
struct sql_bind;
+struct sql_request;
int
sql_unprepare(uint32_t stmt_id);
@@ -113,6 +114,22 @@ sql_stmt_busy(const struct Vdbe *stmt);
int
sql_prepare(const char *sql, int len, struct port *port);
+/**
+ * Process an SQL request received over IPROTO.
+ *
+ * @param request SQL request.
+ * @param port Port to store request response.
+ *
+ * @retval 0 Success.
+ * @retval -1 Error, see diag.
+ *
+ * NOTE: The port may refer to data allocated from the fiber region.
+ * The caller is responsible for truncating the region after using
+ * the port.
+ */
+int
+box_process_sql(const struct sql_request *request, struct port *port);
+
#if defined(__cplusplus)
} /* extern "C" { */
#endif
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 6af38911a2..f825df1a9f 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -2432,11 +2432,6 @@ tx_process_sql(struct cmsg *m)
struct iproto_msg *msg = tx_accept_msg(m);
struct obuf *out;
struct port port;
- struct sql_bind *bind = NULL;
- int bind_count = 0;
- const char *sql;
- uint32_t len;
- bool is_unprepare = false;
RegionGuard region_guard(&fiber()->gc);
if (tx_check_msg(msg) != 0)
@@ -2444,68 +2439,14 @@ tx_process_sql(struct cmsg *m)
assert(msg->header.type == IPROTO_EXECUTE ||
msg->header.type == IPROTO_PREPARE);
tx_inject_delay();
- if (msg->sql.bind != NULL) {
- bind_count = sql_bind_list_decode(msg->sql.bind, &bind);
- if (bind_count < 0)
- goto error;
- }
- /*
- * There are four options:
- * 1. Prepare SQL query (IPROTO_PREPARE + SQL string);
- * 2. Unprepare SQL query (IPROTO_PREPARE + stmt id);
- * 3. Execute SQL query (IPROTO_EXECUTE + SQL string);
- * 4. Execute prepared query (IPROTO_EXECUTE + stmt id).
- */
- if (msg->header.type == IPROTO_EXECUTE) {
- if (msg->sql.sql_text != NULL) {
- assert(msg->sql.stmt_id == NULL);
- sql = msg->sql.sql_text;
- sql = mp_decode_str(&sql, &len);
- if (sql_prepare_and_execute(sql, len, bind, bind_count,
- &port, &fiber()->gc) != 0)
- goto error;
- } else {
- assert(msg->sql.sql_text == NULL);
- assert(msg->sql.stmt_id != NULL);
- sql = msg->sql.stmt_id;
- uint32_t stmt_id = mp_decode_uint(&sql);
- if (sql_execute_prepared(stmt_id, bind, bind_count,
- &port, &fiber()->gc) != 0)
- goto error;
- }
- } else {
- /* IPROTO_PREPARE */
- if (msg->sql.sql_text != NULL) {
- assert(msg->sql.stmt_id == NULL);
- sql = msg->sql.sql_text;
- sql = mp_decode_str(&sql, &len);
- if (sql_prepare(sql, len, &port) != 0)
- goto error;
- } else {
- /* UNPREPARE */
- assert(msg->sql.sql_text == NULL);
- assert(msg->sql.stmt_id != NULL);
- sql = msg->sql.stmt_id;
- uint32_t stmt_id = mp_decode_uint(&sql);
- if (sql_unprepare(stmt_id) != 0)
- goto error;
- is_unprepare = true;
- }
- }
-
+ if (box_process_sql(&msg->sql, &port) != 0)
+ goto error;
/*
* Take an obuf only after execute(). Else the buffer can
* become out of date during yield.
*/
out = msg->connection->tx.p_obuf;
struct obuf_svp header_svp;
- if (is_unprepare) {
- header_svp = obuf_create_svp(out);
- iproto_reply_ok(out, msg->header.sync, schema_version);
- iproto_wpos_create(&msg->wpos, out);
- tx_end_msg(msg, &header_svp);
- return;
- }
iproto_prepare_header(out, &header_svp, IPROTO_HEADER_LEN);
if (port_dump_msgpack(&port, out) != 0) {
port_destroy(&port);
diff --git a/src/box/sql/port.c b/src/box/sql/port.c
index c274482165..3e2113e664 100644
--- a/src/box/sql/port.c
+++ b/src/box/sql/port.c
@@ -345,7 +345,17 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
*/
int keys = 3;
return sql_get_prepare_common_keys(stmt, out, keys);
+ }
+ case UNPREPARE: {
+ int size = mp_sizeof_map(0);
+ char *pos = obuf_alloc(out, size);
+ if (pos == NULL) {
+ diag_set(OutOfMemory, size, "obuf_alloc", "pos");
+ return -1;
}
+ mp_encode_map(pos, 0);
+ break;
+ }
default: {
unreachable();
}
diff --git a/src/box/sql/port.h b/src/box/sql/port.h
index e5f09a5ff0..ff1fdfafe2 100644
--- a/src/box/sql/port.h
+++ b/src/box/sql/port.h
@@ -21,6 +21,7 @@ enum sql_serialization_format {
DML_EXECUTE = 1,
DQL_PREPARE = 2,
DML_PREPARE = 3,
+ UNPREPARE = 4,
};
/** Methods of struct port_sql. */
diff --git a/src/box/xrow.c b/src/box/xrow.c
index de0403cc80..4194568c0c 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -753,6 +753,7 @@ iproto_reply_select_with_position(struct obuf *buf, struct obuf_svp *svp,
int
xrow_decode_sql(const struct xrow_header *row, struct sql_request *request)
{
+ assert(row->type == IPROTO_EXECUTE || row->type == IPROTO_PREPARE);
if (row->bodycnt == 0) {
diag_set(ClientError, ER_INVALID_MSGPACK, "missing request body");
return 1;
@@ -765,6 +766,7 @@ xrow_decode_sql(const struct xrow_header *row, struct sql_request *request)
}
uint32_t map_size = mp_decode_map(&data);
+ request->execute = row->type == IPROTO_EXECUTE;
request->sql_text = NULL;
request->bind = NULL;
request->stmt_id = NULL;
diff --git a/src/box/xrow.h b/src/box/xrow.h
index 9764dd5794..edd2d96e1c 100644
--- a/src/box/xrow.h
+++ b/src/box/xrow.h
@@ -831,6 +831,8 @@ iproto_reply_error(struct obuf *out, const struct error *e, uint64_t sync,
/** EXECUTE/PREPARE request. */
struct sql_request {
+ /** True for EXECUTE, false for PREPARE. */
+ bool execute;
/** SQL statement text. */
const char *sql_text;
/** MessagePack array of parameters. */
diff --git a/test/fuzz/xrow_decode_sql_fuzzer.c b/test/fuzz/xrow_decode_sql_fuzzer.c
index 4cdc1e3704..621b0aa12b 100644
--- a/test/fuzz/xrow_decode_sql_fuzzer.c
+++ b/test/fuzz/xrow_decode_sql_fuzzer.c
@@ -35,6 +35,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
body.iov_len = size;
struct xrow_header row = {0};
+ row.type = IPROTO_EXECUTE;
row.body[0] = body;
row.bodycnt = 1;
--
GitLab