Merge pull request #14536 from vespa-engine/arnej/minor-codec-fixes

Arnej/minor codec fixes 

2 files changed, 133 insertions, 2 deletions

diff --git a/eval/src/tests/eval/value_codec/value_codec_test.cpp b/eval/src/tests/eval/value_codec/value_codec_test.cpp
index 01071b754fb..2b03cffe730 100644
--- a/eval/src/tests/eval/value_codec/value_codec_test.cpp
+++ b/eval/src/tests/eval/value_codec/value_codec_test.cpp
@@ -7,6 +7,7 @@
 #include <vespa/vespalib/data/memory.h>
 #include <vespa/vespalib/gtest/gtest.h>
 #include <vespa/vespalib/objects/nbostream.h>
+#include <vespa/vespalib/util/exceptions.h>
 
 using namespace vespalib;
 using namespace vespalib::eval;
@@ -213,6 +214,19 @@ TEST(ValueCodecTest, dense_tensors_can_be_encoded_and_decoded) {
     f1.verify_encode_decode(true);
 }
 
+TEST(ValueCodecTest, dense_tensors_without_values_are_filled) {
+    TensorSpec empty_dense_spec("tensor(x[3],y[2])");
+    auto value = value_from_spec(empty_dense_spec, SimpleValueBuilderFactory::get());
+    EXPECT_EQ(value->cells().size, 6);
+    auto cells = value->cells().typify<double>();
+    EXPECT_EQ(cells[0], 0.0);
+    EXPECT_EQ(cells[1], 0.0);
+    EXPECT_EQ(cells[2], 0.0);
+    EXPECT_EQ(cells[3], 0.0);
+    EXPECT_EQ(cells[4], 0.0);
+    EXPECT_EQ(cells[5], 0.0);
+}
+
 //-----------------------------------------------------------------------------
 
 struct MixedTensorExample : TensorExample {
@@ -273,4 +287,114 @@ TEST(ValueCodecTest, mixed_tensors_can_be_encoded_and_decoded) {
 
 //-----------------------------------------------------------------------------
 
+struct BadSparseTensorExample : TensorExample {
+    TensorSpec make_spec(bool use_float) const override {
+        return TensorSpec(make_type_spec(use_float, "(x{},y{})"))
+            .add({{"x","a"},{"y","a"}}, 1)
+            .add({{"x","b"},{"y","a"}}, 3);
+    }
+    std::unique_ptr<Value> make_tensor(bool use_float) const override {
+        return value_from_spec(make_spec(use_float), factory);
+    }
+    template <typename T>
+    void encode_inner(nbostream &dst) const {
+        dst.putInt1_4Bytes(2);
+        dst.writeSmallString("x");
+        dst.writeSmallString("y");
+        dst.putInt1_4Bytes(12345678);
+        dst.writeSmallString("a");
+        dst.writeSmallString("a");
+        dst << (T) 1;
+        dst.writeSmallString("b");
+        dst.writeSmallString("a");
+        dst << (T) 3;
+    }
+    void encode_default(nbostream &dst) const override {
+        dst.putInt1_4Bytes(1);
+        encode_inner<double>(dst);
+    }
+    void encode_with_double(nbostream &dst) const override {
+        dst.putInt1_4Bytes(5);
+        dst.putInt1_4Bytes(0);
+        encode_inner<double>(dst);
+    }
+    void encode_with_float(nbostream &dst) const override {
+        dst.putInt1_4Bytes(5);
+        dst.putInt1_4Bytes(1);
+        encode_inner<float>(dst);
+    }
+};
+
+TEST(ValueCodecTest, bad_sparse_tensors_are_caught) {
+    BadSparseTensorExample bad;
+    nbostream data_default;
+    nbostream data_double;
+    nbostream data_float;
+    bad.encode_default(data_default);
+    bad.encode_with_double(data_double);
+    bad.encode_with_float(data_float);
+    EXPECT_EXCEPTION(decode_value(data_default, factory), vespalib::IllegalStateException,
+                     "serialized input claims 12345678 blocks of size 1*8, but only");
+    EXPECT_EXCEPTION(decode_value(data_double, factory), vespalib::IllegalStateException,
+                     "serialized input claims 12345678 blocks of size 1*8, but only");
+    EXPECT_EXCEPTION(decode_value(data_float, factory), vespalib::IllegalStateException,
+                     "serialized input claims 12345678 blocks of size 1*4, but only");
+}
+
+//-----------------------------------------------------------------------------
+
+struct BadDenseTensorExample : TensorExample {
+    TensorSpec make_spec(bool use_float) const override {
+        return TensorSpec(make_type_spec(use_float, "(x[3],y[2])"))
+            .add({{"x",0},{"y",0}}, 1)
+            .add({{"x",2},{"y",1}}, 6);
+    }
+    std::unique_ptr<Value> make_tensor(bool use_float) const override {
+        return value_from_spec(make_spec(use_float), factory);
+    }
+    template <typename T>
+    void encode_inner(nbostream &dst) const {
+        dst.putInt1_4Bytes(2);
+        dst.writeSmallString("x");
+        dst.putInt1_4Bytes(300);
+        dst.writeSmallString("y");
+        dst.putInt1_4Bytes(200);
+        dst << (T) 1;
+        dst << (T) 6;
+    }
+    void encode_default(nbostream &dst) const override {
+        dst.putInt1_4Bytes(2);
+        encode_inner<double>(dst);
+    }
+    void encode_with_double(nbostream &dst) const override {
+        dst.putInt1_4Bytes(6);
+        dst.putInt1_4Bytes(0);
+        encode_inner<double>(dst);
+    }
+    void encode_with_float(nbostream &dst) const override {
+        dst.putInt1_4Bytes(6);
+        dst.putInt1_4Bytes(1);
+        encode_inner<float>(dst);
+    }
+};
+
+TEST(ValueCodecTest, bad_dense_tensors_are_caught) {
+    BadDenseTensorExample bad;
+    nbostream data_default;
+    nbostream data_double;
+    nbostream data_float;
+    bad.encode_default(data_default);
+    bad.encode_with_double(data_double);
+    bad.encode_with_float(data_float);
+    EXPECT_EXCEPTION(decode_value(data_default, factory), vespalib::IllegalStateException,
+                     "serialized input claims 1 blocks of size 60000*8, but only");
+    EXPECT_EXCEPTION(decode_value(data_double, factory), vespalib::IllegalStateException,
+                     "serialized input claims 1 blocks of size 60000*8, but only");
+    EXPECT_EXCEPTION(decode_value(data_float, factory), vespalib::IllegalStateException,
+                     "serialized input claims 1 blocks of size 60000*4, but only");
+}
+
+//-----------------------------------------------------------------------------
+
+
 GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/eval/src/vespa/eval/eval/value_codec.cpp b/eval/src/vespa/eval/eval/value_codec.cpp
index 1755ce164ff..74d0c5aafcc 100644
--- a/eval/src/vespa/eval/eval/value_codec.cpp
+++ b/eval/src/vespa/eval/eval/value_codec.cpp
@@ -3,14 +3,17 @@
 #include "value_codec.h"
 #include "tensor_spec.h"
 #include <vespa/vespalib/objects/nbostream.h>
+#include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/util/typify.h>
+#include <vespa/vespalib/util/stringfmt.h>
+
+using vespalib::make_string_short::fmt;
 
 namespace vespalib::eval {
 
 namespace {
 
 using CellType = ValueType::CellType;
-using IndexList = std::vector<size_t>;
 
 constexpr uint32_t DOUBLE_CELL_TYPE = 0;
 constexpr uint32_t FLOAT_CELL_TYPE = 1;
@@ -132,7 +135,6 @@ void decode_mapped_labels(nbostream &input, size_t num_mapped_dims, std::vector<
     }
 }
 
-
 template<typename T>
 void decode_cells(nbostream &input, size_t num_cells, ArrayRef<T> dst)
 {
@@ -154,6 +156,11 @@ struct ContentDecoder {
     template<typename T>
     static std::unique_ptr<Value> invoke(nbostream &input, const DecodeState &state, const ValueBuilderFactory &factory) {
         std::vector<vespalib::stringref> address(state.num_mapped_dims);
+        if (state.num_blocks * state.subspace_size * sizeof(T) > input.size()) {
+            auto err = fmt("serialized input claims %zu blocks of size %zu*%zu, but only %zu bytes available",
+                           state.num_blocks, state.subspace_size, sizeof(T), input.size());
+            throw IllegalStateException(err);
+        }
         auto builder = factory.create_value_builder<T>(state.type, state.num_mapped_dims, state.subspace_size, state.num_blocks);
         for (size_t i = 0; i < state.num_blocks; ++i) {
             decode_mapped_labels(input, state.num_mapped_dims, address);