Uploaded image for project: 'James Server'
  1. James Server
  2. JAMES-570

James insert a Return-Path: null in outgoing email

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3.0
    • Fix Version/s: 2.3.0
    • Component/s: None
    • Labels:
      None

      Description

      At the moment james insert a Return-Path: null to outgoing emails. That is not correct. On outgoing emails no Return-Path should be insert.

      From RFC (http://www.faqs.org/rfcs/rfc821.html):
      When the receiver-SMTP makes the "final delivery" of a
      message it inserts at the beginning of the mail data a
      return path line. The return path line preserves the
      information in the <reverse-path> from the MAIL command.
      Here, final delivery means the message leaves the SMTP
      world. Normally, this would mean it has been delivered to
      the destination user, but in some cases it may be further
      processed and transmitted by another mail system.

      We have also to get sure what todo if there is allready one ( should never happen). Should we keep the header or strip it.

        Issue Links

          Activity

          Hide
          hilmer@apache.org Soren Hilmer added a comment -

          The current SMTP specification is in (http://www.faqs.org/rfcs/rfc2821.html).

          It has a lot more detail on this subject in section 4.4

          Show
          hilmer@apache.org Soren Hilmer added a comment - The current SMTP specification is in ( http://www.faqs.org/rfcs/rfc2821.html ). It has a lot more detail on this subject in section 4.4
          Hide
          norman Norman Maurer added a comment -

          Its also importent to know that some mailserver strip the return path if there is allready one and some not to reproduce this bug. For example postfix and sendmail seems to strip it. Qmail not.

          Show
          norman Norman Maurer added a comment - Its also importent to know that some mailserver strip the return path if there is allready one and some not to reproduce this bug. For example postfix and sendmail seems to strip it. Qmail not.
          Hide
          bago Stefano Bagnara added a comment -

          The problem seems to be related to code in MimeMessageWrapped added by Noel on 05/05/2005 (r168238)

          "revision 109187 introduced a bug where if we had a real Return-Path header its value would not be the first in the String[]. This tries to eliminate that problem. We should still test to see if this hack is necessary at all, or if InternetHeaders.setHeader("Return-Path", <value>) is smart enough in JavaMail 1.3.2 to put the header at the top, instead of in the middle."

          I this also the comment to r109187 is usefult:

          Additional adjustments to fix JAMES-264. InternetHeaders is an awkward class to work with due its own internal handling of header order. This change takes advantage of how InternetHeaders works, pre-placing a null placeholder for a possible Return-Path header so that when setHeader is called, it will use that placeholder, and remove any other Return-Path headers. It also ensures that there are no other placeholders, providing a bit more control as we manipulate the Delivered-To headers (and possibly other headers that JavaMail may want to change internally, such as Message-ID, Date and Mime-Version).

          InternetHeaders currently works differently if created with empty constructor or from stream. In the first case it create an empty sorted list of headers to keep positions, otherwise it does it the wrong way.

          Changing the MailHeaders(InputStream) constructor to use the empty super constructor and calling load(is) seems to fix this difference.

          Norman is currently working on writing unit testing and to find a solution to this issues.

          Show
          bago Stefano Bagnara added a comment - The problem seems to be related to code in MimeMessageWrapped added by Noel on 05/05/2005 (r168238) "revision 109187 introduced a bug where if we had a real Return-Path header its value would not be the first in the String[]. This tries to eliminate that problem. We should still test to see if this hack is necessary at all, or if InternetHeaders.setHeader("Return-Path", <value>) is smart enough in JavaMail 1.3.2 to put the header at the top, instead of in the middle." I this also the comment to r109187 is usefult: Additional adjustments to fix JAMES-264 . InternetHeaders is an awkward class to work with due its own internal handling of header order. This change takes advantage of how InternetHeaders works, pre-placing a null placeholder for a possible Return-Path header so that when setHeader is called, it will use that placeholder, and remove any other Return-Path headers. It also ensures that there are no other placeholders, providing a bit more control as we manipulate the Delivered-To headers (and possibly other headers that JavaMail may want to change internally, such as Message-ID, Date and Mime-Version). InternetHeaders currently works differently if created with empty constructor or from stream. In the first case it create an empty sorted list of headers to keep positions, otherwise it does it the wrong way. Changing the MailHeaders(InputStream) constructor to use the empty super constructor and calling load(is) seems to fix this difference. Norman is currently working on writing unit testing and to find a solution to this issues.
          Hide
          bago Stefano Bagnara added a comment -

          This bug has been introduced by the fix to JAMES-264

          Show
          bago Stefano Bagnara added a comment - This bug has been introduced by the fix to JAMES-264
          Hide
          bago Stefano Bagnara added a comment -

          InternetHeaders have this bugs:
          1) if you load an header from stream then following addHeader will not follow "default" sorting.
          2) if you add a Return-Path to headers loaded from a stream with a Return-Path in the bad place it will add the new Return-Path just before the old one, and not at the start of the message
          3) if you replace a Return-Path to headers loaded from a stream with a Return-Path in the bad place it will put the new Return-Path in the same position of the old one.

          So we changed MailHeaders to fix this issues:
          1) Changed the default constructor to use the right one from the super class and then load the stream
          2) Changed the setHeader and addHeader to correctly handle the Return-Path header

          Then we removed all the custom routine introduced by Noel to fix the Return-Path issues that had the problem described by this issue and was no more needed with this update.

          This fix also allowed to remove workaround code from MimeMessageTest (it worked around the Return-Path: null). I probably have been too lazy inserting this workaround instead of trying to understand why it was there when testing.

          Btw all the tests now passes, feel free to test it and backport to 2.3

          Show
          bago Stefano Bagnara added a comment - InternetHeaders have this bugs: 1) if you load an header from stream then following addHeader will not follow "default" sorting. 2) if you add a Return-Path to headers loaded from a stream with a Return-Path in the bad place it will add the new Return-Path just before the old one, and not at the start of the message 3) if you replace a Return-Path to headers loaded from a stream with a Return-Path in the bad place it will put the new Return-Path in the same position of the old one. So we changed MailHeaders to fix this issues: 1) Changed the default constructor to use the right one from the super class and then load the stream 2) Changed the setHeader and addHeader to correctly handle the Return-Path header Then we removed all the custom routine introduced by Noel to fix the Return-Path issues that had the problem described by this issue and was no more needed with this update. This fix also allowed to remove workaround code from MimeMessageTest (it worked around the Return-Path: null). I probably have been too lazy inserting this workaround instead of trying to understand why it was there when testing. Btw all the tests now passes, feel free to test it and backport to 2.3
          Hide
          norman Norman Maurer added a comment -

          Backported to 2.3

          Show
          norman Norman Maurer added a comment - Backported to 2.3
          Hide
          danny@apache.org Danny Angus added a comment -

          Closing issue fixed in released version.

          Show
          danny@apache.org Danny Angus added a comment - Closing issue fixed in released version.

            People

            • Assignee:
              norman Norman Maurer
              Reporter:
              norman Norman Maurer
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development