[Tarantool-patches] [PATCH v1 0/2] sql: ignore \0 in string passed to user function

[Tarantool-patches] [PATCH v1 0/2] sql: ignore \0 in string passed to user function

[Mergen Imeev via Tarantool-patches]

Currently, string passed to user-defined function from SQL can be cropped in
case it contains '\0'. This patch-set fixes this behaviour.

https://github.com/tarantool/tarantool/issues/5938
https://github.com/tarantool/tarantool/tree/imeevma/gh-5938-wrong-string-length

Mergen Imeev (2):
  sql: ignore \0 in string passed to C-function
  sql: ignore \0 in string passed to Lua-function

 src/box/sql/func.c                            |  6 ++-
 test/CMakeLists.txt                           |  1 +
 test/sql-tap/CMakeLists.txt                   |  2 +
 test/sql-tap/gh-5938-wrong-string-length.c    | 42 +++++++++++++++++
 .../gh-5938-wrong-string-length.test.lua      | 45 +++++++++++++++++++
 5 files changed, 94 insertions(+), 2 deletions(-)
 create mode 100644 test/sql-tap/CMakeLists.txt
 create mode 100644 test/sql-tap/gh-5938-wrong-string-length.c
 create mode 100755 test/sql-tap/gh-5938-wrong-string-length.test.lua

-- 
2.25.1

[Tarantool-patches] [PATCH v1 1/2] sql: ignore \0 in string passed to C-function

[Mergen Imeev via Tarantool-patches]

Prior to this patch string passed to user-defined C-function from SQL
was cropped in case it contains '\0'. At the same time, it wasn't
cropped if it is passed to the function from BOX. Now it isn't cropped
when passed from SQL.

Part of #5938
---
 src/box/sql/func.c                            |  3 +-
 test/CMakeLists.txt                           |  1 +
 test/sql-tap/CMakeLists.txt                   |  2 +
 test/sql-tap/gh-5938-wrong-string-length.c    | 42 +++++++++++++++++++
 .../gh-5938-wrong-string-length.test.lua      | 28 +++++++++++++
 5 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 test/sql-tap/CMakeLists.txt
 create mode 100644 test/sql-tap/gh-5938-wrong-string-length.c
 create mode 100755 test/sql-tap/gh-5938-wrong-string-length.test.lua

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index f15d27051..c3c14bd22 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -175,7 +175,8 @@ port_vdbemem_get_msgpack(struct port *base, uint32_t *size)
 		}
 		case MP_STR: {
 			const char *str = (const char *) sql_value_text(param);
-			mpstream_encode_str(&stream, str);
+			mpstream_encode_strn(&stream, str,
+					     sql_value_bytes(param));
 			break;
 		}
 		case MP_BIN:
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 7fe078835..7276996d9 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -62,6 +62,7 @@ add_subdirectory(app)
 add_subdirectory(app-tap)
 add_subdirectory(box)
 add_subdirectory(box-tap)
+add_subdirectory(sql-tap)
 if(ENABLE_FUZZER)
     add_subdirectory(fuzz)
 endif()
diff --git a/test/sql-tap/CMakeLists.txt b/test/sql-tap/CMakeLists.txt
new file mode 100644
index 000000000..6e2eae2ff
--- /dev/null
+++ b/test/sql-tap/CMakeLists.txt
@@ -0,0 +1,2 @@
+include_directories(${MSGPUCK_INCLUDE_DIRS})
+build_module(gh-5938-wrong-string-length gh-5938-wrong-string-length.c)
diff --git a/test/sql-tap/gh-5938-wrong-string-length.c b/test/sql-tap/gh-5938-wrong-string-length.c
new file mode 100644
index 000000000..96823f049
--- /dev/null
+++ b/test/sql-tap/gh-5938-wrong-string-length.c
@@ -0,0 +1,42 @@
+#include <msgpuck.h>
+#include "module.h"
+
+enum
+{
+	BUF_SIZE = 512,
+};
+
+int
+ret_str(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	uint32_t arg_count = mp_decode_array(&args);
+	if (arg_count != 1) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+			"invalid argument count");
+	}
+	if (mp_typeof(*args) != MP_STR) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+			"argument should be string");
+	}
+	const char* str;
+	uint32_t str_n;
+	str = mp_decode_str(&args, &str_n);
+
+	uint32_t size = mp_sizeof_array(1) + mp_sizeof_str(str_n);
+	if (size > BUF_SIZE) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
+			"string is too long");
+	}
+
+	char tuple_buf[BUF_SIZE];
+	char *d = tuple_buf;
+	d = mp_encode_array(d, 1);
+	d = mp_encode_str(d, str, str_n);
+	assert(d <= tuple_buf + size);
+
+	box_tuple_format_t *fmt = box_tuple_format_default();
+	box_tuple_t *tuple = box_tuple_new(fmt, tuple_buf, d);
+	if (tuple == NULL)
+		return -1;
+	return box_return_tuple(ctx, tuple);
+}
diff --git a/test/sql-tap/gh-5938-wrong-string-length.test.lua b/test/sql-tap/gh-5938-wrong-string-length.test.lua
new file mode 100755
index 000000000..943389e34
--- /dev/null
+++ b/test/sql-tap/gh-5938-wrong-string-length.test.lua
@@ -0,0 +1,28 @@
+#!/usr/bin/env tarantool
+local build_path = os.getenv("BUILDDIR")
+package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath
+
+local test = require("sqltester")
+test:plan(1)
+
+box.schema.func.create("gh-5938-wrong-string-length.ret_str", {
+    language = "C",
+    param_list = { "string" },
+    returns = "string",
+    exports = { "LUA", "SQL" },
+    is_deterministic = true
+})
+
+test:execsql([[CREATE TABLE t (i INT PRIMARY KEY, s STRING);]])
+box.space.T:insert({1, 'This is a complete string'})
+box.space.T:insert({2, 'This is a cropped\0 string'})
+
+test:do_execsql_test(
+    "gh-5938-1",
+    [[
+        SELECT "gh-5938-wrong-string-length.ret_str"(s) from t;
+    ]], {
+        "This is a complete string","This is a cropped\0 string"
+    })
+
+test:finish_test()
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v1 1/2] sql: ignore \0 in string passed to C-function

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the patch!

See 4 comments below.

On 30.03.2021 13:21, Mergen Imeev via Tarantool-patches wrote:
> Prior to this patch string passed to user-defined C-function from SQL
> was cropped in case it contains '\0'. At the same time, it wasn't
> cropped if it is passed to the function from BOX. Now it isn't cropped
> when passed from SQL.
> 
> Part of #5938
> ---
> diff --git a/test/sql-tap/gh-5938-wrong-string-length.c b/test/sql-tap/gh-5938-wrong-string-length.c
> new file mode 100644
> index 000000000..96823f049
> --- /dev/null
> +++ b/test/sql-tap/gh-5938-wrong-string-length.c
> @@ -0,0 +1,42 @@
> +#include <msgpuck.h>

1. We use "" for non-system headers, not <>.

> +#include "module.h"
> +
> +enum
> +{

2. Enums have { on the same line as 'enum'.

> +	BUF_SIZE = 512,
> +};
> +
> +int
> +ret_str(box_function_ctx_t *ctx, const char *args, const char *args_end)
> +{
> +	uint32_t arg_count = mp_decode_array(&args);
> +	if (arg_count != 1) {
> +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> +			"invalid argument count");

3. You don't need "%s", you can pass the error message right away.

4. The expression is misaligned. The same for the other box_error_set().

> +	}
> +	if (mp_typeof(*args) != MP_STR) {
> +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> +			"argument should be string");
> +	}
> +	const char* str;
> +	uint32_t str_n;
> +	str = mp_decode_str(&args, &str_n);
> +
> +	uint32_t size = mp_sizeof_array(1) + mp_sizeof_str(str_n);
> +	if (size > BUF_SIZE) {
> +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> +			"string is too long");
> +	}

Re: [Tarantool-patches] [PATCH v1 1/2] sql: ignore \0 in string passed to C-function

[Mergen Imeev via Tarantool-patches]

Hi! Thank you for the review! My answers, diff and new version below.

On Wed, Mar 31, 2021 at 10:25:21PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> See 4 comments below.
> 
> On 30.03.2021 13:21, Mergen Imeev via Tarantool-patches wrote:
> > Prior to this patch string passed to user-defined C-function from SQL
> > was cropped in case it contains '\0'. At the same time, it wasn't
> > cropped if it is passed to the function from BOX. Now it isn't cropped
> > when passed from SQL.
> > 
> > Part of #5938
> > ---
> > diff --git a/test/sql-tap/gh-5938-wrong-string-length.c b/test/sql-tap/gh-5938-wrong-string-length.c
> > new file mode 100644
> > index 000000000..96823f049
> > --- /dev/null
> > +++ b/test/sql-tap/gh-5938-wrong-string-length.c
> > @@ -0,0 +1,42 @@
> > +#include <msgpuck.h>
> 
> 1. We use "" for non-system headers, not <>.
Fixed.

