services: Remove dependency on re2j

re2j is a fairly unnecessary dependency. Our usage of Pattern is quite limited
and isn't all that hard to do manually. Our usage would be safe to use normal
java.util.regex, but it's sort of annoying to keep re-explaining to others who
are (rightly) concerned with java.util.regex's poor pathological behavior.
This commit is contained in:
Eric Anderson 2019-04-09 08:35:26 -07:00
parent 4887d99c2f
commit bcb4515832
5 changed files with 80 additions and 67 deletions

View File

@ -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",

View File

@ -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",
],

View File

@ -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'),

View File

@ -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<String, BinlogHelper> perServiceLogs;
private final Map<String, BinlogHelper> perMethodLogs;
@ -632,12 +616,26 @@ final class BinlogHelper {
Set<String> 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 <package-service>/*}.
*/

View File

@ -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);
}