You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by se...@apache.org on 2014/12/01 01:33:11 UTC

svn commit: r1642615 - in /commons/proper/net/trunk/src: changes/changes.xml main/java/org/apache/commons/net/ftp/FTPClient.java test/java/org/apache/commons/net/ftp/FTPClientTest.java

Author: sebb
Date: Mon Dec  1 00:33:10 2014
New Revision: 1642615

URL: http://svn.apache.org/r1642615
Log:
NET-544 FTPClient.initiateListParsing does not correctly check if parserKey was cached

Modified:
    commons/proper/net/trunk/src/changes/changes.xml
    commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java
    commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java

Modified: commons/proper/net/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/net/trunk/src/changes/changes.xml?rev=1642615&r1=1642614&r2=1642615&view=diff
==============================================================================
--- commons/proper/net/trunk/src/changes/changes.xml [utf-8] (original)
+++ commons/proper/net/trunk/src/changes/changes.xml [utf-8] Mon Dec  1 00:33:10 2014
@@ -68,6 +68,9 @@ This is mainly a bug-fix release. See fu
   IMAPExportMbox (example app) allows IMAP folders to be exported into an mbox file.
   This is the inverse of the IMAPImportMbox example added previously
         ">
+            <action issue="NET-544" type="fix" dev="sebb" due-to="Olivier Queyrut ">
+            FTPClient.initiateListParsing does not correctly check if parserKey was cached
+            </action>
             <action issue="NET-554" type="update" dev="sebb">
             Simplify TelnetOptionHandler class hierarchy
             </action>

Modified: commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java
URL: http://svn.apache.org/viewvc/commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java?rev=1642615&r1=1642614&r2=1642615&view=diff
==============================================================================
--- commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java (original)
+++ commons/proper/net/trunk/src/main/java/org/apache/commons/net/ftp/FTPClient.java Mon Dec  1 00:33:10 2014
@@ -3268,9 +3268,16 @@ implements Configurable
             String parserKey, String pathname)
     throws IOException
     {
+        __createParser(parserKey); // create and cache parser
+        return initiateListParsing(__entryParser, pathname);
+    }
+
+    // package access for test purposes
+    void __createParser(String parserKey) throws IOException {
         // We cache the value to avoid creation of a new object every
         // time a file listing is generated.
-        if(__entryParser == null ||  ! __entryParserKey.equals(parserKey)) {
+        // Note: we don't check against a null parserKey (NET-544)
+        if(__entryParser == null ||  (parserKey != null && ! __entryParserKey.equals(parserKey))) {
             if (null != parserKey) {
                 // if a parser key was supplied in the parameters,
                 // use that to create the parser
@@ -3307,7 +3314,6 @@ implements Configurable
             }
         }
 
-        return initiateListParsing(__entryParser, pathname);
 
     }
 
@@ -3784,6 +3790,11 @@ implements Configurable
         return __autodetectEncoding;
     }
 
+    // Method for use by unit test code only
+    FTPFileEntryParser getEntryParser() {
+        return __entryParser;
+    }
+
     // DEPRECATED METHODS - for API compatibility only - DO NOT USE
 
     /**

Modified: commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
URL: http://svn.apache.org/viewvc/commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java?rev=1642615&r1=1642614&r2=1642615&view=diff
==============================================================================
--- commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java (original)
+++ commons/proper/net/trunk/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java Mon Dec  1 00:33:10 2014
@@ -18,6 +18,8 @@
 
 package org.apache.commons.net.ftp;
 
+import java.io.IOException;
+
 import junit.framework.TestCase;
 
 public class FTPClientTest extends TestCase {
@@ -58,4 +60,44 @@ public class FTPClientTest extends TestC
         }
     }
 
+    public void testParserCachingWithKey() throws Exception {
+        FTPClient client = new FTPClient();
+        assertNull(client.getEntryParser());
+        client.__createParser(FTPClientConfig.SYST_UNIX);
+        final FTPFileEntryParser entryParserSYST = client.getEntryParser();
+        assertNotNull(entryParserSYST);
+        client.__createParser(FTPClientConfig.SYST_UNIX);
+        assertSame(entryParserSYST, client.getEntryParser()); // the previous entry was cached
+        client.__createParser(FTPClientConfig.SYST_VMS);
+        final FTPFileEntryParser entryParserVMS = client.getEntryParser();
+        assertNotSame(entryParserSYST, entryParserVMS); // the previous entry was replaced
+        client.__createParser(FTPClientConfig.SYST_VMS);
+        assertSame(entryParserVMS, client.getEntryParser()); // the previous entry was cached
+        client.__createParser(FTPClientConfig.SYST_UNIX); // revert
+        assertNotSame(entryParserVMS, client.getEntryParser()); // the previous entry was replaced
+    }
+
+    private static class LocalClient extends FTPClient {
+        private String systemType;
+        @Override
+        public String getSystemType() throws IOException {
+            return systemType;
+        }
+        public void setSystemType(String type) {
+            systemType = type;
+        }
+    }
+    public void testParserCachingNullKey() throws Exception {
+        LocalClient client = new LocalClient();
+        client.setSystemType(FTPClientConfig.SYST_UNIX);
+        assertNull(client.getEntryParser());
+        client.__createParser(null);
+        final FTPFileEntryParser entryParser = client.getEntryParser();
+        assertNotNull(entryParser);
+        client.__createParser(null);
+        assertSame(entryParser, client.getEntryParser()); // parser was cached
+        client.setSystemType(FTPClientConfig.SYST_NT);
+        client.__createParser(null);
+        assertSame(entryParser, client.getEntryParser()); // parser was cached
+    }
 }