> 
> > +#include "module.h"
> > +
> > +enum
> > +{
> 
> 2. Enums have { on the same line as 'enum'.
Fixed.

> 
> > +	BUF_SIZE = 512,
> > +};
> > +
> > +int
> > +ret_str(box_function_ctx_t *ctx, const char *args, const char *args_end)
> > +{
> > +	uint32_t arg_count = mp_decode_array(&args);
> > +	if (arg_count != 1) {
> > +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> > +			"invalid argument count");
> 
> 3. You don't need "%s", you can pass the error message right away.
Thanks, fixed.

> 
> 4. The expression is misaligned. The same for the other box_error_set().
Fixed.

> 
> > +	}
> > +	if (mp_typeof(*args) != MP_STR) {
> > +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> > +			"argument should be string");
> > +	}
> > +	const char* str;
> > +	uint32_t str_n;
> > +	str = mp_decode_str(&args, &str_n);
> > +
> > +	uint32_t size = mp_sizeof_array(1) + mp_sizeof_str(str_n);
> > +	if (size > BUF_SIZE) {
> > +		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
> > +			"string is too long");
> > +	}


Diff:


diff --git a/test/sql-tap/gh-5938-wrong-string-length.c b/test/sql-tap/gh-5938-wrong-string-length.c
index 96823f049..e53163fd2 100644
--- a/test/sql-tap/gh-5938-wrong-string-length.c
+++ b/test/sql-tap/gh-5938-wrong-string-length.c
@@ -1,8 +1,7 @@
-#include <msgpuck.h>
+#include "msgpuck.h"
 #include "module.h"
 
-enum
-{
+enum {
 	BUF_SIZE = 512,
 };
 
@@ -11,12 +10,12 @@ ret_str(box_function_ctx_t *ctx, const char *args, const char *args_end)
 {
 	uint32_t arg_count = mp_decode_array(&args);
 	if (arg_count != 1) {
-		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
-			"invalid argument count");
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C,
+				     "invalid argument count");
 	}
 	if (mp_typeof(*args) != MP_STR) {
-		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
-			"argument should be string");
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C,
+				     "argument should be string");
 	}
 	const char* str;
 	uint32_t str_n;
@@ -24,8 +23,8 @@ ret_str(box_function_ctx_t *ctx, const char *args, const char *args_end)
 
 	uint32_t size = mp_sizeof_array(1) + mp_sizeof_str(str_n);
 	if (size > BUF_SIZE) {
-		return box_error_set(__FILE__, __LINE__, ER_PROC_C, "%s",
-			"string is too long");
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C,
+				     "string is too long");
 	}
 
 	char tuple_buf[BUF_SIZE];



New patch:


commit dc1daab44f8960ff111eb3127252e51f1ed5c403
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Tue Mar 30 07:08:13 2021 +0300

    sql: ignore \0 in string passed to C-function
    
    Prior to this patch string passed to user-defined C-function from SQL
    was cropped in case it contains '\0'. At the same time, it wasn't
    cropped if it is passed to the function from BOX. Now it isn't cropped
    when passed from SQL.
    
    Part of #5938

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index f15d27051..c3c14bd22 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -175,7 +175,8 @@ port_vdbemem_get_msgpack(struct port *base, uint32_t *size)
 		}
 		case MP_STR: {
 			const char *str = (const char *) sql_value_text(param);
-			mpstream_encode_str(&stream, str);
+			mpstream_encode_strn(&stream, str,
+					     sql_value_bytes(param));
 			break;
 		}
 		case MP_BIN:
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 7fe078835..7276996d9 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -62,6 +62,7 @@ add_subdirectory(app)
 add_subdirectory(app-tap)
 add_subdirectory(box)
 add_subdirectory(box-tap)
+add_subdirectory(sql-tap)
 if(ENABLE_FUZZER)
     add_subdirectory(fuzz)
 endif()
