From 2f174c9af69c43a02876ef3950fe8d7d8aa7204f Mon Sep 17 00:00:00 2001 From: Andy Weiss Date: Mon, 7 Oct 2019 10:24:57 -0700 Subject: [PATCH 1/4] Support empty strings and vectors in standard codec Fixes #41993 Currently an empty string or vector will call through to WriteBytes which asserts that the number of bytes it is being asked to write is strictly positive. Instead we should not call WriteBytes if the length is zero. Similarly, when we read, we don't need to call out if the length is zero. --- .../cpp/client_wrapper/standard_codec.cc | 18 ++++++++++++++---- .../standard_message_codec_unittests.cc | 10 ++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/shell/platform/common/cpp/client_wrapper/standard_codec.cc b/shell/platform/common/cpp/client_wrapper/standard_codec.cc index 7c33fb4d969ec..53d639ee1040f 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_codec.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_codec.cc @@ -111,8 +111,10 @@ EncodableValue StandardCodecSerializer::ReadValue( case EncodedType::kString: { size_t size = ReadSize(stream); std::string string_value; - string_value.resize(size); - stream->ReadBytes(reinterpret_cast(&string_value[0]), size); + if (size > 0) { + string_value.resize(size); + stream->ReadBytes(reinterpret_cast(&string_value[0]), size); + } return EncodableValue(string_value); } case EncodedType::kUInt8List: @@ -126,6 +128,9 @@ EncodableValue StandardCodecSerializer::ReadValue( case EncodedType::kList: { size_t length = ReadSize(stream); EncodableList list_value; + if (length == 0) { + return EncodableValue(list_value); + } list_value.reserve(length); for (size_t i = 0; i < length; ++i) { list_value.push_back(ReadValue(stream)); @@ -176,8 +181,10 @@ void StandardCodecSerializer::WriteValue(const EncodableValue& value, const auto& string_value = value.StringValue(); size_t size = string_value.size(); WriteSize(size, stream); - stream->WriteBytes(reinterpret_cast(string_value.data()), - size); + if (size > 0) { + stream->WriteBytes( + reinterpret_cast(string_value.data()), size); + } break; } case EncodableValue::Type::kByteList: @@ -259,6 +266,9 @@ void StandardCodecSerializer::WriteVector( ByteBufferStreamWriter* stream) const { size_t count = vector.size(); WriteSize(count, stream); + if (count == 0) { + return; + } uint8_t type_size = static_cast(sizeof(T)); if (type_size > 1) { stream->WriteAlignment(type_size); diff --git a/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc b/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc index e56a44d62bebe..6f7f635d658e7 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc @@ -97,6 +97,11 @@ TEST(StandardMessageCodec, CanEncodeAndDecodeStringWithNonBMPCodePoint) { CheckEncodeDecode(EncodableValue(u8"h\U0001F602w"), bytes); } +TEST(StandardMessageCodex, CanEncodeAndDecodeEmptyString) { + std::vector bytes = {0x07, 0x00}; + CheckEncodeDecode(EncodableValue(u8""), bytes); +} + TEST(StandardMessageCodec, CanEncodeAndDecodeList) { std::vector bytes = { 0x0c, 0x05, 0x00, 0x07, 0x05, 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x06, @@ -117,6 +122,11 @@ TEST(StandardMessageCodec, CanEncodeAndDecodeList) { CheckEncodeDecode(value, bytes); } +TEST(StandardMessageCodec, CanEncodeAndDecodeEmptyList) { + std::vector bytes = {0x0c, 0x00}; + CheckEncodeDecode(EncodableValue(EncodableList{}), bytes); +} + TEST(StandardMessageCodec, CanEncodeAndDecodeMap) { std::vector bytes_prefix = {0x0d, 0x04}; EncodableValue value(EncodableMap{ From a0e69c317a9c130b910e885824c19a7cc9ee35d7 Mon Sep 17 00:00:00 2001 From: Andy Weiss Date: Mon, 7 Oct 2019 10:50:48 -0700 Subject: [PATCH 2/4] fix typo in test name --- .../cpp/client_wrapper/standard_message_codec_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc b/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc index 6f7f635d658e7..a0b4437b195f8 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_message_codec_unittests.cc @@ -97,7 +97,7 @@ TEST(StandardMessageCodec, CanEncodeAndDecodeStringWithNonBMPCodePoint) { CheckEncodeDecode(EncodableValue(u8"h\U0001F602w"), bytes); } -TEST(StandardMessageCodex, CanEncodeAndDecodeEmptyString) { +TEST(StandardMessageCodec, CanEncodeAndDecodeEmptyString) { std::vector bytes = {0x07, 0x00}; CheckEncodeDecode(EncodableValue(u8""), bytes); } From 25c0f0f756116d9d4389395614ad63da9680262b Mon Sep 17 00:00:00 2001 From: Andy Weiss Date: Mon, 7 Oct 2019 13:38:46 -0700 Subject: [PATCH 3/4] remove unnecessary length check in ReadValue for List --- shell/platform/common/cpp/client_wrapper/standard_codec.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/shell/platform/common/cpp/client_wrapper/standard_codec.cc b/shell/platform/common/cpp/client_wrapper/standard_codec.cc index 53d639ee1040f..110129b672ff9 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_codec.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_codec.cc @@ -128,9 +128,6 @@ EncodableValue StandardCodecSerializer::ReadValue( case EncodedType::kList: { size_t length = ReadSize(stream); EncodableList list_value; - if (length == 0) { - return EncodableValue(list_value); - } list_value.reserve(length); for (size_t i = 0; i < length; ++i) { list_value.push_back(ReadValue(stream)); From ed41ed667c740c0b70048c83c6b8296b2d31fb51 Mon Sep 17 00:00:00 2001 From: Andy Weiss Date: Mon, 7 Oct 2019 13:42:43 -0700 Subject: [PATCH 4/4] we also don't need this check before calling read as memcpy can handle size 0 --- shell/platform/common/cpp/client_wrapper/standard_codec.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/shell/platform/common/cpp/client_wrapper/standard_codec.cc b/shell/platform/common/cpp/client_wrapper/standard_codec.cc index 110129b672ff9..137a3e849002c 100644 --- a/shell/platform/common/cpp/client_wrapper/standard_codec.cc +++ b/shell/platform/common/cpp/client_wrapper/standard_codec.cc @@ -111,10 +111,8 @@ EncodableValue StandardCodecSerializer::ReadValue( case EncodedType::kString: { size_t size = ReadSize(stream); std::string string_value; - if (size > 0) { - string_value.resize(size); - stream->ReadBytes(reinterpret_cast(&string_value[0]), size); - } + string_value.resize(size); + stream->ReadBytes(reinterpret_cast(&string_value[0]), size); return EncodableValue(string_value); } case EncodedType::kUInt8List: