Commit f7c45b69 authored by Findit's avatar Findit

Revert "Patch in Fix For Protobuf Deserialization Bug"

This reverts commit 5326cda9.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 833751 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzUzMjZjZGE5NmNhOTdlN2ZiMDdlYTBhMzNkODUyN2RmYmRkZmJiNWYM

Sample Failed Build: https://ci.chromium.org/b/8861821025876153104

Sample Failed Step: compile

Original change's description:
> Patch in Fix For Protobuf Deserialization Bug
> 
> This CL picks up a bugfix integrated into protobuf as part of commit:
> http://cl/342360226
> 
> This fix is responsible for fixing a bug in the serialization code
> when trying serialize/deserialize protobuf messages at the end of an
> iostream
> 
> Change-Id: I7608174a083b6b63c152c1250d3ccd1fd53914c6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2535667
> Reviewed-by: Leonard Grey <lgrey@chromium.org>
> Commit-Queue: Ryan Keane <rwkeane@google.com>
> Cr-Commit-Position: refs/heads/master@{#833751}


Change-Id: If9e7cb5da756a985bfc53fe03be0632e3cdbdef1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575436
Cr-Commit-Position: refs/heads/master@{#833786}
parent c64f63ad
...@@ -217,14 +217,6 @@ component("protobuf_lite") { ...@@ -217,14 +217,6 @@ component("protobuf_lite") {
} }
} }
source_set("delimited_mesage_util") {
sources = [
"src/google/protobuf/util/delimited_message_util.cc",
"src/google/protobuf/util/delimited_message_util.h",
]
deps = [ ":protobuf_lite" ]
}
# This is the full, heavy protobuf lib that's needed for c++ .protos that don't # This is the full, heavy protobuf lib that's needed for c++ .protos that don't
# specify the LITE_RUNTIME option. The protocol compiler itself (protoc) falls # specify the LITE_RUNTIME option. The protocol compiler itself (protoc) falls
# into that category. Do not use in Chrome code. # into that category. Do not use in Chrome code.
......
...@@ -79,9 +79,4 @@ Description of the patches: ...@@ -79,9 +79,4 @@ Description of the patches:
- 0022-Allow-deprecated-fields.patch - 0022-Allow-deprecated-fields.patch
Allows depreacated fields to be used without extra C++ compiler warnings. Allows depreacated fields to be used without extra C++ compiler warnings.
\ No newline at end of file
- 0023-fix-delimited-message-parsing.patch
Fixes a bug in delimited message parsing and serialization, as fixed in
protobuf commit with hash bd9a7104e11740e4bcfbde46c190c2d908ef331a
diff --git a/src/google/protobuf/util/delimited_message_util.cc b/src/google/protobuf/util/delimited_message_util.cc
index 425dc2cfdff8..80cab309be3d 100644
--- a/src/google/protobuf/util/delimited_message_util.cc
+++ b/src/google/protobuf/util/delimited_message_util.cc
@@ -74,12 +74,18 @@ bool ParseDelimitedFromCodedStream(MessageLite* message,
return false;
}
+ // Get the position after any size bytes have been read (and only the message
+ // itself remains).
+ int position_after_size = input->CurrentPosition();
+
// Tell the stream not to read beyond that size.
io::CodedInputStream::Limit limit = input->PushLimit(size);
// Parse the message.
if (!message->MergeFromCodedStream(input)) return false;
if (!input->ConsumedEntireMessage()) return false;
+ if (input->CurrentPosition() - position_after_size != static_cast<int>(size))
+ return false;
// Release the limit.
input->PopLimit(limit);
diff --git a/src/google/protobuf/util/delimited_message_util_test.cc b/src/google/protobuf/util/delimited_message_util_test.cc
index 9ed67784ee1c..9483a646e738 100644
--- a/src/google/protobuf/util/delimited_message_util_test.cc
+++ b/src/google/protobuf/util/delimited_message_util_test.cc
@@ -82,6 +82,35 @@ TEST(DelimitedMessageUtilTest, DelimitedMessages) {
}
}
+TEST(DelimitedMessageUtilTest, FailsAtEndOfStream) {
+ std::stringstream full_stream;
+ std::stringstream partial_stream;
+
+ {
+ protobuf_unittest::ForeignMessage message;
+ message.set_c(42);
+ message.set_d(24);
+ EXPECT_TRUE(SerializeDelimitedToOstream(message, &full_stream));
+
+ std::string full_output = full_stream.str();
+ ASSERT_GT(full_output.size(), size_t{2});
+ ASSERT_EQ(full_output[0], 4);
+
+ partial_stream << full_output[0] << full_output[1] << full_output[2];
+ }
+
+ {
+ bool clean_eof;
+ io::IstreamInputStream zstream(&partial_stream);
+
+ protobuf_unittest::ForeignMessage message;
+ clean_eof = true;
+ EXPECT_FALSE(ParseDelimitedFromZeroCopyStream(&message,
+ &zstream, &clean_eof));
+ EXPECT_FALSE(clean_eof);
+ }
+}
+
} // namespace util
} // namespace protobuf
} // namespace google
...@@ -74,18 +74,12 @@ bool ParseDelimitedFromCodedStream(MessageLite* message, ...@@ -74,18 +74,12 @@ bool ParseDelimitedFromCodedStream(MessageLite* message,
return false; return false;
} }
// Get the position after any size bytes have been read (and only the message
// itself remains).
int position_after_size = input->CurrentPosition();
// Tell the stream not to read beyond that size. // Tell the stream not to read beyond that size.
io::CodedInputStream::Limit limit = input->PushLimit(size); io::CodedInputStream::Limit limit = input->PushLimit(size);
// Parse the message. // Parse the message.
if (!message->MergeFromCodedStream(input)) return false; if (!message->MergeFromCodedStream(input)) return false;
if (!input->ConsumedEntireMessage()) return false; if (!input->ConsumedEntireMessage()) return false;
if (input->CurrentPosition() - position_after_size != static_cast<int>(size))
return false;
// Release the limit. // Release the limit.
input->PopLimit(limit); input->PopLimit(limit);
......
...@@ -82,35 +82,6 @@ TEST(DelimitedMessageUtilTest, DelimitedMessages) { ...@@ -82,35 +82,6 @@ TEST(DelimitedMessageUtilTest, DelimitedMessages) {
} }
} }
TEST(DelimitedMessageUtilTest, FailsAtEndOfStream) {
std::stringstream full_stream;
std::stringstream partial_stream;
{
protobuf_unittest::ForeignMessage message;
message.set_c(42);
message.set_d(24);
EXPECT_TRUE(SerializeDelimitedToOstream(message, &full_stream));
std::string full_output = full_stream.str();
ASSERT_GT(full_output.size(), size_t{2});
ASSERT_EQ(full_output[0], 4);
partial_stream << full_output[0] << full_output[1] << full_output[2];
}
{
bool clean_eof;
io::IstreamInputStream zstream(&partial_stream);
protobuf_unittest::ForeignMessage message;
clean_eof = true;
EXPECT_FALSE(ParseDelimitedFromZeroCopyStream(&message,
&zstream, &clean_eof));
EXPECT_FALSE(clean_eof);
}
}
} // namespace util } // namespace util
} // namespace protobuf } // namespace protobuf
} // namespace google } // namespace google
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment