From 5a76e633ea9b5adb215e93fdc11e1c0c08b3fc74 Mon Sep 17 00:00:00 2001 From: Adam Cozzette Date: Thu, 17 Nov 2016 16:48:38 -0800 Subject: Integrated internal changes from Google --- .../java/com/google/protobuf/util/Durations.java | 8 ++--- .../com/google/protobuf/util/FieldMaskTree.java | 7 +++- .../java/com/google/protobuf/util/JsonFormat.java | 39 ++++++++++++++++------ .../java/com/google/protobuf/util/Timestamps.java | 8 ++--- .../com/google/protobuf/util/JsonFormatTest.java | 38 ++++++++++++++++++++- .../proto/com/google/protobuf/util/json_test.proto | 5 +++ 6 files changed, 80 insertions(+), 25 deletions(-) (limited to 'java/util') diff --git a/java/util/src/main/java/com/google/protobuf/util/Durations.java b/java/util/src/main/java/com/google/protobuf/util/Durations.java index 9333168d..46b21828 100644 --- a/java/util/src/main/java/com/google/protobuf/util/Durations.java +++ b/java/util/src/main/java/com/google/protobuf/util/Durations.java @@ -42,7 +42,6 @@ import static com.google.protobuf.util.Timestamps.NANOS_PER_MICROSECOND; import static com.google.protobuf.util.Timestamps.NANOS_PER_MILLISECOND; import static com.google.protobuf.util.Timestamps.NANOS_PER_SECOND; -import com.google.common.collect.ComparisonChain; import com.google.protobuf.Duration; import java.text.ParseException; import java.util.Comparator; @@ -71,11 +70,8 @@ public final class Durations { public int compare(Duration d1, Duration d2) { checkValid(d1); checkValid(d2); - - return ComparisonChain.start() - .compare(d1.getSeconds(), d2.getSeconds()) - .compare(d1.getNanos(), d2.getNanos()) - .result(); + int secDiff = Long.compare(d1.getSeconds(), d2.getSeconds()); + return (secDiff != 0) ? secDiff : Integer.compare(d1.getNanos(), d2.getNanos()); } }; diff --git a/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java b/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java index b577495d..b192b53e 100644 --- a/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java +++ b/java/util/src/main/java/com/google/protobuf/util/FieldMaskTree.java @@ -217,7 +217,12 @@ final class FieldMaskTree { Message source, Message.Builder destination, FieldMaskUtil.MergeOptions options) { - assert source.getDescriptorForType() == destination.getDescriptorForType(); + if (source.getDescriptorForType() != destination.getDescriptorForType()) { + throw new IllegalArgumentException( + String.format( + "source (%s) and destination (%s) descriptor must be equal", + source.getDescriptorForType(), destination.getDescriptorForType())); + } Descriptor descriptor = source.getDescriptorForType(); for (Entry entry : node.children.entrySet()) { diff --git a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java index 6361b4ac..7f6c8aea 100644 --- a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java +++ b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java @@ -67,7 +67,6 @@ import com.google.protobuf.Timestamp; import com.google.protobuf.UInt32Value; import com.google.protobuf.UInt64Value; import com.google.protobuf.Value; - import java.io.IOException; import java.io.Reader; import java.io.StringReader; @@ -224,7 +223,7 @@ public class JsonFormat { * Creates a {@link Parser} with default configuration. */ public static Parser parser() { - return new Parser(TypeRegistry.getEmptyTypeRegistry(), false); + return new Parser(TypeRegistry.getEmptyTypeRegistry(), false, Parser.DEFAULT_RECURSION_LIMIT); } /** @@ -233,10 +232,15 @@ public class JsonFormat { public static class Parser { private final TypeRegistry registry; private final boolean ignoringUnknownFields; + private final int recursionLimit; + + // The default parsing recursion limit is aligned with the proto binary parser. + private static final int DEFAULT_RECURSION_LIMIT = 100; - private Parser(TypeRegistry registry, boolean ignoreUnknownFields) { + private Parser(TypeRegistry registry, boolean ignoreUnknownFields, int recursionLimit) { this.registry = registry; this.ignoringUnknownFields = ignoreUnknownFields; + this.recursionLimit = recursionLimit; } /** @@ -249,16 +253,15 @@ public class JsonFormat { if (this.registry != TypeRegistry.getEmptyTypeRegistry()) { throw new IllegalArgumentException("Only one registry is allowed."); } - return new Parser(registry, this.ignoringUnknownFields); + return new Parser(registry, ignoringUnknownFields, recursionLimit); } /** - * Creates a new {@link Parser} configured to not throw an exception - * when an unknown field is encountered. The new Parser clones all other - * configurations from this Parser. + * Creates a new {@link Parser} configured to not throw an exception when an unknown field is + * encountered. The new Parser clones all other configurations from this Parser. */ public Parser ignoringUnknownFields() { - return new Parser(this.registry, true); + return new Parser(this.registry, true, recursionLimit); } /** @@ -270,7 +273,7 @@ public class JsonFormat { public void merge(String json, Message.Builder builder) throws InvalidProtocolBufferException { // TODO(xiaofeng): Investigate the allocation overhead and optimize for // mobile. - new ParserImpl(registry, ignoringUnknownFields).merge(json, builder); + new ParserImpl(registry, ignoringUnknownFields, recursionLimit).merge(json, builder); } /** @@ -283,7 +286,12 @@ public class JsonFormat { public void merge(Reader json, Message.Builder builder) throws IOException { // TODO(xiaofeng): Investigate the allocation overhead and optimize for // mobile. - new ParserImpl(registry, ignoringUnknownFields).merge(json, builder); + new ParserImpl(registry, ignoringUnknownFields, recursionLimit).merge(json, builder); + } + + // For testing only. + Parser usingRecursionLimit(int recursionLimit) { + return new Parser(registry, ignoringUnknownFields, recursionLimit); } } @@ -1040,11 +1048,15 @@ public class JsonFormat { private final TypeRegistry registry; private final JsonParser jsonParser; private final boolean ignoringUnknownFields; + private final int recursionLimit; + private int currentDepth; - ParserImpl(TypeRegistry registry, boolean ignoreUnknownFields) { + ParserImpl(TypeRegistry registry, boolean ignoreUnknownFields, int recursionLimit) { this.registry = registry; this.ignoringUnknownFields = ignoreUnknownFields; this.jsonParser = new JsonParser(); + this.recursionLimit = recursionLimit; + this.currentDepth = 0; } void merge(Reader json, Message.Builder builder) throws IOException { @@ -1715,8 +1727,13 @@ public class JsonFormat { case MESSAGE: case GROUP: + if (currentDepth >= recursionLimit) { + throw new InvalidProtocolBufferException("Hit recursion limit."); + } + ++currentDepth; Message.Builder subBuilder = builder.newBuilderForField(field); merge(json, subBuilder); + --currentDepth; return subBuilder.build(); default: diff --git a/java/util/src/main/java/com/google/protobuf/util/Timestamps.java b/java/util/src/main/java/com/google/protobuf/util/Timestamps.java index 52b1ab98..2160e4d5 100644 --- a/java/util/src/main/java/com/google/protobuf/util/Timestamps.java +++ b/java/util/src/main/java/com/google/protobuf/util/Timestamps.java @@ -37,7 +37,6 @@ import static com.google.common.math.LongMath.checkedAdd; import static com.google.common.math.LongMath.checkedMultiply; import static com.google.common.math.LongMath.checkedSubtract; -import com.google.common.collect.ComparisonChain; import com.google.protobuf.Duration; import com.google.protobuf.Timestamp; import java.text.ParseException; @@ -101,11 +100,8 @@ public final class Timestamps { public int compare(Timestamp t1, Timestamp t2) { checkValid(t1); checkValid(t2); - - return ComparisonChain.start() - .compare(t1.getSeconds(), t2.getSeconds()) - .compare(t1.getNanos(), t2.getNanos()) - .result(); + int secDiff = Long.compare(t1.getSeconds(), t2.getSeconds()); + return (secDiff != 0) ? secDiff : Integer.compare(t1.getNanos(), t2.getNanos()); } }; diff --git a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java index 32739d44..164ee54b 100644 --- a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java +++ b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java @@ -57,12 +57,15 @@ import com.google.protobuf.util.JsonTestProto.TestDuration; import com.google.protobuf.util.JsonTestProto.TestFieldMask; import com.google.protobuf.util.JsonTestProto.TestMap; import com.google.protobuf.util.JsonTestProto.TestOneof; +import com.google.protobuf.util.JsonTestProto.TestRecursive; import com.google.protobuf.util.JsonTestProto.TestStruct; import com.google.protobuf.util.JsonTestProto.TestTimestamp; import com.google.protobuf.util.JsonTestProto.TestWrappers; import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; +import java.util.HashMap; +import java.util.Map; import junit.framework.TestCase; public class JsonFormatTest extends TestCase { @@ -216,7 +219,9 @@ public class JsonFormatTest extends TestCase { TestMap.Builder mapBuilder = TestMap.newBuilder(); mapBuilder.putInt32ToEnumMapValue(1, 0); - mapBuilder.getMutableInt32ToEnumMapValue().put(2, 12345); + Map mapWithInvalidValues = new HashMap(); + mapWithInvalidValues.put(2, 12345); + mapBuilder.putAllInt32ToEnumMapValue(mapWithInvalidValues); TestMap mapMessage = mapBuilder.build(); assertEquals( "{\n" @@ -1140,6 +1145,7 @@ public class JsonFormatTest extends TestCase { // Expected. } } + public void testParserIgnoringUnknownFields() throws Exception { TestAllTypes.Builder builder = TestAllTypes.newBuilder(); String json = "{\n" + " \"unknownField\": \"XXX\"\n" + "}"; @@ -1358,4 +1364,34 @@ public class JsonFormatTest extends TestCase { Any any = builder.build(); assertEquals(0, any.getValue().size()); } + + public void testRecursionLimit() throws Exception { + String input = + "{\n" + + " \"nested\": {\n" + + " \"nested\": {\n" + + " \"nested\": {\n" + + " \"nested\": {\n" + + " \"value\": 1234\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n"; + + JsonFormat.Parser parser = JsonFormat.parser(); + TestRecursive.Builder builder = TestRecursive.newBuilder(); + parser.merge(input, builder); + TestRecursive message = builder.build(); + assertEquals(1234, message.getNested().getNested().getNested().getNested().getValue()); + + parser = JsonFormat.parser().usingRecursionLimit(3); + builder = TestRecursive.newBuilder(); + try { + parser.merge(input, builder); + fail("Exception is expected."); + } catch (InvalidProtocolBufferException e) { + // Expected. + } + } } diff --git a/java/util/src/test/proto/com/google/protobuf/util/json_test.proto b/java/util/src/test/proto/com/google/protobuf/util/json_test.proto index bd22f65a..a75338ef 100644 --- a/java/util/src/test/proto/com/google/protobuf/util/json_test.proto +++ b/java/util/src/test/proto/com/google/protobuf/util/json_test.proto @@ -201,3 +201,8 @@ message TestAny { message TestCustomJsonName { int32 value = 1 [json_name = "@value"]; } + +message TestRecursive { + int32 value = 1; + TestRecursive nested = 2; +} -- cgit v1.2.3