Fix issue where BinaryToJson a Skip()'s failure on unknown fields was ignored instead of resulting in a parse failure.

This harmonizes the BinaryToJson behavior with wire_format.cc skip unknown: it still has the odd implication of truncating the length varint, but it will now correctly parse-fail if the length is larger than the remaining bytes (before it just skipped to the end of the buffer) and also parse-fail if the 32nd bit is set (which is viewed as a negative length by CodedInputStream and would have just continued parsing without skipping anything)

Fixes #25092

PiperOrigin-RevId: 852791222

Author: protobuf-github-bot

src/google/protobuf/json/internal/untyped_message.cc

@@ -199,6 +199,10 @@ PROTOBUF_NOINLINE static absl::Status MakeTooDeepError() {
   return absl::InvalidArgumentError("allowed depth exceeded");
 }
 
+PROTOBUF_NOINLINE static absl::Status MakeMalformedLengthDelimError() {
+  return absl::InvalidArgumentError("malformed length-delimited field");
+}
+
 absl::Status UntypedMessage::Decode(io::CodedInputStream& stream,
                                     absl::optional<int32_t> current_group) {
   std::vector<int32_t> group_stack;
@@ -256,8 +260,9 @@ absl::Status UntypedMessage::Decode(io::CodedInputStream& stream,
           if (!stream.ReadVarint32(&x)) {
             return MakeUnexpectedEofError();
           }
-          // TODO: Remove this suppression.
-          (void)stream.Skip(x);
+          if (!stream.Skip(x)) {
+            return MakeMalformedLengthDelimError();
+          }
           continue;
         }
         case WireFormatLite::WIRETYPE_START_GROUP: {

src/google/protobuf/json/json_test.cc

@@ -1442,6 +1442,16 @@ TEST_P(JsonTest, UnknownGroupField) {
   EXPECT_EQ(out, "{}");
 }
 
+TEST_P(JsonTest, MalformedLengthDelimitedField) {
+  std::string out;
+  // An unknown length delimited field where the length is larger than
+  // the remaining input.
+  absl::Status s = BinaryToJsonString(resolver_.get(),
+                                      "type.googleapis.com/proto3.TestMessage",
+                                      "\xC2\x3E\x80\x80\x80\x80\x08", &out);
+  ASSERT_THAT(s, StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
 // JSON values get special treatment when it comes to pre-existing values in
 // their repeated fields, when parsing through their dedicated syntax.
 TEST_P(JsonTest, ClearPreExistingRepeatedInJsonValues) {