diff options
author | Tor Egge <Tor.Egge@online.no> | 2024-02-16 12:50:58 +0100 |
---|---|---|
committer | Tor Egge <Tor.Egge@online.no> | 2024-02-16 12:50:58 +0100 |
commit | 833159e9447d17b46b332d26480a5fb2ace07dcb (patch) | |
tree | 2eac88357b6e7510cc2d31b30226a8a341fe514a | |
parent | 7c38895db17aff80157e34148be4fa9879719c4d (diff) |
Check buffer size when loading predicate attribute.
-rw-r--r-- | searchlib/src/tests/attribute/predicate_attribute/predicate_attribute_test.cpp | 74 | ||||
-rw-r--r-- | searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp | 6 |
2 files changed, 68 insertions, 12 deletions
diff --git a/searchlib/src/tests/attribute/predicate_attribute/predicate_attribute_test.cpp b/searchlib/src/tests/attribute/predicate_attribute/predicate_attribute_test.cpp index a3466be5e0e..a0d69f0ff85 100644 --- a/searchlib/src/tests/attribute/predicate_attribute/predicate_attribute_test.cpp +++ b/searchlib/src/tests/attribute/predicate_attribute/predicate_attribute_test.cpp @@ -2,11 +2,16 @@ #include <vespa/document/fieldvalue/predicatefieldvalue.h> #include <vespa/document/predicate/predicate_slime_builder.h> +#include <vespa/fastos/file.h> #include <vespa/searchcommon/attribute/config.h> #include <vespa/searchlib/attribute/attributefactory.h> #include <vespa/searchlib/attribute/attributevector.h> #include <vespa/searchlib/attribute/predicate_attribute.h> +#include <vespa/searchlib/common/fileheadertags.h> +#include <vespa/searchlib/util/file_settings.h> +#include <vespa/vespalib/data/fileheader.h> #include <vespa/vespalib/gtest/gtest.h> +#include <vespa/vespalib/util/exceptions.h> #include <filesystem> #include <memory> #include <sstream> @@ -16,9 +21,13 @@ using document::PredicateSlimeBuilder; using document::PredicateFieldValue; using search::AttributeFactory; using search::AttributeVector; +using search::FileSettings; using search::PredicateAttribute; using search::attribute::Config; using search::attribute::BasicType; +using search::tags::FILE_BIT_SIZE; +using vespalib::IllegalStateException; +using Tag = vespalib::GenericHeader::Tag; namespace { @@ -62,6 +71,40 @@ fv_as_string(const FieldValue& val) return os.str(); } +std::shared_ptr<AttributeVector> +make_sample_predicate_attribute() +{ + auto cfg = get_predicate_with_arity(2); + auto attr = make_attribute(attr_name, cfg, true); + auto& pattr = dynamic_cast<PredicateAttribute&>(*attr); + PredicateSlimeBuilder builder; + builder.neg().feature("foo").value("bar").value("baz"); + PredicateFieldValue val(builder.build()); + EXPECT_EQ("'foo' not in ['bar','baz']\n", fv_as_string(val)); + attr->addDocs(10); + pattr.updateValue(1, val); + attr->commit(); + EXPECT_TRUE(attr->isLoaded()); + return attr; +} + +void +corrupt_file_header(const vespalib::string &name) +{ + vespalib::FileHeader h(FileSettings::DIRECTIO_ALIGNMENT); + FastOS_File f; + auto file_bit_size = 0; + f.OpenReadWrite(name.c_str()); + h.readFile(f); + if (h.hasTag(FILE_BIT_SIZE)) { + file_bit_size = h.getTag(FILE_BIT_SIZE).asInteger() - 8; + } + h.putTag(Tag(FILE_BIT_SIZE, file_bit_size)); + h.rewriteFile(f); + bool sync_ok = f.Sync(); + assert(sync_ok); +} + } class PredicateAttributeTest : public ::testing::Test @@ -91,25 +134,32 @@ PredicateAttributeTest::TearDown() TEST_F(PredicateAttributeTest, save_and_load_predicate_attribute) { - auto cfg = get_predicate_with_arity(2); - auto attr = make_attribute(attr_name, cfg, true); - auto& pattr = dynamic_cast<PredicateAttribute&>(*attr); - PredicateSlimeBuilder builder; - builder.neg().feature("foo").value("bar").value("baz"); - PredicateFieldValue val(builder.build()); - EXPECT_EQ("'foo' not in ['bar','baz']\n", fv_as_string(val)); - attr->addDocs(10); - pattr.updateValue(1, val); - attr->commit(); - EXPECT_TRUE(attr->isLoaded()); + auto attr = make_sample_predicate_attribute(); std::filesystem::path file_name(tmp_dir); file_name.append(attr_name); attr->save(file_name.native()); - auto attr2 = make_attribute(file_name.native(), cfg, false); + auto attr2 = make_attribute(file_name.native(), attr->getConfig(), false); EXPECT_FALSE(attr2->isLoaded()); EXPECT_TRUE(attr2->load()); EXPECT_TRUE(attr2->isLoaded()); EXPECT_EQ(11, attr2->getCommittedDocIdLimit()); } +TEST_F(PredicateAttributeTest, buffer_size_mismatch_is_fatal_during_load) +{ + auto attr = make_sample_predicate_attribute(); + std::filesystem::path file_name(tmp_dir); + file_name.append(attr_name); + attr->save(file_name.native()); + corrupt_file_header(file_name.native() + ".dat"); + auto attr2 = make_attribute(file_name.native(), attr->getConfig(), false); + EXPECT_FALSE(attr2->isLoaded()); + try { + attr2->load(); + FAIL() << "Expected exception not thrown when loading corrupt attribute"; + } catch (IllegalStateException& e) { + EXPECT_EQ("Deserialize error when loading predicate attribute 'test', -1 bytes remaining in buffer", e.getMessage()); + } +} + GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp index 1c273f17176..60898450e50 100644 --- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp @@ -12,6 +12,7 @@ #include <vespa/searchlib/util/fileutil.h> #include <vespa/searchcommon/attribute/config.h> #include <vespa/vespalib/data/slime/slime.h> +#include <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/util/size_literals.h> #include <vespa/log/log.h> @@ -20,6 +21,8 @@ LOG_SETUP(".searchlib.attribute.predicate_attribute"); using document::Predicate; using document::PredicateFieldValue; using vespalib::DataBuffer; +using vespalib::IllegalStateException; +using vespalib::make_string; using namespace search::predicate; namespace search { @@ -236,6 +239,9 @@ PredicateAttribute::onLoad(vespalib::Executor *) _interval_range_vector[docId] = version < 2 ? MAX_INTERVAL_RANGE : buffer.readInt16(); } _max_interval_range = version < 2 ? MAX_INTERVAL_RANGE : buffer.readInt16(); + if (buffer.getDataLen() != 0) { + throw IllegalStateException(make_string("Deserialize error when loading predicate attribute '%s', %" PRId64 " bytes remaining in buffer", getName().c_str(), (int64_t) buffer.getDataLen())); + } _index->adjustDocIdLimit(highest_doc_id); setNumDocs(highest_doc_id + 1); setCommittedDocIdLimit(highest_doc_id + 1); |