commits@javamail.java.net

[javamail~mercurial:469] MailHandlerTest only check logging permissions.

From: <shannon_at_kenai.com>
Date: Fri, 21 Sep 2012 21:19:55 +0000

Project: javamail
Repository: mercurial
Revision: 469
Author: shannon
Date: 2012-09-21 20:25:28 UTC
Link:

Log Message:
------------
Remove some redundant checks for null found by FindBugs.
Oops, typo.
Exclude a FindBugs error.
Upgrade to newer version of the compiler plugin, which required more use
of the build-helper-maven-plugin and associated configuration changes.
Add isSSL() method to all protocol providers.
MailHandlerTest only check logging permissions.
MailHandlerTest show all java.security.debug messages.
MailHandlerTest document tests that can not run concurrently.
(From Jason)


Revisions:
----------
464
465
466
467
468
469


Modified Paths:
---------------
mail/src/main/java/com/sun/mail/pop3/Protocol.java
mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java
mail/src/main/java/javax/mail/EventQueue.java
mail/src/main/java/javax/mail/Flags.java
mail/src/main/java/javax/mail/internet/MimeUtility.java
mail/exclude.xml
mail/src/main/java/com/sun/mail/imap/IMAPBodyPart.java
pom.xml
doc/release/CHANGES.txt
mail/src/main/java/com/sun/mail/iap/Protocol.java
mail/src/main/java/com/sun/mail/imap/IMAPStore.java
mail/src/main/java/com/sun/mail/pop3/POP3Store.java
mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java


Diffs:
------
diff -r 00c923474ded -r 842613972f5d mail/src/main/java/com/sun/mail/pop3/Protocol.java
--- a/mail/src/main/java/com/sun/mail/pop3/Protocol.java Tue Aug 14 15:25:48 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/pop3/Protocol.java Mon Aug 20 15:09:11 2012 -0700
@@ -463,7 +463,7 @@
                     StringTokenizer st = new StringTokenizer(r.data);
                     String s = st.nextToken();
                     String octets = st.nextToken();
