Commons Email
  1. Commons Email
  2. EMAIL-79

SimpleEmail#setMsg() with UTF-8 content honors correct charset in header, but doesn't encode the content correctly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1
    • Fix Version/s: 1.2
    • Labels:
      None
    • Environment:

      Windows Vista, JDK 1.6.0_06

      Description

      When I create a new SimpleEmail and use setMsg(some-utf8-text), the Content-Type header of the generated email correctly changes to UTF-8, but the content is not encoded correctly.

      Adding setCharset("UTF-8") solves the problem, but that shouldn't be needed, as the change of Content-Type header indicates.

      1. EmailTest.zip
        1 kB
        Edvin Syse
      2. EMAIL-79-patch-v2.txt
        4 kB
        Corey Scott
      3. EMAIL-79-patch.txt
        3 kB
        Corey Scott

        Activity

        Edvin Syse created issue -
        Hide
        Corey Scott added a comment -

        Edvin,

        I tried looking into this for you and were unable to come up with a test to prove this condition.
        Could you please see if you can create one so I can move this forward for you?

        Thanks

        Show
        Corey Scott added a comment - Edvin, I tried looking into this for you and were unable to come up with a test to prove this condition. Could you please see if you can create one so I can move this forward for you? Thanks
        Hide
        Edvin Syse added a comment -

        Please se attached MailTest.java and message.eml and the resulting email when recieved. You need to put commons-email-1.1.jar and mail-1.4.jar on the classpath to run the test. As you see, the subject is encoded correctly, but the body is qp-encoded with latin-1 charset instead of UTF-8. I ran the program with -Dfile.encoding="UTF-8". It's easy to spot, since the each qp-encoded letter in the body only consist of one encoded one-byte sequence instead of two.

        Show
        Edvin Syse added a comment - Please se attached MailTest.java and message.eml and the resulting email when recieved. You need to put commons-email-1.1.jar and mail-1.4.jar on the classpath to run the test. As you see, the subject is encoded correctly, but the body is qp-encoded with latin-1 charset instead of UTF-8. I ran the program with -Dfile.encoding="UTF-8". It's easy to spot, since the each qp-encoded letter in the body only consist of one encoded one-byte sequence instead of two.
        Edvin Syse made changes -
        Field Original Value New Value
        Attachment EmailTest.zip [ 12394636 ]
        Hide
        Corey Scott added a comment -

        Edvin,

        I am not sure this is what you wanted but the changes contained are making the simple (and HTML) email classes utilize the charset and content type settings better than they were. (in some cases they werent being used previously)

        You still have to do a email.setCharset() call in order to ensure both the subject and message bodies have the encoding set but I personally feel this is acceptable. Perhaps because I dont see how the encoding can be reliably determined dynamically or even if it could, it is in side the scope of this library.

        Show
        Corey Scott added a comment - Edvin, I am not sure this is what you wanted but the changes contained are making the simple (and HTML) email classes utilize the charset and content type settings better than they were. (in some cases they werent being used previously) You still have to do a email.setCharset() call in order to ensure both the subject and message bodies have the encoding set but I personally feel this is acceptable. Perhaps because I dont see how the encoding can be reliably determined dynamically or even if it could, it is in side the scope of this library.
        Corey Scott made changes -
        Attachment EMAIL-79-patch.txt [ 12394818 ]
        Hide
        Edvin Syse added a comment -

        Thanks, hopefully that will do the trick! I agree that one cannot always determine the correct encoding, but I think it is reasonable that the same encoding should be used in both subject and body even if you don't supply something explicitely, don't you agree?

        Show
        Edvin Syse added a comment - Thanks, hopefully that will do the trick! I agree that one cannot always determine the correct encoding, but I think it is reasonable that the same encoding should be used in both subject and body even if you don't supply something explicitely, don't you agree?
        Hide
        Corey Scott added a comment -

        That certainly seems logical.

        Show
        Corey Scott added a comment - That certainly seems logical.
        Siegfried Goeschl made changes -
        Assignee Siegfried Goeschl [ sgoeschl ]
        Siegfried Goeschl made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Siegfried Goeschl added a comment -

        I applied the patch but it is breaking a few tests

        testSendNoAttachments(org.apache.commons.mail.SendWithAttachmentsTest)
        testSendWAttachments(org.apache.commons.mail.SendWithAttachmentsTest)
        testSend(org.apache.commons.mail.HtmlEmailTest)
        testSend2(org.apache.commons.mail.HtmlEmailTest)
        testHtmlMailMimeLayout(org.apache.commons.mail.EmailLiveTest)
        testSend(org.apache.commons.mail.MultiPartEmailTest)

        I try to commit my pending changes so that you can have a look at the problems

        Show
        Siegfried Goeschl added a comment - I applied the patch but it is breaking a few tests testSendNoAttachments(org.apache.commons.mail.SendWithAttachmentsTest) testSendWAttachments(org.apache.commons.mail.SendWithAttachmentsTest) testSend(org.apache.commons.mail.HtmlEmailTest) testSend2(org.apache.commons.mail.HtmlEmailTest) testHtmlMailMimeLayout(org.apache.commons.mail.EmailLiveTest) testSend(org.apache.commons.mail.MultiPartEmailTest) I try to commit my pending changes so that you can have a look at the problems
        Hide
        Corey Scott added a comment -

        New version of the patch that replaces the earlier one.

        Tests against the current trunk are working again.

        Show
        Corey Scott added a comment - New version of the patch that replaces the earlier one. Tests against the current trunk are working again.
        Corey Scott made changes -
        Attachment EMAIL-79-patch-v2.txt [ 12399963 ]
        Siegfried Goeschl made changes -
        Fix Version/s 1.2 [ 12313573 ]
        Hide
        Siegfried Goeschl added a comment -

        Works now - thx

        Show
        Siegfried Goeschl added a comment - Works now - thx
        Siegfried Goeschl made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Siegfried Goeschl made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Siegfried Goeschl
            Reporter:
            Edvin Syse
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development