diff --git a/repositories.bzl b/repositories.bzl index abb2af0383..523e0c7a0a 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -19,7 +19,6 @@ def grpc_java_repositories( omit_com_google_protobuf_java = False, omit_com_google_protobuf_javalite = False, omit_com_google_protobuf_nano_protobuf_javanano = False, - omit_com_google_re2j = False, omit_com_google_truth_truth = False, omit_com_squareup_okhttp = False, omit_com_squareup_okio = False, @@ -73,8 +72,6 @@ def grpc_java_repositories( com_google_protobuf_javalite() if not omit_com_google_protobuf_nano_protobuf_javanano: com_google_protobuf_nano_protobuf_javanano() - if not omit_com_google_re2j: - com_google_re2j() if not omit_com_google_truth_truth: com_google_truth_truth() if not omit_com_squareup_okhttp: @@ -265,15 +262,6 @@ def com_google_protobuf_nano_protobuf_javanano(): licenses = ["notice"], # BSD 2-clause ) -def com_google_re2j(): - jvm_maven_import_external( - name = "com_google_re2j", - artifact = "com.google.re2j:re2j:1.2", - server_urls = ["http://central.maven.org/maven2"], - artifact_sha256 = "e9dc705fd4c570344b54a7146b2e3a819cdc271a29793f4acc1a93b56a388e59", - licenses = ["notice"], # Go License - ) - def com_google_truth_truth(): jvm_maven_import_external( name = "com_google_truth_truth", diff --git a/services/BUILD.bazel b/services/BUILD.bazel index 04cdb83950..896e8015f8 100644 --- a/services/BUILD.bazel +++ b/services/BUILD.bazel @@ -19,7 +19,6 @@ java_library( "@com_google_guava_guava//jar", "@com_google_protobuf//:protobuf_java", "@com_google_protobuf//:protobuf_java_util", - "@com_google_re2j//jar", "@io_grpc_grpc_proto//:binarylog_java_proto", ], ) @@ -60,7 +59,6 @@ java_library( "@com_google_guava_guava//jar", "@com_google_protobuf//:protobuf_java", "@com_google_protobuf//:protobuf_java_util", - "@com_google_re2j//jar", "@io_grpc_grpc_proto//:reflection_java_proto_deprecated", "@javax_annotation_javax_annotation_api//jar", ], diff --git a/services/build.gradle b/services/build.gradle index 3b41c8f3ba..9fe6c87bca 100644 --- a/services/build.gradle +++ b/services/build.gradle @@ -24,7 +24,6 @@ dependencies { // prefer 26.0-android from libraries instead of 20.0 exclude group: 'com.google.guava', module: 'guava' } - compile libraries.re2j compileOnly libraries.javax_annotation testCompile project(':grpc-testing'), diff --git a/services/src/main/java/io/grpc/services/BinlogHelper.java b/services/src/main/java/io/grpc/services/BinlogHelper.java index ab503ca3a3..e38524d327 100644 --- a/services/src/main/java/io/grpc/services/BinlogHelper.java +++ b/services/src/main/java/io/grpc/services/BinlogHelper.java @@ -29,8 +29,6 @@ import com.google.protobuf.ByteString; import com.google.protobuf.Duration; import com.google.protobuf.util.Durations; import com.google.protobuf.util.Timestamps; -import com.google.re2j.Matcher; -import com.google.re2j.Pattern; import io.grpc.Attributes; import io.grpc.CallOptions; import io.grpc.Channel; @@ -601,20 +599,6 @@ final class BinlogHelper { } static final class FactoryImpl implements Factory { - // '*' for global, 'service/*' for service glob, or 'service/method' for fully qualified. - private static final Pattern logPatternRe = Pattern.compile("[^{]+"); - // A curly brace wrapped expression. Will be further matched with the more specified REs below. - private static final Pattern logOptionsRe = Pattern.compile("\\{[^}]+}"); - private static final Pattern configRe = Pattern.compile( - String.format("^(%s)(%s)?$", logPatternRe.pattern(), logOptionsRe.pattern())); - // Regexes to extract per-binlog options - // The form: {m:256} - private static final Pattern msgRe = Pattern.compile("\\{m(?::(\\d+))?}"); - // The form: {h:256} - private static final Pattern headerRe = Pattern.compile("\\{h(?::(\\d+))?}"); - // The form: {h:256,m:256} - private static final Pattern bothRe = Pattern.compile("\\{h(?::(\\d+))?;m(?::(\\d+))?}"); - private final BinlogHelper globalLog; private final Map perServiceLogs; private final Map perMethodLogs; @@ -632,12 +616,26 @@ final class BinlogHelper { Set blacklistedMethods = new HashSet<>(); if (configurationString != null && configurationString.length() > 0) { for (String configuration : Splitter.on(',').split(configurationString)) { - Matcher configMatcher = configRe.matcher(configuration); - if (!configMatcher.matches()) { + int leftCurly = configuration.indexOf('{'); + // '*' for global, 'service/*' for service glob, or 'service/method' for fully qualified + String methodOrSvc; + // An expression originally wrapped in curly braces; like {m:256,h:256}, {m:256}, {h:256} + String binlogOptionStr; + if (leftCurly == -1) { + methodOrSvc = configuration; + binlogOptionStr = null; + } else { + int rightCurly = configuration.indexOf('}', leftCurly); + if (rightCurly != configuration.length() - 1) { + throw new IllegalArgumentException("Illegal log config pattern: " + configuration); + } + methodOrSvc = configuration.substring(0, leftCurly); + // option without the curly braces + binlogOptionStr = configuration.substring(leftCurly + 1, configuration.length() - 1); + } + if (methodOrSvc.isEmpty()) { throw new IllegalArgumentException("Illegal log config pattern: " + configuration); } - String methodOrSvc = configMatcher.group(1); - String binlogOptionStr = configMatcher.group(2); if (methodOrSvc.equals("*")) { // parse config for "*" checkState( @@ -729,26 +727,21 @@ final class BinlogHelper { sink, TimeProvider.SYSTEM_TIME_PROVIDER, Integer.MAX_VALUE, Integer.MAX_VALUE)); } try { - Matcher headerMatcher; - Matcher msgMatcher; - Matcher bothMatcher; final int maxHeaderBytes; final int maxMsgBytes; - if ((headerMatcher = headerRe.matcher(logConfig)).matches()) { - String maxHeaderStr = headerMatcher.group(1); - maxHeaderBytes = - maxHeaderStr != null ? Integer.parseInt(maxHeaderStr) : Integer.MAX_VALUE; + String[] parts = logConfig.split(";", 2); + if (parts.length == 2) { + if (!(parts[0].startsWith("h") && parts[1].startsWith("m"))) { + throw new IllegalArgumentException("Illegal log config pattern"); + } + maxHeaderBytes = optionalInt(parts[0].substring(1)); + maxMsgBytes = optionalInt(parts[1].substring(1)); + } else if (parts[0].startsWith("h")) { + maxHeaderBytes = optionalInt(parts[0].substring(1)); maxMsgBytes = 0; - } else if ((msgMatcher = msgRe.matcher(logConfig)).matches()) { + } else if (parts[0].startsWith("m")) { maxHeaderBytes = 0; - String maxMsgStr = msgMatcher.group(1); - maxMsgBytes = maxMsgStr != null ? Integer.parseInt(maxMsgStr) : Integer.MAX_VALUE; - } else if ((bothMatcher = bothRe.matcher(logConfig)).matches()) { - String maxHeaderStr = bothMatcher.group(1); - String maxMsgStr = bothMatcher.group(2); - maxHeaderBytes = - maxHeaderStr != null ? Integer.parseInt(maxHeaderStr) : Integer.MAX_VALUE; - maxMsgBytes = maxMsgStr != null ? Integer.parseInt(maxMsgStr) : Integer.MAX_VALUE; + maxMsgBytes = optionalInt(parts[0].substring(1)); } else { throw new IllegalArgumentException("Illegal log config pattern"); } @@ -760,6 +753,29 @@ final class BinlogHelper { } } + /** Returns {@code s}, after verifying it contains only digits. */ + static String checkDigits(String s) { + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); + if (c < '0' || '9' < c) { + throw new IllegalArgumentException("Illegal log config pattern"); + } + } + return s; + } + + /** Parses the optional int of the form "" (max int) or ":123" (123). */ + static int optionalInt(String s) { + if (s.isEmpty()) { + return Integer.MAX_VALUE; + } + if (!s.startsWith(":")) { + throw new IllegalArgumentException("Illegal log config pattern"); + } + s = checkDigits(s.substring(1)); + return Integer.parseInt(s); + } + /** * Returns true if the input string is a glob of the form: {@code /*}. */ diff --git a/services/src/test/java/io/grpc/services/BinlogHelperTest.java b/services/src/test/java/io/grpc/services/BinlogHelperTest.java index a707491fc5..8162143791 100644 --- a/services/src/test/java/io/grpc/services/BinlogHelperTest.java +++ b/services/src/test/java/io/grpc/services/BinlogHelperTest.java @@ -226,18 +226,18 @@ public final class BinlogHelperTest { @Test public void createLogFromOptionString() throws Exception { - assertSameLimits(BOTH_FULL, makeLog(null)); - assertSameLimits(HEADER_FULL, makeLog("{h}")); - assertSameLimits(MSG_FULL, makeLog("{m}")); - assertSameLimits(HEADER_256, makeLog("{h:256}")); - assertSameLimits(MSG_256, makeLog("{m:256}")); - assertSameLimits(BOTH_256, makeLog("{h:256;m:256}")); + assertSameLimits(BOTH_FULL, makeOptions(null)); + assertSameLimits(HEADER_FULL, makeOptions("h")); + assertSameLimits(MSG_FULL, makeOptions("m")); + assertSameLimits(HEADER_256, makeOptions("h:256")); + assertSameLimits(MSG_256, makeOptions("m:256")); + assertSameLimits(BOTH_256, makeOptions("h:256;m:256")); assertSameLimits( new Builder().header(Integer.MAX_VALUE).msg(256).build(), - makeLog("{h;m:256}")); + makeOptions("h;m:256")); assertSameLimits( new Builder().header(256).msg(Integer.MAX_VALUE).build(), - makeLog("{h:256;m}")); + makeOptions("h:256;m")); } private void assertIllegalPatternDetected(String perSvcOrMethodConfig) { @@ -259,18 +259,30 @@ public final class BinlogHelperTest { } } + @Test + public void badFactoryConfigStrDetected_empty() throws Exception { + try { + new FactoryImpl(sink, "*,"); + fail(); + } catch (IllegalArgumentException expected) { + assertThat(expected).hasMessageThat().startsWith("Illegal log config pattern"); + } + } + @Test public void createLogFromOptionString_malformed() throws Exception { + assertIllegalPatternDetected(""); assertIllegalPatternDetected("bad"); - assertIllegalPatternDetected("{bad}"); - assertIllegalPatternDetected("{x;y}"); - assertIllegalPatternDetected("{h:abc}"); - assertIllegalPatternDetected("{2}"); - assertIllegalPatternDetected("{2;2}"); + assertIllegalPatternDetected("mad"); + assertIllegalPatternDetected("x;y"); + assertIllegalPatternDetected("h:abc"); + assertIllegalPatternDetected("h:1e8"); + assertIllegalPatternDetected("2"); + assertIllegalPatternDetected("2;2"); // The grammar specifies that if both h and m are present, h comes before m - assertIllegalPatternDetected("{m:123;h:123}"); + assertIllegalPatternDetected("m:123;h:123"); // NumberFormatException - assertIllegalPatternDetected("{h:99999999999999}"); + assertIllegalPatternDetected("h:99999999999999"); } @Test @@ -1530,7 +1542,7 @@ public final class BinlogHelperTest { return new BinlogHelper.FactoryImpl(sink, factoryConfigStr).getLog(lookup); } - private BinlogHelper makeLog(String logConfigStr) { + private BinlogHelper makeOptions(String logConfigStr) { return FactoryImpl.createBinaryLog(sink, logConfigStr); }