diff --git a/test/sql-tap/CMakeLists.txt b/test/sql-tap/CMakeLists.txt
new file mode 100644
index 000000000..6e2eae2ff
--- /dev/null
+++ b/test/sql-tap/CMakeLists.txt
@@ -0,0 +1,2 @@
+include_directories(${MSGPUCK_INCLUDE_DIRS})
+build_module(gh-5938-wrong-string-length gh-5938-wrong-string-length.c)
diff --git a/test/sql-tap/gh-5938-wrong-string-length.c b/test/sql-tap/gh-5938-wrong-string-length.c
new file mode 100644
index 000000000..e53163fd2
--- /dev/null
+++ b/test/sql-tap/gh-5938-wrong-string-length.c
@@ -0,0 +1,41 @@
+#include "msgpuck.h"
+#include "module.h"
+
+enum {
+	BUF_SIZE = 512,
+};
+
+int
+ret_str(box_function_ctx_t *ctx, const char *args, const char *args_end)
+{
+	uint32_t arg_count = mp_decode_array(&args);
+	if (arg_count != 1) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C,
+				     "invalid argument count");
+	}
+	if (mp_typeof(*args) != MP_STR) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C,
+				     "argument should be string");
+	}
+	const char* str;
+	uint32_t str_n;
+	str = mp_decode_str(&args, &str_n);
+
+	uint32_t size = mp_sizeof_array(1) + mp_sizeof_str(str_n);
+	if (size > BUF_SIZE) {
+		return box_error_set(__FILE__, __LINE__, ER_PROC_C,
+				     "string is too long");
+	}
+
+	char tuple_buf[BUF_SIZE];
+	char *d = tuple_buf;
+	d = mp_encode_array(d, 1);
+	d = mp_encode_str(d, str, str_n);
+	assert(d <= tuple_buf + size);
+
+	box_tuple_format_t *fmt = box_tuple_format_default();
+	box_tuple_t *tuple = box_tuple_new(fmt, tuple_buf, d);
+	if (tuple == NULL)
+		return -1;
+	return box_return_tuple(ctx, tuple);
+}
diff --git a/test/sql-tap/gh-5938-wrong-string-length.test.lua b/test/sql-tap/gh-5938-wrong-string-length.test.lua
new file mode 100755
index 000000000..943389e34
--- /dev/null
+++ b/test/sql-tap/gh-5938-wrong-string-length.test.lua
@@ -0,0 +1,28 @@
+#!/usr/bin/env tarantool
+local build_path = os.getenv("BUILDDIR")
+package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath
+
+local test = require("sqltester")
+test:plan(1)
+
+box.schema.func.create("gh-5938-wrong-string-length.ret_str", {
+    language = "C",
+    param_list = { "string" },
+    returns = "string",
+    exports = { "LUA", "SQL" },
+    is_deterministic = true
+})
+
+test:execsql([[CREATE TABLE t (i INT PRIMARY KEY, s STRING);]])
+box.space.T:insert({1, 'This is a complete string'})
+box.space.T:insert({2, 'This is a cropped\0 string'})
+
+test:do_execsql_test(
+    "gh-5938-1",
+    [[
+        SELECT "gh-5938-wrong-string-length.ret_str"(s) from t;
+    ]], {
+        "This is a complete string","This is a cropped\0 string"
+    })
+
+test:finish_test()

[Tarantool-patches] [PATCH v1 2/2] sql: ignore \0 in string passed to Lua-function

[Mergen Imeev via Tarantool-patches]

Prior to this patch string passed to user-defined Lua-function from SQL
was cropped in case it contains '\0'. At the same time, it wasn't
cropped if it is passed to the function from BOX. After this patch the
string won't be cropped when passed from SQL if it contain '\0'.

Closes #5938
---
 src/box/sql/func.c                            |  3 ++-
 .../gh-5938-wrong-string-length.test.lua      | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index c3c14bd22..7d5a55d3f 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -120,7 +120,8 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
 			lua_pushnumber(L, sql_value_double(param));
 			break;
 		case MP_STR:
-			lua_pushstring(L, (const char *) sql_value_text(param));
+			lua_pushlstring(L, (const char *) sql_value_text(param),
+					(size_t) sql_value_bytes(param));
 			break;
 		case MP_BIN:
 		case MP_ARRAY:
diff --git a/test/sql-tap/gh-5938-wrong-string-length.test.lua b/test/sql-tap/gh-5938-wrong-string-length.test.lua
index 943389e34..415fc7729 100755
--- a/test/sql-tap/gh-5938-wrong-string-length.test.lua
+++ b/test/sql-tap/gh-5938-wrong-string-length.test.lua
@@ -3,7 +3,7 @@ local build_path = os.getenv("BUILDDIR")
 package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath
 
 local test = require("sqltester")
