[Tarantool-patches] [PATCH v2] engine: validate key in space_before_replace

[Ilya Kosarev]

Since 9fba29abb4e05babb9b23b4413bd8083f0fba933 (memtx: introduce tuple
compare hint) index_get uses key_hint to get a comparison hint for the
key. It requires key type to be validated. However in
space_before_replace index_get was used without key validation on some
execution paths. Now it is fixed and validation is being performed if
needed. Corresponding test case is introduced. Also
space_before_replace() is minor refactored to reduce conditions amount
on a hot path.

Closes #5093
---
Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-5093-before_replace-key-validation
Issue: https://github.com/tarantool/tarantool/issues/5093

@ChangeLog:
 * Add needed key validation to space_before_replace (gh-5093).

Changes in v2:
- refactored to avoid extra conditions
- added cleanup in test

 src/box/space.c                               | 15 ++++++------
 .../gh-5093-before_replace-key-type.result    | 24 +++++++++++++++++++
 .../gh-5093-before_replace-key-type.test.lua  |  8 +++++++
 3 files changed, 40 insertions(+), 7 deletions(-)
 create mode 100644 test/engine/gh-5093-before_replace-key-type.result
 create mode 100644 test/engine/gh-5093-before_replace-key-type.test.lua

diff --git a/src/box/space.c b/src/box/space.c
index eecbde7fa..1d375cc41 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -347,6 +347,7 @@ space_before_replace(struct space *space, struct txn *txn,
 	const char *key = NULL;
 	uint32_t part_count = 0;
 	struct index *index = NULL;
+	struct tuple *old_tuple = NULL;
 
 	/*
 	 * Lookup the old tuple.
@@ -359,15 +360,12 @@ space_before_replace(struct space *space, struct txn *txn,
 			return -1;
 		key = request->key;
 		part_count = mp_decode_array(&key);
-		if (exact_key_validate(index->def->key_def,
-				       key, part_count) != 0)
-			return -1;
 		break;
 	case IPROTO_INSERT:
 	case IPROTO_REPLACE:
 	case IPROTO_UPSERT:
 		if (pk == NULL)
-			break;
+			goto after_old_tuple_lookup;
 		index = pk;
 		key = tuple_extract_key_raw(request->tuple, request->tuple_end,
 					    index->def->key_def, MULTIKEY_NONE,
@@ -381,11 +379,14 @@ space_before_replace(struct space *space, struct txn *txn,
 		return 0;
 	}
 
-	struct tuple *old_tuple = NULL;
-	if (index != NULL &&
-	    index_get(index, key, part_count, &old_tuple) != 0)
+	if (exact_key_validate(index->def->key_def, key, part_count) != 0)
 		return -1;
 
+	if (index_get(index, key, part_count, &old_tuple) != 0)
+		return -1;
+
+after_old_tuple_lookup:;
+
 	/*
 	 * Create the new tuple.
 	 */
diff --git a/test/engine/gh-5093-before_replace-key-type.result b/test/engine/gh-5093-before_replace-key-type.result
new file mode 100644
index 000000000..890e62666
--- /dev/null
+++ b/test/engine/gh-5093-before_replace-key-type.result
@@ -0,0 +1,24 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+trigger = function() end
+ | ---
+ | ...
+
+s = box.schema.space.create('gh-5093', {engine=test_run:get_cfg('engine')})
+ | ---
+ | ...
+_ = s:create_index('value', {parts={{1, type='decimal'}}})
+ | ---
+ | ...
+_ = s:before_replace(trigger)
+ | ---
+ | ...
+s:replace{'1111.1111'}
+ | ---
+ | - error: 'Supplied key type of part 0 does not match index part type: expected decimal'
+ | ...
+s:drop()
+ | ---
+ | ...
diff --git a/test/engine/gh-5093-before_replace-key-type.test.lua b/test/engine/gh-5093-before_replace-key-type.test.lua
new file mode 100644
index 000000000..565e3defb
--- /dev/null
+++ b/test/engine/gh-5093-before_replace-key-type.test.lua
@@ -0,0 +1,8 @@
+test_run = require('test_run').new()
+trigger = function() end
+
+s = box.schema.space.create('gh-5093', {engine=test_run:get_cfg('engine')})
+_ = s:create_index('value', {parts={{1, type='decimal'}}})
+_ = s:before_replace(trigger)
+s:replace{'1111.1111'}
+s:drop()
-- 
2.17.1