From a8c49c44e5a708b6400a421a55c90cbc8feb8a13 Mon Sep 17 00:00:00 2001 From: Jan Ciolek Date: Fri, 3 Feb 2023 21:37:31 +0100 Subject: [PATCH] cql/query_options: add a check for missing bind marker name There was a missing check in validation of named bind markers. Let's say that a user prepares a query like: ```cql INSERT INTO ks.tab (pk, ck, v) VALUES (:pk, :ck, :v) ``` Then they execute the query, but specify only values for `:pk` and `:ck`. We should detect that a value for :v is missing and throw an invalid_request_exception. Until now there was no such check, in case of a missing variable invalid `query_options` were created and Scylla crashed. Sadly it's impossible to create a regression test using `cql-pytest` or `boost`. `cql-pytest` uses the python driver, which silently ignores mising named bind variables, deciding that the user meant to send an UNSET_VALUE for them. When given values like `{'pk': 1, 'ck': 2}`, it will automaticaly extend them to `{'pk': 1, 'ck': 2, 'v': UNSET_VALUE}`. In `boost` I tried to use `cql_test_env`, but it only has methods which take valid `query_options` as a parameter. I could create a separate unit tests for the creation and validation of `query_options` but it won't be a true end-to-end test like `cql-pytest`. The bug was found using the rust driver, the reproducer is available in the issue description. Fixes: #12727 Signed-off-by: Jan Ciolek Closes #12730 (cherry picked from commit 2a5ed115caa072c11ccd2d275c6a800d6cdd3073) --- cql3/query_options.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cql3/query_options.cc b/cql3/query_options.cc index 47ea3725a8..1265252d97 100644 --- a/cql3/query_options.cc +++ b/cql3/query_options.cc @@ -135,12 +135,21 @@ void query_options::prepare(const std::vectorname->text(); + bool found_value_for_name = false; for (size_t j = 0; j < names.size(); j++) { if (names[j] == spec_name) { ordered_values.emplace_back(_value_views[j]); + found_value_for_name = true; break; } } + + // No bound value was found with the name `spec_name`. + // This means that the user forgot to include a bound value with such name. + if (!found_value_for_name) { + throw exceptions::invalid_request_exception( + format("Missing value for bind marker with name: {}", spec_name)); + } } _value_views = std::move(ordered_values); }