Project: javamail
Repository: mercurial
Revision: 873
Author: shannon
Date: 2016-10-14 20:30:45 UTC
Link:
Log Message:
------------
MailHandler should choose a better default subject formatter - Bug K6568
MailHandler body content type should be determined from cumulative output.
MailHandlerTest add test to ensure default subject is CollectorFormatter.
MailHandlerTest add test for content type of nested formatters.
(From Jason)
Revisions:
----------
873
Modified Paths:
---------------
doc/release/CHANGES.txt
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 e284cf35ba77 -r 21e84c3c1f53 doc/release/CHANGES.txt
--- a/doc/release/CHANGES.txt Mon Oct 03 14:22:38 2016 -0700
+++ b/doc/release/CHANGES.txt Fri Oct 14 13:30:45 2016 -0700
@@ -21,6 +21,7 @@
K 6074 MimeMessage.updateHeaders should set the Date header if not already set
K 6281 The UIDFolder interface should have a getter for UIDNEXT
+K 6568 MailHandler should choose a better default subject formatter.
K 6823 Store, Transport, and Folder should implement AutoCloseable
K 6949 MailDateFormat changes for version 1.6
K 7010 Fix javac warnings
diff -r e284cf35ba77 -r 21e84c3c1f53 mail/src/main/java/com/sun/mail/util/logging/MailHandler.java
--- a/mail/src/main/java/com/sun/mail/util/logging/MailHandler.java Mon Oct 03 14:22:38 2016 -0700
+++ b/mail/src/main/java/com/sun/mail/util/logging/MailHandler.java Fri Oct 14 13:30:45 2016 -0700
@@ -125,8 +125,8 @@
* attachment. (default is no attachments)
*
* <li><handler-name>.attachment.names a comma separated
- * list of names or <tt>Formatter</tt> class names of each attachment. The
- * attachment file names must not contain any line breaks.
+ * list of names or <tt>Formatter</tt> class names of each attachment. All
+ * control characters are removed from the attachment names.
* (default is {_at_linkplain java.util.logging.Formatter#toString() toString}
* of the attachment formatter)
*
@@ -216,8 +216,9 @@
*
* <li><handler-name>.subject the name of a
* <tt>Formatter</tt> class or string literal used to create the subject line.
- * The empty string can be used to specify no subject. The subject line must
- * not contain any line breaks. (defaults to the empty string)
+ * The empty string can be used to specify no subject. All control characters
+ * are removed from the subject line. (defaults to {_at_linkplain
+ * com.sun.mail.util.logging.CollectorFormatter CollectorFormatter}.)
*
* <li><handler-name>.pushFilter the name of a
* <tt>Filter</tt> class used to trigger an early push.
@@ -1003,7 +1004,7 @@
if (newFilter != filter) {
clearMatches(-1);
}
- this.filter = newFilter;
+ this.filter = newFilter; //Volatile access.
}
}
@@ -1210,7 +1211,9 @@
/**
* Sets the <tt>Authenticator</tt> used to login to the email server.
- * @param password a password or null if none is required.
+ * @param password a password, empty array can be used to only supply a
+ * user name set by <tt>mail.user</tt> property, or null if no credentials
+ * are required.
* @throws SecurityException if a security manager exists and the
* caller does not have <tt>LoggingPermission("control")</tt>.
* @throws IllegalStateException if called from inside a push.
@@ -1228,6 +1231,9 @@
/**
* A private hook to handle possible future overrides. See public method.
* @param auth see public method.
+ * @throws SecurityException if a security manager exists and the
+ * caller does not have <tt>LoggingPermission("control")</tt>.
+ * @throws IllegalStateException if called from inside a push.
*/
private void setAuthenticator0(final Authenticator auth) {
checkAccess();
@@ -1408,8 +1414,8 @@
}
/**
- * Sets the attachment file name for each attachment. The caller must
- * ensure that the attachment file names do not contain any line breaks.
+ * Sets the attachment file name for each attachment. All control
+ * characters are removed from the attachment names.
* This method will create a set of custom formatters.
* @param names an array of names.
* @throws SecurityException if a security manager exists and the
@@ -1464,10 +1470,10 @@
* typically return an empty string. Instead of being used to format
* records, it is used to gather information about the contents of an
* attachment. The <tt>getTail</tt> method should be used to construct the
- * attachment file name and reset any formatter collected state. The
- * formatter must ensure that the attachment file name does not contain any
- * line breaks. The <tt>toString</tt> method of the given formatter should
- * be overridden to provide a useful attachment file name, if possible.
+ * attachment file name and reset any formatter collected state. All
+ * control characters will be removed from the output of the formatter. The
+ * <tt>toString</tt> method of the given formatter should be overridden to
+ * provide a useful attachment file name, if possible.
* @param formatters and array of attachment name formatters.
* @throws SecurityException if a security manager exists and the
* caller does not have <tt>LoggingPermission("control")</tt>.
@@ -1517,8 +1523,8 @@
}
/**
- * Sets a literal string for the email subject. The caller must ensure that
- * the subject line does not contain any line breaks.
+ * Sets a literal string for the email subject. All control characters are
+ * removed from the subject line.
* @param subject a non <tt>null</tt> string.
* @throws SecurityException if a security manager exists and the
* caller does not have <tt>LoggingPermission("control")</tt>.
@@ -1543,8 +1549,8 @@
* empty string. This formatter is used to gather information to create a
* summary about what information is contained in the email. The
* <tt>getTail</tt> method should be used to construct the subject and reset
- * any formatter collected state. The formatter must ensure that the
- * subject line does not contain any line breaks. The <tt>toString</tt>
+ * any formatter collected state. All control characters
+ * will be removed from the formatter output. The <tt>toString</tt>
* method of the given formatter should be overridden to provide a useful
* subject, if possible.
* @param format the subject formatter.
@@ -1610,20 +1616,19 @@
* this type of conversion. Currently, this is only used for the body
* since the attachments are computed by filename.
* Package-private for unit testing.
- * @param head any head string.
+ * @param chunk any char sequence or null.
* @return return the mime type or null for text/plain.
*/
- final String contentTypeOf(String head) {
- if (!isEmpty(head)) {
+ final String contentTypeOf(CharSequence chunk) {
+ if (!isEmpty(chunk)) {
final int MAX_CHARS = 25;
- if (head.length() > MAX_CHARS) {
- head = head.substring(0, MAX_CHARS);
+ if (chunk.length() > MAX_CHARS) {
+ chunk = chunk.subSequence(0, MAX_CHARS);
}
try {
final String charset = getEncodingName();
- final ByteArrayInputStream in
- = new ByteArrayInputStream(head.getBytes(charset));
-
+ final byte[] b = chunk.toString().getBytes(charset);
+ final ByteArrayInputStream in = new ByteArrayInputStream(b);
assert in.markSupported() : in.getClass().getName();
return URLConnection.guessContentTypeFromStream(in);
} catch (final IOException IOE) {
@@ -1641,14 +1646,26 @@
* Package-private for unit testing.
*
* @param f the formatter or null.
- * @return return the mime type or text/plain.
+ * @return return the mime type or null, meaning text/plain.
* @since JavaMail 1.5.6
*/
final String contentTypeOf(final Formatter f) {
+ assert Thread.holdsLock(this);
if (f != null) {
+ String type = getContentType(f.getClass().getName());
+ if (type != null) {
+ return type;
+ }
+
for (Class<?> k = f.getClass(); k != Formatter.class;
k = k.getSuperclass()) {
- String name = k.getName().toLowerCase(Locale.ENGLISH);
+ String name;
+ try {
+ name = k.getSimpleName();
+ } catch (final InternalError JDK8057919) {
+ name = k.getName();
+ }
+ name = name.toLowerCase(Locale.ENGLISH);
for (int idx = name.indexOf('$') + 1;
(idx = name.indexOf("ml", idx)) > -1; idx += 2) {
if (idx > 0) {
@@ -1663,7 +1680,7 @@
}
}
}
- return "text/plain";
+ return null;
}
/**
@@ -1790,7 +1807,7 @@
* Set the content for a part using the encoding assigned to the handler.
* @param part the part to assign.
* @param buf the formatted data.
- * @param type the mime type.
+ * @param type the mime type or null, meaning text/plain.
* @throws MessagingException if there is a problem.
*/
private void setContent(MimeBodyPart part, CharSequence buf, String type) throws MessagingException {
@@ -2216,11 +2233,11 @@
}
/**
- * Checks a string value for null or empty.
- * @param s the string.
+ * Checks a char sequence value for null or empty.
+ * @param s the char sequence.
* @return true if the given string is null or zero length.
*/
- private static boolean isEmpty(final String s) {
+ private static boolean isEmpty(final CharSequence s) {
return s == null || s.length() == 0;
}
@@ -2641,6 +2658,10 @@
private void initSubject(final String p) {
assert Thread.holdsLock(this);
String name = fromLogManager(p.concat(".subject"));
+ if (name == null) { //Soft dependency on CollectorFormatter.
+ name = "com.sun.mail.util.logging.CollectorFormatter";
+ }
+
if (hasValue(name)) {
try {
this.subjectFormatter = LogManagerProperties.newFormatter(name);
@@ -2654,14 +2675,8 @@
this.subjectFormatter = TailNameFormatter.of(name);
reportError(E.getMessage(), E, ErrorManager.OPEN_FAILURE);
}
- } else {
- if (name != null) {
- this.subjectFormatter = TailNameFormatter.of(name);
- }
- }
-
- if (this.subjectFormatter == null) { //Ensure not null.
- this.subjectFormatter = TailNameFormatter.of("");
+ } else { //User has forced empty or literal null.
+ this.subjectFormatter = TailNameFormatter.of(name);
}
}
@@ -2847,7 +2862,6 @@
*/
StringBuilder[] buffers = new StringBuilder[parts.length];
- String contentType = null;
StringBuilder buf = null;
appendSubject(msg, head(subjectFormatter));
@@ -2871,9 +2885,7 @@
lmf = bodyFilter;
if (buf == null) {
buf = new StringBuilder();
- final String head = head(bodyFormat);
- buf.append(head);
- contentType = contentTypeOf(head);
+ buf.append(head(bodyFormat));
}
formatted = true;
buf.append(format(bodyFormat, r));
@@ -2948,7 +2960,8 @@
appendSubject(msg, tail(subjectFormatter, ""));
MimeMultipart multipart = new MimeMultipart();
- String altType = getContentType(bodyFormat.getClass().getName());
+ String contentType = contentTypeOf(buf);
+ String altType = contentTypeOf(bodyFormat);
setContent(body, buf, altType == null ? contentType : altType);
multipart.addBodyPart(body);
@@ -3207,7 +3220,7 @@
for (int i = 0; i < atn.length; ++i) {
ambp[i] = createBodyPart(i);
ambp[i].setFileName(atn[i]);
- //Convert names to mime type.
+ //Convert names to mime type under lock.
atn[i] = getContentType(atn[i]);
}
}
diff -r e284cf35ba77 -r 21e84c3c1f53 mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java
--- a/mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java Mon Oct 03 14:22:38 2016 -0700
+++ b/mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java Fri Oct 14 13:30:45 2016 -0700
@@ -2488,11 +2488,13 @@
InternalErrorManager em = new InternalErrorManager();
instance.setErrorManager(em);
- assertEquals("text/plain", instance.contentTypeOf(new SimpleFormatter()));
- assertEquals("text/plain", instance.contentTypeOf(new SimpleFormatter(){}));
-
- assertEquals("application/xml", instance.contentTypeOf(new XMLFormatter()));
- assertEquals("application/xml", instance.contentTypeOf(new XMLFormatter(){}));
+ synchronized (instance) {
+ assertNull(instance.contentTypeOf(new SimpleFormatter()));
+ assertNull(instance.contentTypeOf(new SimpleFormatter(){}));
+
+ assertEquals("application/xml", instance.contentTypeOf(new XMLFormatter()));
+ assertEquals("application/xml", instance.contentTypeOf(new XMLFormatter(){}));
+ }
/**
* None of the Formatter methods that can generate content should be
@@ -2520,8 +2522,11 @@
throw new UnsupportedOperationException();
}
}
- assertEquals("text/html", instance.contentTypeOf(new UnsupportedHTML()));
- assertEquals("text/html", instance.contentTypeOf(new UnsupportedHTML(){}));
+
+ synchronized (instance) {
+ assertEquals("text/html", instance.contentTypeOf(new UnsupportedHTML()));
+ assertEquals("text/html", instance.contentTypeOf(new UnsupportedHTML(){}));
+ }
instance.close();
for (Exception exception : em.exceptions) {
@@ -2559,9 +2564,23 @@
}
@Test
+ public void testContentTypeNestedFormatter() throws Exception {
+ String expected = "application/xml; charset=us-ascii";
+ String type = getInlineContentType(new CollectorFormatter("{0}{1}{2}",
+ new XMLFormatter(), new SeverityComparator()));
+ assertEquals(expected, type);
+
+
+ expected = "text/plain; charset=us-ascii";
+ type = getInlineContentType(new CollectorFormatter("{0}{1}{2}",
+ new CompactFormatter(), new SeverityComparator()));
+ assertEquals(expected, type);
+ }
+
+ @Test
public void testContentTypeOverride() throws Exception {
String expected = "application/xml; charset=us-ascii";
- String type = getInlineContentType();
+ String type = getInlineContentType(new XMLFormatter());
assertEquals(expected, type);
MimetypesFileTypeMap m = new MimetypesFileTypeMap();
@@ -2569,17 +2588,17 @@
final FileTypeMap old = FileTypeMap.getDefaultFileTypeMap();
FileTypeMap.setDefaultFileTypeMap(m);
try {
- type = getInlineContentType();
+ type = getInlineContentType(new XMLFormatter());
assertEquals("text/plain; charset=us-ascii", type);
} finally {
FileTypeMap.setDefaultFileTypeMap(old);
}
- type = getInlineContentType();
+ type = getInlineContentType(new XMLFormatter());
assertEquals(expected, type);
}
- private String getInlineContentType() throws Exception {
+ private String getInlineContentType(Formatter f) throws Exception {
final String[] value = new String[1];
MailHandler instance = new MailHandler(createInitProperties(""));
instance.setEncoding("us-ascii");
@@ -2602,7 +2621,7 @@
Properties props = createInitProperties("");
props.put("mail.to", "localhost_at_localdomain");
instance.setMailProperties(props);
- instance.setFormatter(new XMLFormatter());
+ instance.setFormatter(f);
instance.publish(new LogRecord(Level.SEVERE, "test"));
instance.close();
@@ -3975,6 +3994,7 @@
instance.setErrorManager(em);
assertNotNull(instance.getSubject());
+ assertEquals(CollectorFormatter.class, instance.getSubject().getClass());
try {
instance.setSubject((Formatter) null);
@@ -5704,7 +5724,7 @@
props.put("subject", "test");
props.put("mail.from", "badAddress");
props.put("verify", "local");
-
+
instance = new MailHandler(props);
try {
InternalErrorManager em = internalErrorManagerFrom(instance);