mirror of
https://github.com/scylladb/scylladb.git
synced 2026-05-12 19:02:12 +00:00
large_data_handler: fix segmentation fault when constructing data_value from a nullptr
It turns out that `cql_table_large_data_handler::record_large_rows`
and `cql_table_large_data_handler::record_large_cells` were broken
for reporting static cells and static rows from the very beginning:
In case a large static cell or a large static row is encountered,
it tries to execute `db::try_record` with `nullptr` additional values,
denoting that there is no clustering key to be recorded.
These values are next passed to `qctx.execute_cql()`, which
creates `data_value` instances for each statement parameter,
hence invoking `data_value(nullptr)`.
This uses `const char*` overload which delegates to
`std::string_view` ctor overload. It is UB to pass `nullptr`
pointer to `std::string_view` ctor. Hence leading to
segmentation faults in the aforementioned large data reporting
code.
What we want here is to make a null `data_value` instead, so
just add an overload specifically for `std::nullptr_t`, which
will create a null `data_value` with `text` type.
A regression test is provided for the issue (written in
`cql-pytest` framework).
Tests: test/cql-pytest/test_large_cells_rows.py
Fixes: #6780
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Message-Id: <20201223204552.61081-1-pa.solodovnikov@scylladb.com>
(cherry picked from commit 219ac2bab5)
This commit is contained in:
committed by
Avi Kivity
parent
5bc48673aa
commit
06e785994f
@@ -113,7 +113,7 @@ future<> cql_table_large_data_handler::record_large_cells(const sstables::sstabl
|
||||
auto ck_str = key_to_str(*clustering_key, s);
|
||||
return try_record("cell", sst, partition_key, int64_t(cell_size), cell_type, format("{} {}", ck_str, column_name), extra_fields, ck_str, column_name);
|
||||
} else {
|
||||
return try_record("cell", sst, partition_key, int64_t(cell_size), cell_type, column_name, extra_fields, nullptr, column_name);
|
||||
return try_record("cell", sst, partition_key, int64_t(cell_size), cell_type, column_name, extra_fields, data_value::make_null(utf8_type), column_name);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -125,7 +125,7 @@ future<> cql_table_large_data_handler::record_large_rows(const sstables::sstable
|
||||
std::string ck_str = key_to_str(*clustering_key, s);
|
||||
return try_record("row", sst, partition_key, int64_t(row_size), "row", ck_str, extra_fields, ck_str);
|
||||
} else {
|
||||
return try_record("row", sst, partition_key, int64_t(row_size), "static row", "", extra_fields, nullptr);
|
||||
return try_record("row", sst, partition_key, int64_t(row_size), "static row", "", extra_fields, data_value::make_null(utf8_type));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
43
test/cql-pytest/test_large_cells_rows.py
Normal file
43
test/cql-pytest/test_large_cells_rows.py
Normal file
@@ -0,0 +1,43 @@
|
||||
# Copyright 2020 ScyllaDB
|
||||
#
|
||||
# This file is part of Scylla.
|
||||
#
|
||||
# Scylla is free software: you can redistribute it and/or modify
|
||||
# it under the terms of the GNU Affero General Public License as published by
|
||||
# the Free Software Foundation, either version 3 of the License, or
|
||||
# (at your option) any later version.
|
||||
#
|
||||
# Scylla is distributed in the hope that it will be useful,
|
||||
# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
# GNU General Public License for more details.
|
||||
#
|
||||
# You should have received a copy of the GNU Affero General Public License
|
||||
# along with Scylla. If not, see <http://www.gnu.org/licenses/>.
|
||||
|
||||
from util import new_test_table
|
||||
|
||||
import requests
|
||||
|
||||
def test_create_large_static_cells_and_rows(cql, test_keyspace):
|
||||
'''Test that `large_data_handler` successfully reports large static cells
|
||||
and static rows and this doesn't cause a crash of Scylla server.
|
||||
|
||||
This is a regression test for https://github.com/scylladb/scylla/issues/6780'''
|
||||
schema = "pk int, ck int, user_ids set<text> static, PRIMARY KEY (pk, ck)"
|
||||
with new_test_table(cql, test_keyspace, schema) as table:
|
||||
insert_stmt = cql.prepare(f"INSERT INTO {table} (pk, ck, user_ids) VALUES (?, ?, ?) USING TIMEOUT 5m")
|
||||
# Default large data threshold for cells is 1 mb, for rows it is 10 mb.
|
||||
# Take 10 mb cell to trigger large data reporting code both for
|
||||
# static cells and static rows simultaneously.
|
||||
large_set = {'x' * 1024 * 1024 * 10}
|
||||
cql.execute(insert_stmt, [1, 1, large_set])
|
||||
|
||||
# REST API endpoint address for test scylla node
|
||||
node_address = f'http://{cql.cluster.contact_points[0]}:10000'
|
||||
# Execute force flush of test table to persistent storage, which is necessary to trigger
|
||||
# `large_data_handler` execution.
|
||||
table_without_ks = table[table.find('.') + 1:] # strip keyspace part from the table name
|
||||
requests.post(f'{node_address}/storage_service/keyspace_flush/{test_keyspace}', params={'cf' : table_without_ks})
|
||||
# No need to check that the Scylla server is running here, since the test will
|
||||
# fail automatically in case Scylla crashes.
|
||||
8
types.hh
8
types.hh
@@ -380,6 +380,14 @@ public:
|
||||
data_value(const std::string&);
|
||||
data_value(const sstring&);
|
||||
|
||||
// Do not allow construction of a data_value from nullptr. The reason is
|
||||
// that this is error prone, for example: it conflicts with `const char*` overload
|
||||
// which tries to allocate a value from it and will cause UB.
|
||||
//
|
||||
// We want the null value semantics here instead. So the user will be forced
|
||||
// to explicitly call `make_null()` instead.
|
||||
data_value(std::nullptr_t) = delete;
|
||||
|
||||
data_value(ascii_native_type);
|
||||
data_value(bool);
|
||||
data_value(int8_t);
|
||||
|
||||
Reference in New Issue
Block a user