handle value decoding failures more gracefully

7 files changed, 59 insertions, 29 deletions

diff --git a/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp b/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp
index 6567b8cb6b5..d88f7437112 100644
--- a/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp
+++ b/document/src/vespa/document/serialization/vespadocumentdeserializer.cpp
@@ -373,7 +373,11 @@ VespaDocumentDeserializer::readTensor()
     std::unique_ptr<vespalib::eval::Value> tensor;
     if (length != 0) {
         nbostream wrapStream(_stream.peek(), length);
-        tensor = vespalib::eval::decode_value(wrapStream, FastValueBuilderFactory::get());
+        try {
+            tensor = vespalib::eval::decode_value(wrapStream, FastValueBuilderFactory::get());
+        } catch (const vespalib::eval::DecodeValueException &e) {
+            throw DeserializeException("tensor value decode failed", e, VESPA_STRLOC);
+        }
         if (wrapStream.size() != 0) {
             throw DeserializeException("Leftover bytes deserializing tensor field value.", VESPA_STRLOC);
         }
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 0bb1bcfb337..99afba4aed9 100644
--- a/eval/src/tests/eval/value_codec/value_codec_test.cpp
+++ b/eval/src/tests/eval/value_codec/value_codec_test.cpp
@@ -335,11 +335,11 @@ TEST(ValueCodecTest, bad_sparse_tensors_are_caught) {
     bad.encode_default(data_default);
     bad.encode_with_double(data_double);
     bad.encode_with_float(data_float);
-    VESPA_EXPECT_EXCEPTION(decode_value(data_default, factory), vespalib::IllegalStateException,
+    VESPA_EXPECT_EXCEPTION(decode_value(data_default, factory), vespalib::eval::DecodeValueException,
                      "serialized input claims 12345678 blocks of size 1*8, but only");
-    VESPA_EXPECT_EXCEPTION(decode_value(data_double, factory), vespalib::IllegalStateException,
+    VESPA_EXPECT_EXCEPTION(decode_value(data_double, factory), vespalib::eval::DecodeValueException,
                      "serialized input claims 12345678 blocks of size 1*8, but only");
-    VESPA_EXPECT_EXCEPTION(decode_value(data_float, factory), vespalib::IllegalStateException,
+    VESPA_EXPECT_EXCEPTION(decode_value(data_float, factory), vespalib::eval::DecodeValueException,
                      "serialized input claims 12345678 blocks of size 1*4, but only");
 }
 
@@ -388,11 +388,11 @@ TEST(ValueCodecTest, bad_dense_tensors_are_caught) {
     bad.encode_default(data_default);
     bad.encode_with_double(data_double);
     bad.encode_with_float(data_float);
-    VESPA_EXPECT_EXCEPTION(decode_value(data_default, factory), vespalib::IllegalStateException,
+    VESPA_EXPECT_EXCEPTION(decode_value(data_default, factory), vespalib::eval::DecodeValueException,
                      "serialized input claims 1 blocks of size 60000*8, but only");
-    VESPA_EXPECT_EXCEPTION(decode_value(data_double, factory), vespalib::IllegalStateException,
+    VESPA_EXPECT_EXCEPTION(decode_value(data_double, factory), vespalib::eval::DecodeValueException,
                      "serialized input claims 1 blocks of size 60000*8, but only");
-    VESPA_EXPECT_EXCEPTION(decode_value(data_float, factory), vespalib::IllegalStateException,
+    VESPA_EXPECT_EXCEPTION(decode_value(data_float, factory), vespalib::eval::DecodeValueException,
                      "serialized input claims 1 blocks of size 60000*4, but only");
 }
 
diff --git a/eval/src/vespa/eval/eval/value_codec.cpp b/eval/src/vespa/eval/eval/value_codec.cpp
index bf45f34fd64..85feadca85e 100644
--- a/eval/src/vespa/eval/eval/value_codec.cpp
+++ b/eval/src/vespa/eval/eval/value_codec.cpp
@@ -14,6 +14,8 @@ using vespalib::make_string_short::fmt;
 
 namespace vespalib::eval {
 
+VESPA_IMPLEMENT_EXCEPTION(DecodeValueException, Exception);
+
 namespace {
 
 constexpr uint32_t DOUBLE_CELL_TYPE = 0;
@@ -308,13 +310,19 @@ void encode_value(const Value &value, nbostream &output) {
 }
 
 std::unique_ptr<Value> decode_value(nbostream &input, const ValueBuilderFactory &factory) {
-    Format format(input.getInt1_4Bytes());
-    ValueType type = decode_type(input, format);
-    size_t num_mapped_dims = type.count_mapped_dimensions();
-    size_t dense_subspace_size = type.dense_subspace_size();
-    const size_t num_blocks = maybe_decode_num_blocks(input, (num_mapped_dims > 0), format);
-    DecodeState state{type, dense_subspace_size, num_blocks, num_mapped_dims};
-    return typify_invoke<1,TypifyCellType,ContentDecoder>(type.cell_type(), input, state, factory);
+    try {
+        Format format(input.getInt1_4Bytes());
+        ValueType type = decode_type(input, format);
+        size_t num_mapped_dims = type.count_mapped_dimensions();
+        size_t dense_subspace_size = type.dense_subspace_size();
+        const size_t num_blocks = maybe_decode_num_blocks(input, (num_mapped_dims > 0), format);
+        DecodeState state{type, dense_subspace_size, num_blocks, num_mapped_dims};
+        return typify_invoke<1,TypifyCellType,ContentDecoder>(type.cell_type(), input, state, factory);
+    } catch (const OOMException &) {
+        throw;
+    } catch (const Exception &e) {
+        throw DecodeValueException("failed to decode value", e);
+    }
 }
 
 //-----------------------------------------------------------------------------
diff --git a/eval/src/vespa/eval/eval/value_codec.h b/eval/src/vespa/eval/eval/value_codec.h
index 058b2d7bf4f..23eb2de8e41 100644
--- a/eval/src/vespa/eval/eval/value_codec.h
+++ b/eval/src/vespa/eval/eval/value_codec.h
@@ -5,11 +5,14 @@
 #include "value.h"
 #include "tensor_spec.h"
 #include <vespa/vespalib/stllike/string.h>
+#include <vespa/vespalib/util/exception.h>
 
 namespace vespalib { class nbostream; }
 
 namespace vespalib::eval {
 
+VESPA_DEFINE_EXCEPTION(DecodeValueException, Exception);
+
 /**
  * encode a value (which must support the new APIs) to binary format
  **/
@@ -17,6 +20,8 @@ void encode_value(const Value &value, nbostream &output);
 
 /**
  * decode a value from binary format
+ *
+ * will throw DecodeValueException if input contains malformed data
  **/
 std::unique_ptr<Value> decode_value(nbostream &input, const ValueBuilderFactory &factory);
 
diff --git a/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp b/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp
index 06f0de631be..a67e74f3978 100644
--- a/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp
+++ b/searchlib/src/vespa/searchlib/features/dotproductfeature.cpp
@@ -501,12 +501,16 @@ template <typename T>
 ArrayParam<T>::ArrayParam(vespalib::nbostream & stream) {
     using vespalib::typify_invoke;
     using vespalib::eval::TypifyCellType;
-    auto tensor = vespalib::eval::decode_value(stream, FastValueBuilderFactory::get());
-    if (tensor->type().is_dense()) {
-        TypedCells cells = tensor->cells();
-        typify_invoke<1,TypifyCellType,CopyCellsToVector<T>>(cells.type, cells, values);
-    } else {
-        LOG(warning, "Expected dense tensor, but got type '%s'", tensor->type().to_spec().c_str());
+    try {
+        auto tensor = vespalib::eval::decode_value(stream, FastValueBuilderFactory::get());
+        if (tensor->type().is_dense()) {
+            TypedCells cells = tensor->cells();
+            typify_invoke<1,TypifyCellType,CopyCellsToVector<T>>(cells.type, cells, values);
+        } else {
+            LOG(warning, "Expected dense tensor, but got type '%s'", tensor->type().to_spec().c_str());
+        }
+    } catch (const vespalib::eval::DecodeValueException &e) {
+        LOG(warning, "Failed to decode tensor: %s", e.what());
     }
 }
 
diff --git a/searchlib/src/vespa/searchlib/features/queryfeature.cpp b/searchlib/src/vespa/searchlib/features/queryfeature.cpp
index 60bd77e4883..a69147e64d5 100644
--- a/searchlib/src/vespa/searchlib/features/queryfeature.cpp
+++ b/searchlib/src/vespa/searchlib/features/queryfeature.cpp
@@ -121,13 +121,18 @@ createTensorExecutor(const IQueryEnvironment &env,
     if (prop.found() && !prop.get().empty()) {
         const vespalib::string &value = prop.get();
         vespalib::nbostream stream(value.data(), value.size());
-        auto tensor = vespalib::eval::decode_value(stream, vespalib::eval::FastValueBuilderFactory::get());
-        if (!TensorDataType::isAssignableType(valueType, tensor->type())) {
-            LOG(warning, "Query feature type is '%s' but other tensor type is '%s'",
-                valueType.to_spec().c_str(), tensor->type().to_spec().c_str());
+        try {
+            auto tensor = vespalib::eval::decode_value(stream, vespalib::eval::FastValueBuilderFactory::get());
+            if (!TensorDataType::isAssignableType(valueType, tensor->type())) {
+                LOG(warning, "Query feature type is '%s' but other tensor type is '%s'",
+                    valueType.to_spec().c_str(), tensor->type().to_spec().c_str());
+                return ConstantTensorExecutor::createEmpty(valueType, stash);
+            }
+            return ConstantTensorExecutor::create(std::move(tensor), stash);
+        } catch (const vespalib::eval::DecodeValueException &e) {
+            LOG(warning, "Query feature has invalid binary format: %s", e.what());
             return ConstantTensorExecutor::createEmpty(valueType, stash);
         }
-        return ConstantTensorExecutor::create(std::move(tensor), stash);
     }
     return ConstantTensorExecutor::createEmpty(valueType, stash);
 }
diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_deserialize.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_deserialize.cpp
index 4fddd092451..3a00cb229e3 100644
--- a/searchlib/src/vespa/searchlib/tensor/tensor_deserialize.cpp
+++ b/searchlib/src/vespa/searchlib/tensor/tensor_deserialize.cpp
@@ -14,11 +14,15 @@ namespace search::tensor {
 
 std::unique_ptr<Value> deserialize_tensor(vespalib::nbostream &buffer)
 {
-    auto tensor = vespalib::eval::decode_value(buffer, FastValueBuilderFactory::get());
-    if (buffer.size() != 0) {
-        throw DeserializeException("Leftover bytes deserializing tensor attribute value.", VESPA_STRLOC);
+    try {
+        auto tensor = vespalib::eval::decode_value(buffer, FastValueBuilderFactory::get());
+        if (buffer.size() != 0) {
+            throw DeserializeException("Leftover bytes deserializing tensor attribute value.", VESPA_STRLOC);
+        }
+        return tensor;
+    } catch (const vespalib::eval::DecodeValueException &e) {
+        throw DeserializeException("tensor value decode failed", e, VESPA_STRLOC);
     }
-    return tensor;
 }
 
 std::unique_ptr<Value> deserialize_tensor(const void *data, size_t size)