Make MetadataParser more resistent
In this commit:
- Make MetadataParser resistent to blanks
- Convert all CRLF to LF for unified processing
- For HTML processing split on <div> instead of <p> to adapt the parser
to the new email format we send out to users.
Change-Id: I7e0316915765dab256771467e0e8345ca75802aa
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index c62839e..7db5b9e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -285,7 +285,7 @@
+ "Gerrit-MessageType: comment\n"
+ "Gerrit-Comment-Date: "
+ timestamp
- + "\n";
+ + " \n";
}
private MailMessage.Builder messageBuilderWithDefaultFields() {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java
index c812568..f703ed7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java
@@ -23,9 +23,14 @@
import com.google.gerrit.server.mail.MetadataName;
import java.sql.Timestamp;
import java.time.Instant;
+import java.time.format.DateTimeParseException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/** Parse metadata from inbound email */
public class MetadataParser {
+ private static final Logger log = LoggerFactory.getLogger(MailProcessor.class.getName());
+
public static MailMetadata parse(MailMessage m) {
MailMetadata metadata = new MailMetadata();
// Find author
@@ -40,8 +45,12 @@
String ps = header.substring(toHeaderWithDelimiter(MetadataName.PATCH_SET).length());
metadata.patchSet = Ints.tryParse(ps);
} else if (header.startsWith(toHeaderWithDelimiter(MetadataName.TIMESTAMP))) {
- String ts = header.substring(toHeaderWithDelimiter(MetadataName.TIMESTAMP).length());
- metadata.timestamp = Timestamp.from(MailUtil.rfcDateformatter.parse(ts, Instant::from));
+ String ts = header.substring(toHeaderWithDelimiter(MetadataName.TIMESTAMP).length()).trim();
+ try {
+ metadata.timestamp = Timestamp.from(MailUtil.rfcDateformatter.parse(ts, Instant::from));
+ } catch (DateTimeParseException e) {
+ log.error("Mail: Error while parsing timestamp from header of message " + m.id(), e);
+ }
} else if (header.startsWith(toHeaderWithDelimiter(MetadataName.MESSAGE_TYPE))) {
metadata.messageType =
header.substring(toHeaderWithDelimiter(MetadataName.MESSAGE_TYPE).length());
@@ -53,18 +62,18 @@
// If the required fields were not yet found, continue to parse the text
if (!Strings.isNullOrEmpty(m.textContent())) {
- String[] lines = m.textContent().split("\n");
- extractFooters(lines, metadata);
+ String[] lines = m.textContent().replace("\r\n", "\n").split("\n");
+ extractFooters(lines, metadata, m);
if (metadata.hasRequiredFields()) {
return metadata;
}
}
// If the required fields were not yet found, continue to parse the HTML
- // HTML footer are contained inside a <p> tag
+ // HTML footer are contained inside a <div> tag
if (!Strings.isNullOrEmpty(m.htmlContent())) {
- String[] lines = m.htmlContent().split("</p>");
- extractFooters(lines, metadata);
+ String[] lines = m.htmlContent().replace("\r\n", "\n").split("</div>");
+ extractFooters(lines, metadata, m);
if (metadata.hasRequiredFields()) {
return metadata;
}
@@ -73,7 +82,7 @@
return metadata;
}
- private static void extractFooters(String[] lines, MailMetadata metadata) {
+ private static void extractFooters(String[] lines, MailMetadata metadata, MailMessage m) {
for (String line : lines) {
if (metadata.changeId == null && line.contains(MetadataName.CHANGE_ID)) {
metadata.changeId = extractFooter(toFooterWithDelimiter(MetadataName.CHANGE_ID), line);
@@ -82,7 +91,11 @@
Ints.tryParse(extractFooter(toFooterWithDelimiter(MetadataName.PATCH_SET), line));
} else if (metadata.timestamp == null && line.contains(MetadataName.TIMESTAMP)) {
String ts = extractFooter(toFooterWithDelimiter(MetadataName.TIMESTAMP), line);
- metadata.timestamp = Timestamp.from(MailUtil.rfcDateformatter.parse(ts, Instant::from));
+ try {
+ metadata.timestamp = Timestamp.from(MailUtil.rfcDateformatter.parse(ts, Instant::from));
+ } catch (DateTimeParseException e) {
+ log.error("Mail: Error while parsing timestamp from footer of message " + m.id(), e);
+ }
} else if (metadata.messageType == null && line.contains(MetadataName.MESSAGE_TYPE)) {
metadata.messageType =
extractFooter(toFooterWithDelimiter(MetadataName.MESSAGE_TYPE), line);
@@ -91,6 +104,6 @@
}
private static String extractFooter(String key, String line) {
- return line.substring(line.indexOf(key) + key.length(), line.length());
+ return line.substring(line.indexOf(key) + key.length(), line.length()).trim();
}
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java
index 2c60b5d..76995d5 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java
@@ -62,11 +62,11 @@
b.subject("");
StringBuilder stringBuilder = new StringBuilder();
- stringBuilder.append(toFooterWithDelimiter(MetadataName.CHANGE_ID) + "cid" + "\n");
- stringBuilder.append(toFooterWithDelimiter(MetadataName.PATCH_SET) + "1" + "\n");
+ stringBuilder.append(toFooterWithDelimiter(MetadataName.CHANGE_ID) + "cid" + "\r\n");
+ stringBuilder.append("> " + toFooterWithDelimiter(MetadataName.PATCH_SET) + "1" + "\n");
stringBuilder.append(toFooterWithDelimiter(MetadataName.MESSAGE_TYPE) + "comment" + "\n");
stringBuilder.append(
- toFooterWithDelimiter(MetadataName.TIMESTAMP) + "Tue, 25 Oct 2016 02:11:35 -0700" + "\n");
+ toFooterWithDelimiter(MetadataName.TIMESTAMP) + "Tue, 25 Oct 2016 02:11:35 -0700" + "\r\n");
b.textContent(stringBuilder.toString());
Address author = new Address("Diffy", "test@gerritcodereview.com");
@@ -91,15 +91,16 @@
b.subject("");
StringBuilder stringBuilder = new StringBuilder();
- stringBuilder.append("<p>" + toFooterWithDelimiter(MetadataName.CHANGE_ID) + "cid" + "</p>");
- stringBuilder.append("<p>" + toFooterWithDelimiter(MetadataName.PATCH_SET) + "1" + "</p>");
stringBuilder.append(
- "<p>" + toFooterWithDelimiter(MetadataName.MESSAGE_TYPE) + "comment" + "</p>");
+ "<div id\"someid\">" + toFooterWithDelimiter(MetadataName.CHANGE_ID) + "cid" + "</div>");
+ stringBuilder.append("<div>" + toFooterWithDelimiter(MetadataName.PATCH_SET) + "1" + "</div>");
stringBuilder.append(
- "<p>"
+ "<div>" + toFooterWithDelimiter(MetadataName.MESSAGE_TYPE) + "comment" + "</div>");
+ stringBuilder.append(
+ "<div>"
+ toFooterWithDelimiter(MetadataName.TIMESTAMP)
+ "Tue, 25 Oct 2016 02:11:35 -0700"
- + "</p>");
+ + "</div>");
b.htmlContent(stringBuilder.toString());
Address author = new Address("Diffy", "test@gerritcodereview.com");