- if (octets != null && octets.equals("octets")) {
+ if (octets.equals("octets")) {
                         size = Integer.parseInt(s);
                         // don't allow ridiculous sizes
                         if (size > 1024*1024*1024 || size < 0)

diff -r 00c923474ded -r 842613972f5d mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java
--- a/mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java Tue Aug 14 15:25:48 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java Mon Aug 20 15:09:11 2012 -0700
@@ -2221,7 +2221,7 @@
             //out.println("DEBUG SMTP RCVD: " + serverResponse);
 
         // parse out the return code
- if (serverResponse != null && serverResponse.length() >= 3) {
+ if (serverResponse.length() >= 3) {
             try {
                 returnCode = Integer.parseInt(serverResponse.substring(0, 3));
             } catch (NumberFormatException nfe) {

diff -r 00c923474ded -r 842613972f5d mail/src/main/java/javax/mail/EventQueue.java
--- a/mail/src/main/java/javax/mail/EventQueue.java Tue Aug 14 15:25:48 2012 -0700
+++ b/mail/src/main/java/javax/mail/EventQueue.java Mon Aug 20 15:09:11 2012 -0700
@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 1997-2010 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997-2012 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -125,7 +125,8 @@
 
         try {
             loop:
- while ((qe = dequeue()) != null) {
+ for (;;) {
+ qe = dequeue(): // blocks until an item is available
                 MailEvent e = qe.event;
                 Vector v = qe.vector;
 

diff -r 00c923474ded -r 842613972f5d mail/src/main/java/javax/mail/Flags.java
--- a/mail/src/main/java/javax/mail/Flags.java Tue Aug 14 15:25:48 2012 -0700
+++ b/mail/src/main/java/javax/mail/Flags.java Mon Aug 20 15:09:11 2012 -0700
@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 1997-2010 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997-2012 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -450,7 +450,7 @@
         } catch (CloneNotSupportedException cex) {
             // ignore, can't happen
         }
- if (this.user_flags != null && f != null)
+ if (this.user_flags != null)
             f.user_flags = (Hashtable)this.user_flags.clone();
         return f;
     }

diff -r 00c923474ded -r 842613972f5d mail/src/main/java/javax/mail/internet/MimeUtility.java
--- a/mail/src/main/java/javax/mail/internet/MimeUtility.java Tue Aug 14 15:25:48 2012 -0700
+++ b/mail/src/main/java/javax/mail/internet/MimeUtility.java Mon Aug 20 15:09:11 2012 -0700
@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 1997-2011 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997-2012 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -252,7 +252,7 @@
             try {
                 byte[] b = "\r\n".getBytes(charset);
                 bool = Boolean.valueOf(
- b == null || b.length != 2 || b[0] != 015 || b[1] != 012);
+ b.length != 2 || b[0] != 015 || b[1] != 012);
             } catch (UnsupportedEncodingException uex) {
                 bool = Boolean.FALSE; // a guess
             } catch (RuntimeException ex) {


diff -r 842613972f5d -r 40c708f3c2be mail/src/main/java/javax/mail/EventQueue.java
--- a/mail/src/main/java/javax/mail/EventQueue.java Mon Aug 20 15:09:11 2012 -0700
+++ b/mail/src/main/java/javax/mail/EventQueue.java Mon Aug 20 15:33:47 2012 -0700
@@ -126,7 +126,7 @@
         try {
             loop:
             for (;;) {
- qe = dequeue(): // blocks until an item is available
+ qe = dequeue(); // blocks until an item is available
                 MailEvent e = qe.event;
                 Vector v = qe.vector;
 


diff -r 40c708f3c2be -r d50cb5ccdc6e mail/exclude.xml
--- a/mail/exclude.xml Mon Aug 20 15:33:47 2012 -0700
+++ b/mail/exclude.xml Mon Aug 20 15:39:50 2012 -0700
@@ -390,4 +390,17 @@
         <Bug pattern="DM_DEFAULT_ENCODING"/>
     </Match>
 
+ <!--
+ MimeBodyPart.headers is set in the constructor. IMAPBodyPart has
+ perhaps paranoid defensive code to set it if it's not set. That
+ code is protected by the object lock, but all of the reads of the
+ field are not, thus FindBugs complains. For now I think it's
+ better to leave the defensive code in and ignore this error.
+ -->
+ <Match>
+ <Class name="javax.mail.internet.MimeBodyPart"/>
+ <Field name="headers"/>
+ <Bug pattern="IS2_INCONSISTENT_SYNC"/>
+ </Match>
+
 </FindBugsFilter>

diff -r 40c708f3c2be -r d50cb5ccdc6e mail/src/main/java/com/sun/mail/imap/IMAPBodyPart.java
--- a/mail/src/main/java/com/sun/mail/imap/IMAPBodyPart.java Mon Aug 20 15:33:47 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/imap/IMAPBodyPart.java Mon Aug 20 15:39:50 2012 -0700
@@ -376,6 +376,9 @@
         if (headersLoaded)
             return;
 
+ // "headers" should never be null since it's set in the constructor.
+ // If something did go wrong this will fix it, but is an unsynchronized
+ // assignment of "headers".
         if (headers == null)
             headers = new InternetHeaders();
 


diff -r d50cb5ccdc6e -r b3cfc99bc009 pom.xml
--- a/pom.xml Mon Aug 20 15:39:50 2012 -0700
+++ b/pom.xml Fri Aug 31 15:19:34 2012 -0700
@@ -72,16 +72,16 @@
 
     <licenses>
       <license>
- <name>CDDL</name>
- <url>http://www.sun.com/cddl</url>
- <distribution>repo</distribution>
- <comments>A business-friendly OSS license</comments>
+ <name>CDDL</name>
+ <url>http://www.sun.com/cddl</url>
+ <distribution>repo</distribution>
+ <comments>A business-friendly OSS license</comments>
       </license>
       <license>
- <name>GPLv2+CE</name>
- <url>https://glassfish.java.net/public/CDDL+GPL_1_1.html</url>
- <distribution>repo</distribution>
- <comments>GPL version 2 plus the Classpath Exception</comments>
+ <name>GPLv2+CE</name>
+ <url>https://glassfish.java.net/public/CDDL+GPL_1_1.html</url>
+ <distribution>repo</distribution>
+ <comments>GPL version 2 plus the Classpath Exception</comments>
       </license>
     </licenses>
 
@@ -138,15 +138,15 @@
     </properties>
 
     <developers>
- <developer>
- <id>shannon</id>
- <name>Bill Shannon</name>
+ <developer>
+ <id>shannon</id>
+ <name>Bill Shannon</name>
             <email>bill.shannon_at_oracle.com</email>
- <organization>Oracle</organization>
- <roles>
- <role>lead</role>
- </roles>
- </developer>
+ <organization>Oracle</organization>
+ <roles>
+ <role>lead</role>
+ </roles>
+ </developer>
     </developers>
 
     <modules>
@@ -322,7 +322,7 @@
 
     <build>
         <defaultGoal>install</defaultGoal>
- <plugins>
+ <plugins>
             <!--
                 Make sure we're using the correct version of maven,
                 required to allow us to build JavaMail with JDK 1.4
@@ -422,8 +422,8 @@
                 Use the 1.4 compiler for JavaMail itself, but use the
                 1.5 compiler for the test classes, so we can use JUnit 4.
             -->
- <plugin>
- <artifactId>maven-compiler-plugin</artifactId>
+ <plugin>
+ <artifactId>maven-compiler-plugin</artifactId>
                 <executions>
                     <execution>
                         <id>default-compile</id>
@@ -495,22 +495,31 @@
             <!--
                 Tell the source plugin about the sources that may have
                 been downloaded by the maven-dependency-plugin.
+
+ Also, need this plugin to define target/classes as another
+ source directory so that the filtered Version.java
+ that's copied there will also be compiled when using
+ the latest version of the maven-compiler-plugin.
             -->
+
             <plugin>
                 <groupId>org.codehaus.mojo</groupId>
                 <artifactId>build-helper-maven-plugin</artifactId>
                 <executions>
                     <execution>
                         <id>add-source</id>
- <phase>package</phase>
+ <phase>generate-sources</phase>
                         <goals>
                             <goal>add-source</goal>
                         </goals>
                         <configuration>
                             <sources>
- <source>
+ <source> <!-- for dependencies -->
                                     ${project.build.directory}/sources
                                 </source>
+ <source> <!-- for Version.java -->
+ target/classes
+ </source>
                             </sources>
                         </configuration>
                     </execution>
@@ -536,6 +545,14 @@
                 </executions>
                 <configuration>
                     <includePom>true</includePom>
+ <!--
+ Since we added the classes directory using the
+ build-helper-maven-plugin above, we need to exclude
+ the class files from the source jar file.
+ -->
+ <excludes>
+ <exclude>**/*.class</exclude>
+ </excludes>
                 </configuration>
             </plugin>
 
@@ -579,14 +596,14 @@
                 </configuration>
             </plugin>
 -->
- </plugins>
+ </plugins>
 
         <pluginManagement>
             <plugins>
                 <plugin>
                     <groupId>org.apache.maven.plugins</groupId>
                     <artifactId>maven-compiler-plugin</artifactId>
- <version>2.0.2</version>
+ <version>2.4</version>
                 </plugin>
                 <plugin>
                     <groupId>org.apache.maven.plugins</groupId>
@@ -691,11 +708,11 @@
     </dependencyManagement>
 
     <dependencies>
- <dependency>
- <groupId>javax.activation</groupId>
- <artifactId>activation</artifactId>
- <version>${activation-api.version}</version>
- </dependency>
+ <dependency>
+ <groupId>javax.activation</groupId>
+ <artifactId>activation</artifactId>
+ <version>${activation-api.version}</version>
+ </dependency>
     </dependencies>
 
     <reporting>


diff -r b3cfc99bc009 -r 15ca73b5b264 doc/release/CHANGES.txt
--- a/doc/release/CHANGES.txt Fri Aug 31 15:19:34 2012 -0700
+++ b/doc/release/CHANGES.txt Tue Sep 18 13:45:09 2012 -0700
@@ -24,6 +24,7 @@
 K 5233 Infinite loop if Quota information is not correctly reported by server
 <no id> add mail.imap.ignorebodystructuresize to work around server bugs
 <no id> add "gimap" EXPERIMENTAL Gmail IMAP provider
+<no id> add isSSL() method to all protocol providers
 
 
                   CHANGES IN THE 1.4.5 RELEASE

diff -r b3cfc99bc009 -r 15ca73b5b264 mail/src/main/java/com/sun/mail/iap/Protocol.java
--- a/mail/src/main/java/com/sun/mail/iap/Protocol.java Fri Aug 31 15:19:34 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/iap/Protocol.java Tue Sep 18 13:45:09 2012 -0700
@@ -391,6 +391,16 @@
     }
 
     /**
+ * Is this connection using an SSL socket?
+ *
+ * @return true if using SSL
+ * @since JavaMail 1.4.6
+ */
+ public boolean isSSL() {
+ return socket instanceof SSLSocket;
+ }
+
+ /**
      * Disconnect.
      */
     protected synchronized void disconnect() {

diff -r b3cfc99bc009 -r 15ca73b5b264 mail/src/main/java/com/sun/mail/imap/IMAPStore.java
--- a/mail/src/main/java/com/sun/mail/imap/IMAPStore.java Fri Aug 31 15:19:34 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/imap/IMAPStore.java Tue Sep 18 13:45:09 2012 -0700
@@ -189,6 +189,7 @@
     private boolean disableAuthNtlm = false; // disable AUTH=NTLM
     private boolean enableStartTLS = false; // enable STARTTLS
     private boolean requireStartTLS = false; // require STARTTLS
+ private boolean usingSSL = false; // using SSL?
     private boolean enableSASL = false; // enable SASL authentication
     private String[] saslMechanisms;
     private boolean forcePasswordRefresh = false;
@@ -652,6 +653,8 @@
 
                 protocol.addResponseHandler(this);
 
+ usingSSL = protocol.isSSL(); // in case anyone asks
+
                 this.host = host;
                 this.user = user;
                 this.password = password;
@@ -788,6 +791,16 @@
     }
 
     /**
+ * Does this IMAPStore use SSL when connecting to the server?
+ *
+ * @return true if using SSL
+ * @since JavaMail 1.4.6
+ */
+ public boolean isSSL() {
+ return usingSSL;
+ }
+
+ /**
      * Set the user name that will be used for subsequent connections
      * after this Store is first connected (for example, when creating
      * a connection to open a Folder). This value is overridden

diff -r b3cfc99bc009 -r 15ca73b5b264 mail/src/main/java/com/sun/mail/pop3/POP3Store.java
--- a/mail/src/main/java/com/sun/mail/pop3/POP3Store.java Fri Aug 31 15:19:34 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/pop3/POP3Store.java Tue Sep 18 13:45:09 2012 -0700
@@ -1,7 +1,7 @@
 /*
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER.
  *
- * Copyright (c) 1997-2011 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997-2012 Oracle and/or its affiliates. All rights reserved.
  *
  * The contents of this file are subject to the terms of either the GNU
  * General Public License Version 2 only ("GPL") or the Common Development
@@ -77,6 +77,7 @@
     private String passwd = null;
     private boolean useStartTLS = false;
     private boolean requireStartTLS = false;
+ private boolean usingSSL = false;
     private Map capabilities;
     private PrintStream out;
 
@@ -289,6 +290,7 @@
         }
 
         capabilities = p.getCapabilities(); // save for later, may be null
+ usingSSL = p.isSSL(); // in case anyone asks
 
         /*
          * If we haven't explicitly disabled use of the TOP command,
@@ -396,6 +398,16 @@
             return Collections.EMPTY_MAP;
     }
 
+ /**
+ * Is this POP3Store using SSL to connect to the server?
+ *
+ * @return true if using SSL
+ * @since JavaMail 1.4.6
+ */
+ public boolean isSSL() {
+ return usingSSL;
+ }
+
     protected void finalize() throws Throwable {
         super.finalize();
 

diff -r b3cfc99bc009 -r 15ca73b5b264 mail/src/main/java/com/sun/mail/pop3/Protocol.java
--- a/mail/src/main/java/com/sun/mail/pop3/Protocol.java Fri Aug 31 15:19:34 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/pop3/Protocol.java Tue Sep 18 13:45:09 2012 -0700
@@ -671,6 +671,13 @@
     }
 
     /**
+ * Is this connection using SSL?
+ */
+ synchronized boolean isSSL() {
+ return socket instanceof SSLSocket;
+ }
+
+ /**
      * Get server capabilities using CAPA command specified by RFC 2449.
      * Returns null if not supported.
      */

diff -r b3cfc99bc009 -r 15ca73b5b264 mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java
--- a/mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java Fri Aug 31 15:19:34 2012 -0700
+++ b/mail/src/main/java/com/sun/mail/smtp/SMTPTransport.java Tue Sep 18 13:45:09 2012 -0700
@@ -504,6 +504,16 @@
     }
 
     /**
+ * Is this Transport using SSL to connect to the server?
+ *
+ * @return true if using SSL
+ * @since JavaMail 1.4.6
+ */
+ public boolean isSSL() {
+ return serverSocket instanceof SSLSocket;
+ }
+
+ /**
      * Should we use the RSET command instead of the NOOP command
      * in the @{link #isConnected isConnected} method?
      *


diff -r 15ca73b5b264 -r b9c0981a9ce2 mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java
--- a/mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java Tue Sep 18 13:45:09 2012 -0700
+++ b/mail/src/test/java/com/sun/mail/util/logging/MailHandlerTest.java Fri Sep 21 13:25:28 2012 -0700
@@ -3141,6 +3141,11 @@
         instance.close();
     }
 
+ /**
+ * Test logging permissions of the MailHandler.
+ * Must run by itself or run in isolated VM.
+ * Use system property java.security.debug=all to troubleshoot failures.
+ */
     @Test
     public void testSecurityManager() {
         InternalErrorManager em;
@@ -4630,7 +4635,7 @@
     }
 
     /**
- * Test must run last.
+ * Must run by itself or run in isolated VM.
      */
     @Test
     public void testZInit() {
@@ -5302,18 +5307,34 @@
 
         @Override
         public void checkPermission(java.security.Permission perm) {
- if (secure) {
+ try { //Call super class always for 'java.security.debug=all'.
                 super.checkPermission(perm);
- throw new SecurityException(perm.toString());
+ if (secure && isLogging(perm)) {
+ throw new SecurityException(perm.toString());
+ }
+ } catch (SecurityException se) {
+ if (secure && isLogging(perm)) {
+ throw se;
+ }
             }
         }
 
         @Override
         public void checkPermission(java.security.Permission perm, Object context) {
- if (secure) {
+ try { //Call super class always for 'java.security.debug=all'.
                 super.checkPermission(perm, context);
- throw new SecurityException(perm.toString());
- }
+ if (secure && isLogging(perm)) {
+ throw new SecurityException(perm.toString());
+ }
+ } catch (SecurityException se) {
+ if (secure && isLogging(perm)) {
+ throw se;
+ }
+ }
+ }
+
+ private static boolean isLogging(java.security.Permission perm) {
+ return perm instanceof LoggingPermission;
         }
     }