Bug in reply-by-email HTML parsing: Gmail's gmail_attr quote line leaves an orphaned <> in posted comments

[Danny]

Hi all,

I ran into a small but reproducible bug in Gerrit's reply-by-email HTML parsing
(java/com/google/gerrit/mail/HtmlParser.java) while setting up reply-by-email against a
Google Workspace mailbox on our 3.14.2 instance. I don't have Buganizer filing access, so posting here first per the guidance on the Gerrit support page -- happy to have someone file the actual tracker issue, or file it myself if pointed at the right process.

Summary:

Replying to a Gerrit notification email from Gmail produces a comment that retains a
trailing artifact line from the reply's quote-attribution header, with the quoted sender's
email address rendered as an empty <>. The actual quoted notification body beneath it is
correctly stripped -- only the attribution line itself survives.

Steps to reproduce:

1. Enable reply-by-email ([receiveEmail] in gerrit.config).
2. From Gmail, reply to a Gerrit change notification with a new comment, e.g. "Testing
   adding a comment by replying to Code Review email."
3. Let Gerrit poll and process the reply.
4. View the resulting comment on the change.

Expected: the comment contains only the new reply text.

Actual:

Testing adding a comment by replying to Code Review email.

On Wed, Sep 9, 2026 at 1:55 PM Jane Doe (Code Review) <> wrote:

Relevant HTML from the actual Gmail-composed reply (MIME-decoded):

<div dir="ltr">Testing adding a comment by replying to Code Review email.</div><br>
<div class="gmail_quote gmail_quote_container">
  <div dir="ltr" class="gmail_attr">On Wed, Sep 9, 2026 at 1:55 PM Jane Doe (Code Review) &lt;<a href="mailto:jane...@example.com">jane...@example.com</a>&gt; wrote:<br></div>
  <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
    ...quoted notification body...
  </blockquote>
</div>

Root cause:

HtmlParser.MAIL_PROVIDER_EXTRAS recognizes gmail_extra/gmail_quote as quote-wrapper
classes, but not gmail_attr -- the class Gmail actually uses for this attribution line.
Even setting that aside, two independent bugs in the matching logic mean simply adding
gmail_attr to the set wouldn't be sufficient on its own:

1. isInBlockQuote only checks an element's ancestors (e.parents()), never the element
   itself:

     boolean isInBlockQuote =
         e.parents().stream()
             .anyMatch(p -> p.tagName().equals("blockquote") || MAIL_PROVIDER_EXTRAS.contains(p.className()));

   The attribution text is extracted via e.ownText() when e is the
   <div class="gmail_attr"> element itself -- its own class is never inspected, only its
   ancestors' are.

2. MAIL_PROVIDER_EXTRAS.contains(p.className()) does exact whole-string matching against
   Element.className(), which returns the raw, untokenized class attribute. Real Gmail
   markup emits compound classes (class="gmail_quote gmail_quote_container"), so even the
   already-recognized gmail_quote class fails to match here -- it happens not to matter for
   the quoted body in this repro only because that content is also inside a real
   <blockquote> tag, matched independently via tagName().

Separately, HtmlParser explicitly skips <a href="mailto:..."> elements (there's a comment
anticipating exactly this scenario: // (User<us...@gmail.com> wrote: ...)), which is why the
address itself is dropped while the surrounding </> literal characters -- direct text
siblings of that link, captured via the parent div's ownText() -- survive.

Suggested fix:

--- a/java/com/google/gerrit/mail/HtmlParser.java
+++ b/java/com/google/gerrit/mail/HtmlParser.java
@@ -32,7 +32,8 @@
   private static final ImmutableSet<String> MAIL_PROVIDER_EXTRAS =
       ImmutableSet.of(
           "gmail_extra", // "On 01/01/2017 User<us...@gmail.com> wrote:"
-          "gmail_quote" // Used for quoting original content
+          "gmail_quote", // Used for quoting original content
+          "gmail_attr" // "On <date>, <name> <<email>> wrote:" attribution line
           );

   private static final ImmutableSet<String> ALLOWED_HTML_TAGS =
@@ -44,6 +45,11 @@

   private HtmlParser() {}

+  private static boolean hasProviderExtraClass(Element e) {
+    return MAIL_PROVIDER_EXTRAS.stream().anyMatch(e::hasClass);
+  }
+
   /**
    * Parses comments from html email.
    *
@@ -78,11 +84,9 @@
     for (Element e : d.body().getAllElements()) {
       String elementName = e.tagName();
       boolean isInBlockQuote =
-          e.parents().stream()
-              .anyMatch(
-                  p ->
-                      p.tagName().equals("blockquote")
-                          || MAIL_PROVIDER_EXTRAS.contains(p.className()));
+          hasProviderExtraClass(e)
+              || e.parents().stream()
+                  .anyMatch(p -> p.tagName().equals("blockquote") || hasProviderExtraClass(p));

       if (elementName.equals("a")) {
         String href = e.attr("href");

Verified working against the real reproduction above -- I patched it directly into a
running 3.14.2 instance (compiled-class swap, since a full source checkout wasn't available
in that environment) and confirmed the artifact line no longer appears. Happy to send this
as an actual CL if that's preferred over a patch snippet here -- just let me know the right
way to get it in front of someone who can review it.

Impact: cosmetic only -- the quoted notification body itself is correctly stripped in every
case observed; this is a trailing artifact line, not a functional or security issue.

Thanks,
Danny

[Nasser Grainawi]

On Wed, Sep 9, 2026 at 4:38 PM 'Danny' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

Hi all,

I ran into a small but reproducible bug in Gerrit's reply-by-email HTML parsing
(java/com/google/gerrit/mail/HtmlParser.java) while setting up reply-by-email against a
Google Workspace mailbox on our 3.14.2 instance. I don't have Buganizer filing access, so posting here first per the guidance on the Gerrit support page -- happy to have someone file the actual tracker issue, or file it myself if pointed at the right process.

Sorry, looks like the docs never got the update that you need to join the repo-discuss Google Group to get permission to create issues. I created a change to fix that: Note spam prevention requirements (628482) · Gerrit Code Review

Thanks for reporting this, but we can't accept patches over email. You can follow the steps in SUBMITTING_PATCHES to create a code change for review. You can reference the issue you create with `Bug: Issue <your issue number>` in your commit message footers.

If you don't want to get emails for everything posted to the group, you can set your Subscription to "No email".

Thanks,

Nasser

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/repo-discuss/48a32c40-d537-42c1-adf0-9ab85c136fa4n%40googlegroups.com.

[Danny]

Thank you Nasser.

I have was able to create https://issues.gerritcodereview.com/issues/559249628 (and I also submitted a patch for it).