diff options
author | Håvard Pettersen <havardpe@oath.com> | 2021-03-24 14:56:45 +0000 |
---|---|---|
committer | Håvard Pettersen <havardpe@oath.com> | 2021-03-24 15:01:34 +0000 |
commit | 83605debe0a6b3a53b339f24992c9a54511d2a28 (patch) | |
tree | dcd73fa28405aba498d48cd01d15e5da1a903570 | |
parent | 08f5c940cd3160b07ea48ec1e814c7641597e078 (diff) |
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) |