git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] t5100: make rfc822 comment test more careful

From
Jeff King <peff@peff.net>
Date
Dec 14, 2023, 21:47 UTC
Message-ID
<20231214214746.GA2798346@coredump.intra.peff.net>
In-Reply-To
<20231214214444.GB2297853@coredump.intra.peff.net>

When processing "From" headers in an email, mailinfo "unquotes" quoted strings and rfc822 parenthesized comments. For quoted strings, we actually remove the double-quotes, so:

  From: "A U Thor" <someone@example.com>
become:
  Author: A U Thor
  Email: someone@example.com
But for comments, we leave the outer parentheses in place, so:
  From: A U (this is a comment) Thor <someone@example.com>
becomes:
  Author: A U (this is a comment) Thor
  Email: someone@example.com

So what is the comment "unquoting" actually doing? In our code, being in a comment section has exactly two effects:

  1. We'll unquote backslash-escaped characters inside a comment
     section.
  2. We _won't_ unquote double-quoted strings inside a comment section.
Our test for comments in t5100 checks this:
  From: "A U Thor" <somebody@example.com> (this is \(really\) a comment (honestly))

So it is covering (1), but not (2). Let's add in a quoted string to cover this.

Moreover, because the comment appears at the end of the From header, there's nothing to confirm that we correctly found the end of the comment section (and not just the end-of-string). Let's instead move it to the beginning of the header, which means we can confirm that the existing quoted string is detected (which will only happen if we know we've left the comment block).

As expected, the test continues to pass, but this will give us more confidence as we refactor the code in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
---
I did this as a separate patch so it is easy to see that the existing
code already passes the test (i.e., our refactor is a true noop in terms
of behavior, and not fixing or breaking anything).
 t/t5100/comment.expect | 2 +-
 t/t5100/comment.in     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t5100/comment.expect b/t/t5100/comment.expect
index 7228177984..bd71956a47 100644
--- a/t/t5100/comment.expect
+++ b/t/t5100/comment.expect
@@ -1,4 +1,4 @@
-Author: A U Thor (this is (really) a comment (honestly))
+Author: (this is (really) a "comment" (honestly)) A U Thor
 Email: somebody@example.com
 Subject: testing comments
 Date: Sun, 25 May 2008 00:38:18 -0700
diff --git a/t/t5100/comment.in b/t/t5100/comment.in
index c53a192dfe..0b7e903b06 100644
--- a/t/t5100/comment.in
+++ b/t/t5100/comment.in
@@ -1,5 +1,5 @@
 From 1234567890123456789012345678901234567890 Mon Sep 17 00:00:00 2001
-From: "A U Thor" <somebody@example.com> (this is \(really\) a comment (honestly))
+From: (this is \(really\) a "comment" (honestly)) "A U Thor" <somebody@example.com>
 Date: Sun, 25 May 2008 00:38:18 -0700
 Subject: [PATCH] testing comments
 
-- 
2.43.0.363.g1d22a8f302
Previous: Jeff KingNext: Jeff King
Message 6 of 10 in “mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair()”
  1. mailinfo: fix out-of-bounds memory reads in unquote_quoted_pair()Jeff King, Dec 12, 2023
  2. Patrick SteinhardtDec 13, 2023
  3. Jeff KingDec 13, 2023
  4. Patrick SteinhardtDec 14, 2023
  5. 0/2 avoiding recursion in mailinfoJeff King, Dec 14, 2023
  6. 1/2 t5100: make rfc822 comment test more carefulJeff King, Dec 14, 2023
  7. 2/2 mailinfo: avoid recursion when unquoting From headersJeff King, Dec 14, 2023
  8. Patrick SteinhardtDec 15, 2023
  9. Junio C HamanoDec 13, 2023
  10. Jeff KingDec 14, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.