Details

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

      Operating System: other
      Platform: Other

      Description

      Add new class Charset the let the whole thing extensible and less error prone

      1. ASF.LICENSE.NOT.GRANTED--Charset.java
        0.7 kB
        Piero Ottuzzi
      2. ASF.LICENSE.NOT.GRANTED--Email.java.patch
        0.6 kB
        Piero Ottuzzi
      3. charset-support.patch
        31 kB
        Ben Speakmon

        Issue Links

          Activity

          Hide
          Piero Ottuzzi added a comment -

          Created an attachment (id=17777)
          Patch for email to add method setCharset(Charset aCharset)

          Show
          Piero Ottuzzi added a comment - Created an attachment (id=17777) Patch for email to add method setCharset(Charset aCharset)
          Hide
          Piero Ottuzzi added a comment -

          Created an attachment (id=17778)
          This is the new Charset class

          Show
          Piero Ottuzzi added a comment - Created an attachment (id=17778) This is the new Charset class
          Hide
          dion gillard added a comment -

          Hmmm. JDK 14 introduced a java.nio.Charset class. Is that usable?

          Show
          dion gillard added a comment - Hmmm. JDK 14 introduced a java.nio.Charset class. Is that usable?
          Hide
          Piero Ottuzzi added a comment -

          (In reply to comment #3)
          > Hmmm. JDK 14 introduced a java.nio.Charset class. Is that usable?

          Yes, Charset would be usable but then we must use JDK 1.4 or above; in
          Email.java in method createInternetAddress() you can read:
          // Using this instead of new InternetAddress(email, name, [charset]) makes
          // commons-email usable with javamail 1.2 / J2EE 1.3

          If we go with javamail1.3/jdk1.4 then we can also fix easily COM-2527.

          Show
          Piero Ottuzzi added a comment - (In reply to comment #3) > Hmmm. JDK 14 introduced a java.nio.Charset class. Is that usable? Yes, Charset would be usable but then we must use JDK 1.4 or above; in Email.java in method createInternetAddress() you can read: // Using this instead of new InternetAddress(email, name, [charset] ) makes // commons-email usable with javamail 1.2 / J2EE 1.3 If we go with javamail1.3/jdk1.4 then we can also fix easily COM-2527 .
          Hide
          Piero Ottuzzi added a comment -

          Maybe we can go directly to commons-email 2.0 breaking API compatibility and
          setting new requirements.

          I think also we should release a 1.0.1 (or 1.1) with fixes committed so far.

          Show
          Piero Ottuzzi added a comment - Maybe we can go directly to commons-email 2.0 breaking API compatibility and setting new requirements. I think also we should release a 1.0.1 (or 1.1) with fixes committed so far.
          Hide
          Ben Speakmon added a comment -

          I've done some work on mail clients and servers with Java in the past – I would strongly recommend moving to JDK 1.4's Charset, as it saves all kinds of headaches like this.

          I'm not sure if the current commons-email API could be preserved, so it may be a breaking change reserved for 2.0, but attempts to handle charsets with pre-1.4 tools is a truly Sisyphean task.

          Releasing a 1.1 with current fixes is a great idea.

          Show
          Ben Speakmon added a comment - I've done some work on mail clients and servers with Java in the past – I would strongly recommend moving to JDK 1.4's Charset, as it saves all kinds of headaches like this. I'm not sure if the current commons-email API could be preserved, so it may be a breaking change reserved for 2.0, but attempts to handle charsets with pre-1.4 tools is a truly Sisyphean task. Releasing a 1.1 with current fixes is a great idea.
          Hide
          Ben Speakmon added a comment -

          Adding link to related issue.

          Show
          Ben Speakmon added a comment - Adding link to related issue.
          Hide
          Ben Speakmon added a comment -

          Attaching a patch and updated test cases for better charset support.

          • adds addTo(), addCc(), addReplyTo(), addBcc() overloads which take parameters (email, name, charset) and creates InternetAddresses that get encoded using the specified charset. This fixes EMAIL-25.
          • All charset names are now passed to java.nio.charset.Charset.forName(), which confirms that the requested charset is available in the current JVM and returns the canonical name for it. Invalid charset names will be detected and the proper exception thrown. This fixes EMAIL-14.
          • Finally, delegating charset handling to the JVM means we don't need a separate Charset class or create a need to maintain some kind of registry of correct names. A couple of comments on the charset issues request specific charsets; now, if the JVM supports it, we don't need to do anything else.
          Show
          Ben Speakmon added a comment - Attaching a patch and updated test cases for better charset support. adds addTo(), addCc(), addReplyTo(), addBcc() overloads which take parameters (email, name, charset) and creates InternetAddresses that get encoded using the specified charset. This fixes EMAIL-25 . All charset names are now passed to java.nio.charset.Charset.forName(), which confirms that the requested charset is available in the current JVM and returns the canonical name for it. Invalid charset names will be detected and the proper exception thrown. This fixes EMAIL-14 . Finally, delegating charset handling to the JVM means we don't need a separate Charset class or create a need to maintain some kind of registry of correct names. A couple of comments on the charset issues request specific charsets; now, if the JVM supports it, we don't need to do anything else.
          Hide
          dion gillard added a comment -

          Applied the charset-support patch. Looks fine.

          Show
          dion gillard added a comment - Applied the charset-support patch. Looks fine.

            People

            • Assignee:
              Unassigned
              Reporter:
              Piero Ottuzzi
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development