-test:plan(1)
+test:plan(2)
 
 box.schema.func.create("gh-5938-wrong-string-length.ret_str", {
     language = "C",
@@ -25,4 +25,21 @@ test:do_execsql_test(
         "This is a complete string","This is a cropped\0 string"
     })
 
+box.schema.func.create("ret_str", {
+    language = "Lua",
+    body = [[function(str) return str end]],
+    param_list = { "string" },
+    returns = "string",
+    exports = { "LUA", "SQL" },
+    is_deterministic = true
+})
+
+test:do_execsql_test(
+    "gh-5938-2",
+    [[
+        SELECT "ret_str"(s) from t;
+    ]], {
+        "This is a complete string","This is a cropped\0 string"
+    })
+
 test:finish_test()
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v1 2/2] sql: ignore \0 in string passed to Lua-function

[Vladislav Shpilevoy via Tarantool-patches]

Good job on the patch!

See 2 comments below.

1. Please, add a changelog file.

On 30.03.2021 13:21, Mergen Imeev via Tarantool-patches wrote:
> Prior to this patch string passed to user-defined Lua-function from SQL
> was cropped in case it contains '\0'. At the same time, it wasn't
> cropped if it is passed to the function from BOX. After this patch the
> string won't be cropped when passed from SQL if it contain '\0'.
> 
> Closes #5938
> ---
>  src/box/sql/func.c                            |  3 ++-
>  .../gh-5938-wrong-string-length.test.lua      | 19 ++++++++++++++++++-
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index c3c14bd22..7d5a55d3f 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -120,7 +120,8 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
>  			lua_pushnumber(L, sql_value_double(param));
>  			break;
>  		case MP_STR:
> -			lua_pushstring(L, (const char *) sql_value_text(param));
> +			lua_pushlstring(L, (const char *) sql_value_text(param),
> +					(size_t) sql_value_bytes(param));

2. Unary operators should not have a whitespace after them.

>  			break;
>  		case MP_BIN:
>  		case MP_ARRAY:

Re: [Tarantool-patches] [PATCH v1 2/2] sql: ignore \0 in string passed to Lua-function

[Mergen Imeev via Tarantool-patches]

On Wed, Mar 31, 2021 at 10:25:23PM +0200, Vladislav Shpilevoy wrote:
> Good job on the patch!
Thank you! My answers and new patch below.

> 
> See 2 comments below.
> 
> 1. Please, add a changelog file.
Added.

> 
> On 30.03.2021 13:21, Mergen Imeev via Tarantool-patches wrote:
> > Prior to this patch string passed to user-defined Lua-function from SQL
> > was cropped in case it contains '\0'. At the same time, it wasn't
> > cropped if it is passed to the function from BOX. After this patch the
> > string won't be cropped when passed from SQL if it contain '\0'.
> > 
> > Closes #5938
> > ---
> >  src/box/sql/func.c                            |  3 ++-
> >  .../gh-5938-wrong-string-length.test.lua      | 19 ++++++++++++++++++-
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> > index c3c14bd22..7d5a55d3f 100644
> > --- a/src/box/sql/func.c
> > +++ b/src/box/sql/func.c
> > @@ -120,7 +120,8 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
> >  			lua_pushnumber(L, sql_value_double(param));
> >  			break;
> >  		case MP_STR:
> > -			lua_pushstring(L, (const char *) sql_value_text(param));
> > +			lua_pushlstring(L, (const char *) sql_value_text(param),
> > +					(size_t) sql_value_bytes(param));
> 
> 2. Unary operators should not have a whitespace after them.
Fixed.

> 
> >  			break;
> >  		case MP_BIN:
> >  		case MP_ARRAY:


New patch:


commit 337a6cddd1d9e70c77e3ba48600b5d5ec9477dd7
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Tue Mar 30 09:41:34 2021 +0300

    sql: ignore \0 in string passed to Lua-function
    
    Prior to this patch string passed to user-defined Lua-function from SQL
    was cropped in case it contains '\0'. At the same time, it wasn't
    cropped if it is passed to the function from BOX. After this patch the
    string won't be cropped when passed from SQL if it contain '\0'.
    
    Closes #5938

