commits@javamail.java.net

[javamail~mercurial:375] Updates from Jason:

From: <shannon_at_kenai.com>
Date: Wed, 13 Jul 2011 22:45:25 +0000

Project: javamail
Repository: mercurial
Revision: 375
Author: shannon
Date: 2011-07-11 23:30:21 UTC
Link:

Log Message:
------------
Updates from Jason:

MailHandler X-Mailer header should be encoded and folded to handle subclass name.
MailHandler reportError when published LogRecord was filtered from all mime parts.
MailHandler cache empty string only when verify is not specified.
MailHandler method docs for setPushLevel and setPushFilter should match class docs.
MailHandler class docs for subject should also warn of line breaks.
MailHandler class docs for attachment names should also warn of line breaks.
MailHandlerTest added tests for published LogRecord filtered from all mime parts.


Revisions:
----------
375


Modified Paths:
---------------
mail/src/main/java/com/sun/mail/util/logging/MailHandler.java
mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java


Diffs:
------
diff -r 9d754c6b40ff -r 45770b43aa92 mail/src/main/java/com/sun/mail/util/logging/MailHandler.java
--- a/mail/src/main/java/com/sun/mail/util/logging/MailHandler.java Mon Jun 20 17:29:23 2011 -0700
+++ b/mail/src/main/java/com/sun/mail/util/logging/MailHandler.java Mon Jul 11 16:30:21 2011 -0700
@@ -107,8 +107,9 @@
  * attachment. (default is no attachments)
  *
  * <li>com.sun.mail.util.logging.MailHandler.attachment.names a comma separated
- * list of names or <tt>Formatter</tt> class names of each attachment.
- * (default is no attachments names)
+ * list of names or <tt>Formatter</tt> class names of each attachment. The
+ * attachment file names must not contain any line breaks.
+ * (default is no attachments names)
  *
  * <li>com.sun.mail.util.logging.MailHandler.authenticator name of a
  * {_at_linkplain javax.mail.Authenticator} class used to provide login credentials
@@ -190,7 +191,7 @@
  *
  * <li>com.sun.mail.util.logging.MailHandler.subject the name of a
  * <tt>Formatter</tt> class or string literal used to create the subject line.
- * (defaults to empty string)
+ * The subject line must not contain any line breaks. (defaults to empty string)
  *
  * <li>com.sun.mail.util.logging.MailHandler.verify <a name="verify">used</a> to
  * verify all of the <tt>Handler</tt> properties prior to a push. If set to a
@@ -510,19 +511,31 @@
      * Flushes the buffer into a MessageContext and then tries to store
      * the given record. This method will only be called in cases of
      * reentrant filters, formatters, or comparators. The push filter is
- * the only filter allowed to be reentrant. This is because it is never
- * used during formatting and only called after a record is stored.
+ * the only filter allowed to be reentrant. This is because the push filter
+ * is never used during formatting and only called after a record is stored.
      * @param record the log record still needed to publish.
