From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp21.mail.ru (smtp21.mail.ru [94.100.179.250]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 47EFF445320 for ; Sun, 12 Jul 2020 22:58:15 +0300 (MSK) From: Ilya Kosarev Date: Sun, 12 Jul 2020 22:58:12 +0300 Message-Id: <20200712195812.7189-1-i.kosarev@tarantool.org> Subject: [Tarantool-patches] [PATCH v2] engine: validate key in space_before_replace List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org 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