diff --git "a/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md" "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
new file mode 100644
index 000000000..a0aa8a425
--- /dev/null
+++ "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
@@ -0,0 +1,4 @@
+## bugfix/sql
+
+* Fixed cropping of a string if it contains '\0' when passing a string
+  to user-defined Lua or C functions.
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index c3c14bd22..9b6179f3a 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -120,7 +120,8 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
 			lua_pushnumber(L, sql_value_double(param));
 			break;
 		case MP_STR:
-			lua_pushstring(L, (const char *) sql_value_text(param));
+			lua_pushlstring(L, (const char *)sql_value_text(param),
+					(size_t)sql_value_bytes(param));
 			break;
 		case MP_BIN:
 		case MP_ARRAY:
diff --git a/test/sql-tap/gh-5938-wrong-string-length.test.lua b/test/sql-tap/gh-5938-wrong-string-length.test.lua
index 943389e34..415fc7729 100755
--- a/test/sql-tap/gh-5938-wrong-string-length.test.lua
+++ b/test/sql-tap/gh-5938-wrong-string-length.test.lua
@@ -3,7 +3,7 @@ local build_path = os.getenv("BUILDDIR")
 package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath
 
 local test = require("sqltester")
-test:plan(1)
+test:plan(2)
 
 box.schema.func.create("gh-5938-wrong-string-length.ret_str", {
     language = "C",
@@ -25,4 +25,21 @@ test:do_execsql_test(
         "This is a complete string","This is a cropped\0 string"
     })
 
+box.schema.func.create("ret_str", {
+    language = "Lua",
+    body = [[function(str) return str end]],
+    param_list = { "string" },
+    returns = "string",
+    exports = { "LUA", "SQL" },
+    is_deterministic = true
+})
+
+test:do_execsql_test(
+    "gh-5938-2",
+    [[
+        SELECT "ret_str"(s) from t;
+    ]], {
+        "This is a complete string","This is a cropped\0 string"
+    })
+
 test:finish_test()

Re: [Tarantool-patches] [PATCH v1 2/2] sql: ignore \0 in string passed to Lua-function

[Mergen Imeev via Tarantool-patches]

Sorry, I found that '\0' in changelog file name was a bad idea. Quite a few
CI runs failed. I renamed this file and force-pushed this patch. Diff below.

On Thu, Apr 01, 2021 at 11:41:02AM +0300, Mergen Imeev via Tarantool-patches wrote:
> On Wed, Mar 31, 2021 at 10:25:23PM +0200, Vladislav Shpilevoy wrote:
> > Good job on the patch!
> Thank you! My answers and new patch below.
> 
> > 
> > See 2 comments below.
> > 
> > 1. Please, add a changelog file.
> Added.
> 
> > 
> > On 30.03.2021 13:21, Mergen Imeev via Tarantool-patches wrote:
> > > Prior to this patch string passed to user-defined Lua-function from SQL
> > > was cropped in case it contains '\0'. At the same time, it wasn't
> > > cropped if it is passed to the function from BOX. After this patch the
> > > string won't be cropped when passed from SQL if it contain '\0'.
> > > 
> > > Closes #5938
> > > ---
> > >  src/box/sql/func.c                            |  3 ++-
> > >  .../gh-5938-wrong-string-length.test.lua      | 19 ++++++++++++++++++-
> > >  2 files changed, 20 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> > > index c3c14bd22..7d5a55d3f 100644
> > > --- a/src/box/sql/func.c
> > > +++ b/src/box/sql/func.c
> > > @@ -120,7 +120,8 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
> > >  			lua_pushnumber(L, sql_value_double(param));
> > >  			break;
> > >  		case MP_STR:
> > > -			lua_pushstring(L, (const char *) sql_value_text(param));
> > > +			lua_pushlstring(L, (const char *) sql_value_text(param),
> > > +					(size_t) sql_value_bytes(param));
> > 
> > 2. Unary operators should not have a whitespace after them.
> Fixed.
> 
> > 
> > >  			break;
> > >  		case MP_BIN:
> > >  		case MP_ARRAY:
> 
> 
> New patch:
> 
> 
> commit 337a6cddd1d9e70c77e3ba48600b5d5ec9477dd7
> Author: Mergen Imeev <imeevma@gmail.com>
> Date:   Tue Mar 30 09:41:34 2021 +0300
> 
>     sql: ignore \0 in string passed to Lua-function
>     
>     Prior to this patch string passed to user-defined Lua-function from SQL
>     was cropped in case it contains '\0'. At the same time, it wasn't
>     cropped if it is passed to the function from BOX. After this patch the
>     string won't be cropped when passed from SQL if it contain '\0'.
>     
>     Closes #5938
> 
> diff --git "a/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md" "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
> new file mode 100644
> index 000000000..a0aa8a425
> --- /dev/null
> +++ "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
> @@ -0,0 +1,4 @@
> +## bugfix/sql
> +
> +* Fixed cropping of a string if it contains '\0' when passing a string
> +  to user-defined Lua or C functions.
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index c3c14bd22..9b6179f3a 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -120,7 +120,8 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
>  			lua_pushnumber(L, sql_value_double(param));
>  			break;
>  		case MP_STR:
> -			lua_pushstring(L, (const char *) sql_value_text(param));
> +			lua_pushlstring(L, (const char *)sql_value_text(param),
> +					(size_t)sql_value_bytes(param));
>  			break;
>  		case MP_BIN:
>  		case MP_ARRAY:
> diff --git a/test/sql-tap/gh-5938-wrong-string-length.test.lua b/test/sql-tap/gh-5938-wrong-string-length.test.lua
> index 943389e34..415fc7729 100755
> --- a/test/sql-tap/gh-5938-wrong-string-length.test.lua
> +++ b/test/sql-tap/gh-5938-wrong-string-length.test.lua
> @@ -3,7 +3,7 @@ local build_path = os.getenv("BUILDDIR")
>  package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath
>  
>  local test = require("sqltester")
> -test:plan(1)
> +test:plan(2)
>  
>  box.schema.func.create("gh-5938-wrong-string-length.ret_str", {
>      language = "C",
> @@ -25,4 +25,21 @@ test:do_execsql_test(
>          "This is a complete string","This is a cropped\0 string"
>      })
>  
> +box.schema.func.create("ret_str", {
> +    language = "Lua",
> +    body = [[function(str) return str end]],
> +    param_list = { "string" },
> +    returns = "string",
> +    exports = { "LUA", "SQL" },
> +    is_deterministic = true
> +})
> +
> +test:do_execsql_test(
> +    "gh-5938-2",
> +    [[
> +        SELECT "ret_str"(s) from t;
> +    ]], {
> +        "This is a complete string","This is a cropped\0 string"
> +    })
> +
>  test:finish_test()


Diff:


commit f13a1b4ddda6b6a26a7140fc67268f0ecf498a64
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Apr 1 13:39:12 2021 +0300

    Fix

diff --git "a/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md" b/changelogs/unreleased/fix-string-cropping-in-user-functions.md
similarity index 100%
rename from "changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
rename to changelogs/unreleased/fix-string-cropping-in-user-functions.md

Re: [Tarantool-patches] [PATCH v1 2/2] sql: ignore \0 in string passed to Lua-function

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the fixes!

> diff --git "a/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md" "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
> new file mode 100644
> index 000000000..a0aa8a425
> --- /dev/null
> +++ "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
> @@ -0,0 +1,4 @@
> +## bugfix/sql
> +
> +* Fixed cropping of a string if it contains '\0' when passing a string
> +  to user-defined Lua or C functions.

In the changelog records we use imperative, like in commit titles (because it
seems to be a commonly used way of writing them). Also you need to add the
issue reference to the text in a form `(gh-####)`. Look at the other changelog
files for examples.

Re: [Tarantool-patches] [PATCH v1 2/2] sql: ignore \0 in string passed to Lua-function

[Mergen Imeev via Tarantool-patches]

Hi! Thank you for the review. My asnwer and new patch below.

On Thu, Apr 01, 2021 at 09:51:06PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes!
> 
> > diff --git "a/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md" "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
> > new file mode 100644
> > index 000000000..a0aa8a425
> > --- /dev/null
> > +++ "b/changelogs/unreleased/fix-crop-strings-by-\\0-in-user-functions.md"
> > @@ -0,0 +1,4 @@
> > +## bugfix/sql
> > +
> > +* Fixed cropping of a string if it contains '\0' when passing a string
> > +  to user-defined Lua or C functions.
> 
> In the changelog records we use imperative, like in commit titles (because it
> seems to be a commonly used way of writing them). Also you need to add the
> issue reference to the text in a form `(gh-####)`. Look at the other changelog
> files for examples.
Thank you. Fixed.


New patch:


commit 3a9ba7639621c0afd8af1e4de8ef6bb639edeb6e
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Tue Mar 30 09:41:34 2021 +0300

    sql: ignore \0 in string passed to Lua-function
    
    Prior to this patch string passed to user-defined Lua-function from SQL
    was cropped in case it contains '\0'. At the same time, it wasn't
    cropped if it is passed to the function from BOX. After this patch the
    string won't be cropped when passed from SQL if it contain '\0'.
    
    Closes #5938

diff --git a/changelogs/unreleased/fix-string-cropping-in-user-functions.md b/changelogs/unreleased/fix-string-cropping-in-user-functions.md
new file mode 100644
index 000000000..f883aa0e0
--- /dev/null
+++ b/changelogs/unreleased/fix-string-cropping-in-user-functions.md
@@ -0,0 +1,5 @@
+## bugfix/sql
+
+* The string received by the user-defined C or Lua function could be different
+  from the string passed to the function. This could happen if the string passed
+  from SQL contains '\0' (gh-5938).
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index c3c14bd22..9b6179f3a 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -120,7 +120,8 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
 			lua_pushnumber(L, sql_value_double(param));
 			break;
 		case MP_STR:
-			lua_pushstring(L, (const char *) sql_value_text(param));
+			lua_pushlstring(L, (const char *)sql_value_text(param),
+					(size_t)sql_value_bytes(param));
 			break;
 		case MP_BIN:
 		case MP_ARRAY:
diff --git a/test/sql-tap/gh-5938-wrong-string-length.test.lua b/test/sql-tap/gh-5938-wrong-string-length.test.lua
index 943389e34..415fc7729 100755
--- a/test/sql-tap/gh-5938-wrong-string-length.test.lua
+++ b/test/sql-tap/gh-5938-wrong-string-length.test.lua
@@ -3,7 +3,7 @@ local build_path = os.getenv("BUILDDIR")
 package.cpath = build_path..'/test/sql-tap/?.so;'..build_path..'/test/sql-tap/?.dylib;'..package.cpath
 
 local test = require("sqltester")
-test:plan(1)
+test:plan(2)
 
 box.schema.func.create("gh-5938-wrong-string-length.ret_str", {
     language = "C",
@@ -25,4 +25,21 @@ test:do_execsql_test(
         "This is a complete string","This is a cropped\0 string"
     })
 
+box.schema.func.create("ret_str", {
+    language = "Lua",
+    body = [[function(str) return str end]],
+    param_list = { "string" },
+    returns = "string",
+    exports = { "LUA", "SQL" },
+    is_deterministic = true
+})
+
+test:do_execsql_test(
+    "gh-5938-2",
+    [[
+        SELECT "ret_str"(s) from t;
+    ]], {
+        "This is a complete string","This is a cropped\0 string"
+    })
+
 test:finish_test()

Re: [Tarantool-patches] [PATCH v1 0/2] sql: ignore \0 in string passed to user function

[Vladislav Shpilevoy via Tarantool-patches]

Thanks for the patchset!

LGTM.

Re: [Tarantool-patches] [PATCH v1 0/2] sql: ignore \0 in string passed to user function

[Kirill Yukhin via Tarantool-patches]

Hello,

On 30 мар 14:21, Mergen Imeev via Tarantool-patches wrote:
> Currently, string passed to user-defined function from SQL can be cropped in
> case it contains '\0'. This patch-set fixes this behaviour.
> 
> https://github.com/tarantool/tarantool/issues/5938
> https://github.com/tarantool/tarantool/tree/imeevma/gh-5938-wrong-string-length
> 
> Mergen Imeev (2):
>   sql: ignore \0 in string passed to C-function
>   sql: ignore \0 in string passed to Lua-function
> 
>  src/box/sql/func.c                            |  6 ++-
>  test/CMakeLists.txt                           |  1 +
>  test/sql-tap/CMakeLists.txt                   |  2 +
>  test/sql-tap/gh-5938-wrong-string-length.c    | 42 +++++++++++++++++
>  .../gh-5938-wrong-string-length.test.lua      | 45 +++++++++++++++++++
>  5 files changed, 94 insertions(+), 2 deletions(-)
>  create mode 100644 test/sql-tap/CMakeLists.txt
>  create mode 100644 test/sql-tap/gh-5938-wrong-string-length.c
>  create mode 100755 test/sql-tap/gh-5938-wrong-string-length.test.lua

LGTM.

I've checked your patchset into 2.6, 2.7 and master.

--
Regards, Kirill Yukhin