- * @return the MessageContext or null.
+ * @return the MessageContext containing the previous push or null.
      * @since JavaMail 1.4.5
      */
     private MessageContext flushAndRePublish(final LogRecord record) {
         MessageContext ctx = writeLogRecords(ErrorManager.WRITE_FAILURE);
- if (ctx != null) {
+ if (ctx != null) { //try again with an empty buffer.
+ /**
+ * The common case should be a simple store, no push.
+ * Rarely should we be pushing while holding a lock.
+ */
             publish0(record);
- } else { //Use ISE because this is a push inside of a push.
- reportError("Unable to drain buffer.",
- new IllegalStateException(),
+ } else {
+ /**
+ * This record is lost, report this to the error manager.
+ * Use ISE because this is a push inside of a push.
+ */
+ final SimpleFormatter f = new SimpleFormatter();
+ final String msg = "Log record " + record.getSequenceNumber()
+ + " was not published. "
+ + head(f) + format(f, record) + tail(f, "");
+ reportError(msg,
+ new IllegalStateException("Unable to drain buffer."),
                     ErrorManager.WRITE_FAILURE);
         }
         return ctx;
@@ -621,7 +634,7 @@
      * Sets the push level. This level is used to trigger a push so that
      * all pending records are formatted and sent to the email server. When
      * the push level triggers a send, the resulting email is flagged as
- * high priority.
+ * high importance with urgent priority.
      * @param level Level object.
      * @throws NullPointerException if <tt>level</tt> is <tt>null</tt>.
      * @throws SecurityException if a security manager exists and the
@@ -653,7 +666,7 @@
      * <tt>LogRecord</tt> level was greater than the push level. If this
      * filter returns <tt>true</tt>, all pending records are formatted and sent
      * to the email server. When the push filter triggers a send, the resulting
- * email is flagged as high priority.
+ * email is flagged as high importance with urgent priority.
      * @param filter push filter or <tt>null</tt>
      * @throws SecurityException if a security manager exists and the
      * caller does not have <tt>LoggingPermission("control")</tt>.
@@ -878,7 +891,7 @@
 
     /**
      * Sets the attachment file name for each attachment. The caller must
- * ensure that the attachment file name does not contain any line breaks.
+ * ensure that the attachment file names do not contain any line breaks.
      * This method will create a set of custom formatters.
      * @param names an array of names.
      * @throws SecurityException if a security manager exists and the
@@ -1705,6 +1718,7 @@
             final Filter bodyFilter = getFilter();
 
             for (int ix = 0; ix < size; ++ix) {
+ boolean filtered = true;
                 final LogRecord r = data[ix];
                 data[ix] = null; //clear while formatting.
 
@@ -1717,7 +1731,7 @@
                         buf.append(head);
                         contentType = contentTypeOf(head);
                     }
-
+ filtered = false;
                     buf.append(format(bodyFormat, r));
                 }
 
@@ -1730,10 +1744,15 @@
                             buffers[i].append(head(attachmentFormatters[i]));
                             appendFileName(parts[i], head(attachmentNames[i]));
                         }
+ filtered = false;
                         appendFileName(parts[i], format(attachmentNames[i], r));
                         buffers[i].append(format(attachmentFormatters[i], r));
                     }
                 }
+
+ if (filtered) { //belongs to no mime part.
+ reportFilterError(r);
+ }
             }
             this.size = 0;
 
@@ -1810,7 +1829,9 @@
             value = (String) props.get(key); //search only props.
             if (value == null) {
                 value = props.getProperty(key); //search parent props.
- props.put(key, ""); //disable future checks.
+ if (props.get(key) == null) {
+ props.put(key, ""); //disable future checks.
+ }
             } else {
                 return; //verify complete or in progress.
             }
@@ -2114,6 +2135,24 @@
         }
     }
 
+ /**
+ * Used when a log record was loggable prior to being inserted
+ * into the buffer and at the time of formatting was no longer loggable.
+ * Filters were changed after publish but prior to a push or a bug in the
+ * body filter or one of the attachment filters.
+ * @param record that was not formatted.
+ * @since JavaMail 1.4.5
+ */
+ private void reportFilterError(final LogRecord record) {
+ assert Thread.holdsLock(this);
+ final SimpleFormatter f = new SimpleFormatter();
+ final String msg = "Log record " + record.getSequenceNumber()
+ + " was filtered from all message parts. "
+ + head(f) + format(f, record) + tail(f, "");
+ final String txt = getFilter() + ", " + Arrays.asList(readOnlyAttachmentFilters());
+ reportError(msg, new IllegalArgumentException(txt), ErrorManager.FORMAT_FAILURE);
+ }
+
     private void reportNullError(final int code) {
         reportError("null", new NullPointerException(), code);
     }
@@ -2149,12 +2188,18 @@
         try {
             final Class mail = MailHandler.class;
             final Class k = getClass();
- final String value;
+ String value;
             if (k == mail) {
                 value = mail.getName();
             } else {
- value = mail.getName() + " using the "
- + k.getName() + " extension.";
+ try {
+ value = MimeUtility.encodeText(k.getName());
+ } catch (final UnsupportedEncodingException E) {
+ reportError(E.getMessage(), E, ErrorManager.FORMAT_FAILURE);
+ value = k.getName().replaceAll("[^\\x00-\\x7F]", "\uu001A");
+ }
+ value = MimeUtility.fold(10, mail.getName() + " using the "
+ + value + " extension.");
             }
             msg.setHeader("X-Mailer", value);
         } catch (final MessagingException ME) {

diff -r 9d754c6b40ff -r 45770b43aa92 mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java
--- a/mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java Mon Jun 20 17:29:23 2011 -0700
+++ b/mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java Mon Jul 11 16:30:21 2011 -0700
@@ -46,6 +46,7 @@
 import java.net.ServerSocket;
 import java.net.UnknownHostException;
 import java.io.*;
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
@@ -226,7 +227,7 @@
             instance.setMailProperties(props);
 
             Authenticator auth = new EmptyAuthenticator();
- Filter filter = new BooleanFilter(true);
+ Filter filter = BooleanFilter.TRUE;
             Formatter formatter = new SimpleFormatter();
             instance.setSubject("publishDuringClose");
             Formatter subject = instance.getSubject();
@@ -2009,6 +2010,117 @@
     }
 
     @Test
+ public void testAttachmentFilterSwapBeforePush() {
+ MailHandler instance = new MailHandler(10);
+ instance.setLevel(Level.ALL);
+ instance.setPushLevel(Level.OFF);
+ instance.setPushFilter(null);
+ instance.setFilter(BooleanFilter.FALSE);
+ instance.setAttachmentFormatters(new Formatter[]{new XMLFormatter()});
+ instance.setAttachmentFilters(new Filter[]{null});
+ InternalErrorManager em = new InternalErrorManager();
+ instance.setErrorManager(em);
+
+ LogRecord record = new LogRecord(Level.SEVERE, "lost record");
+ assertTrue(instance.isLoggable(record));
+
+ instance.publish(record);
+ instance.setAttachmentFilters(new Filter[]{BooleanFilter.FALSE});
+ assertFalse(instance.isLoggable(record));
+ instance.close();
+
+ int seenFormat = 0;
+ for (int i = 0; i < em.exceptions.size(); i++) {
+ if (em.exceptions.get(i) instanceof MessagingException) {
+ continue;
+ } else if (em.exceptions.get(i) instanceof RuntimeException
+ && em.exceptions.get(i).getMessage().indexOf(instance.getFilter().toString()) > -1
+ && em.exceptions.get(i).getMessage().indexOf(
+ Arrays.asList(instance.getAttachmentFilters()).toString()) > -1) {
+ seenFormat++;
+ continue; //expected.
+ } else {
+ fail(String.valueOf(em.exceptions.get(i)));
+ }
+ }
+ assertTrue("No format error", seenFormat > 0);
+ }
+
+ @Test
+ public void testFilterSwapBeforePush() {
+ MailHandler instance = new MailHandler(10);
+ instance.setLevel(Level.ALL);
+ instance.setPushLevel(Level.OFF);
+ instance.setPushFilter(null);
+ InternalErrorManager em = new InternalErrorManager();
+ instance.setErrorManager(em);
+
+ LogRecord record = new LogRecord(Level.SEVERE, "lost record");
+ assertTrue(instance.isLoggable(record));
+
+ instance.publish(record);
+ instance.setFilter(BooleanFilter.FALSE);
+ assertFalse(instance.isLoggable(record));
+ instance.close();
+
+ int seenFormat = 0;
+ for (int i = 0; i < em.exceptions.size(); i++) {
+ if (em.exceptions.get(i) instanceof MessagingException) {
+ continue;
+ } else if (em.exceptions.get(i) instanceof RuntimeException
+ && em.exceptions.get(i).getMessage().indexOf(instance.getFilter().toString()) > -1) {
+ seenFormat++;
+ continue; //expected.
+ } else {
+ fail(String.valueOf(em.exceptions.get(i)));
+ }
+ }
+ assertTrue("No format error", seenFormat > 0);
+ }
+
+ @Test
+ public void testFilterFlipFlop() {
+ MailHandler instance = new MailHandler(10);
+ instance.setLevel(Level.ALL);
+ instance.setPushLevel(Level.OFF);
+ instance.setPushFilter(null);
+ FlipFlopFilter badFilter = new FlipFlopFilter();
+ instance.setFilter(badFilter);
+
+ InternalErrorManager em = new InternalErrorManager();
+ instance.setErrorManager(em);
+
+ LogRecord record = new LogRecord(Level.SEVERE, "lost record");
+
+ assertSame(badFilter, instance.getFilter());
+ badFilter.value = true;
+ assertSame(badFilter, instance.getFilter());
+
+ assertTrue(instance.isLoggable(record));
+ instance.publish(record);
+ badFilter.value = false;
+
+ assertSame(badFilter, instance.getFilter());
+ assertFalse(instance.isLoggable(record));
+ instance.close();
+ assertSame(badFilter, instance.getFilter());
+
+ int seenFormat = 0;
+ for (int i = 0; i < em.exceptions.size(); i++) {
+ if (em.exceptions.get(i) instanceof MessagingException) {
+ continue;
+ } else if (em.exceptions.get(i) instanceof RuntimeException
+ && em.exceptions.get(i).getMessage().indexOf(instance.getFilter().toString()) > -1) {
+ seenFormat++;
+ continue; //expected.
+ } else {
+ fail(String.valueOf(em.exceptions.get(i)));
+ }
+ }
+ assertTrue("No format error", seenFormat > 0);
+ }
+
+ @Test
     public void testFilterReentrance() {
         Logger logger = Logger.getLogger("testFilterReentrance");
 
@@ -2653,7 +2765,7 @@
             props.put(p.concat(".encoding"), "us-ascii");
             props.put(p.concat(".mail.host"), "bad-host-name");
             props.put(p.concat(".mail.smtp.host"), "bad-host-name");
- props.put(p.concat(".mail.smtp.port"), Integer.toString(OPEN_PORT)); //bad port.
+ props.put(p.concat(".mail.smtp.port"), Integer.toString(OPEN_PORT));
             props.put(p.concat(".mail.to"), "foo_at_bar.com");
             props.put(p.concat(".mail.cc"), "fizz_at_buzz.com");
             props.put(p.concat(".mail.bcc"), "baz_at_bar.com");
@@ -3061,6 +3173,7 @@
              * This code swallows that error message.
              */
             LogManager.getLogManager().readConfiguration();
+ System.err.print(""); //flushBuffer.
             System.err.flush();
             String result = oldErrors.toString().trim();
             oldErrors.reset();
@@ -3532,6 +3645,15 @@
         }
     }
 
+ public static class FlipFlopFilter implements Filter {
+
+ volatile boolean value;
+
+ public boolean isLoggable(LogRecord record) {
+ return value;
+ }
+ }
+
     public final static class MailHandlerExt extends MailHandler {
 
         public MailHandlerExt() {