Ping.

On Sun, Oct 09, 2022 at 08:01:01AM +0200, Martijn van Duren wrote:
> Bit focused on other things atm and not familiar with this part of the
> code, but some comments.
> 
> On Sat, 2022-10-08 at 12:18 -0600, Chris Waddey wrote:
>> A message with a single successful recipient but with a failed
>> rcpt to: command afterward generates an incorrect Received: header.
...
>> The following patch fixes the problem:
>> Index: smtp_session.c
>> ===================================================================
>> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
>> retrieving revision 1.432
>> diff -u -p -r1.432 smtp_session.c
>> --- smtp_session.c      1 Jul 2021 07:42:16 -0000       1.432
>> +++ smtp_session.c      8 Oct 2022 18:04:51 -0000
>> @@ -2732,6 +2732,7 @@ static void
>> smtp_message_begin(struct smtp_tx *tx)
>> {
>>        struct smtp_session *s;
>> +       struct smtp_rcpt *srfp;
>>        int     (*m_printf)(struct smtp_tx *, const char *, ...);
>>        m_printf = smtp_message_printf;
>> @@ -2780,10 +2781,13 @@ smtp_message_begin(struct smtp_tx *tx)
>>                }
>>        }
>> +       /* If we get failed "RCPT TO" commands that modify tx->evp, then
>> +        * make sure we use the real recipient for the "for" clause */
> See style(9) how comments should be formatted:
>           /*
>            * Multi-line comments look like this.  Make them real sentences.
>            * Fill them so they look like real paragraphs.
>            */
Got it.
>>        if (tx->rcptcount == 1) {
>> +               srfp = TAILQ_FIRST(&tx->rcpts);
>>                m_printf(tx, "\n\tfor <%s@%s>",
>> -                   tx->evp.rcpt.user,
>> -                   tx->evp.rcpt.domain);
>> +                   srfp->maddr.user,
>> +                   srfp->maddr.domain);
> I don't see anything wrong with this, but does the code still do the correct
> thing with multiple recipients?
> RCPT TO: <exists1>
> RCPT TO: <exists2>
> RCPT TO: <nonexistent>
> or
> RCPT TO: <exists1>
> RCPT TO: <nonexistent>
> RCPT TO: <exists2>

For reference, testing the two above scenarios with the original gives
the following Received: header (except date and message id
differences):

Received: by thief.private (OpenSMTPD) with ESMTP id 4f5144f3; Sun, 9 Oct 2022
       12:09:22 -0600 (MDT)

With the patch it still gives the following for both scenarios:

Received: by thief.private (OpenSMTPD) with ESMTP id 50bf7075; Sun, 9 Oct 2022
       12:14:32 -0600 (MDT)

Modified patch with comment fix (feel free to remove the comment
entirely if you think it's not needed):

Index: smtp_session.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.432
diff -u -p -r1.432 smtp_session.c
--- smtp_session.c    1 Jul 2021 07:42:16 -0000    1.432
+++ smtp_session.c    9 Oct 2022 18:15:53 -0000
@@ -2732,6 +2732,7 @@ static void
smtp_message_begin(struct smtp_tx *tx)
{
   struct smtp_session *s;
+    struct smtp_rcpt *srfp;
   int    (*m_printf)(struct smtp_tx *, const char *, ...);

   m_printf = smtp_message_printf;
@@ -2780,10 +2781,15 @@ smtp_message_begin(struct smtp_tx *tx)
       }
   }

+    /*
+     * If we get failed "RCPT TO" commands that modify tx->evp, then
+     * make sure we use the real recipient for the "for" clause
+     */
   if (tx->rcptcount == 1) {
+        srfp = TAILQ_FIRST(&tx->rcpts);
       m_printf(tx, "\n\tfor <%s@%s>",
-            tx->evp.rcpt.user,
-            tx->evp.rcpt.domain);
+            srfp->maddr.user,
+            srfp->maddr.domain);
   }

   m_printf(tx, ";\n\t%s\n", time_to_text(time(&tx->time)));

Patch with no comment:

Index: smtp_session.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.432
diff -u -p -r1.432 smtp_session.c
--- smtp_session.c    1 Jul 2021 07:42:16 -0000    1.432
+++ smtp_session.c    9 Oct 2022 18:18:44 -0000
@@ -2732,6 +2732,7 @@ static void
smtp_message_begin(struct smtp_tx *tx)
{
   struct smtp_session *s;
+    struct smtp_rcpt *srfp;
   int    (*m_printf)(struct smtp_tx *, const char *, ...);

   m_printf = smtp_message_printf;
@@ -2781,9 +2782,10 @@ smtp_message_begin(struct smtp_tx *tx)
   }

   if (tx->rcptcount == 1) {
+        srfp = TAILQ_FIRST(&tx->rcpts);
       m_printf(tx, "\n\tfor <%s@%s>",
-            tx->evp.rcpt.user,
-            tx->evp.rcpt.domain);
+            srfp->maddr.user,
+            srfp->maddr.domain);
   }

   m_printf(tx, ";\n\t%s\n", time_to_text(time(&tx->time)));

Reply via email to