FELIX-4302 Config Admin can create illegal names on windows

Configuration PIDs are encoded to be used as path names for
configuration files. On Windows platforms each segment in the
path must not start with one of the Windows device special names
such as LPT, COM, etc. Improved the encodePid method to encode
the first character of a segment name if it starts with one of the
special names.

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1554073 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java b/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
index ddd43c7..5cd224c 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
@@ -33,8 +33,11 @@
 import java.util.BitSet;
 import java.util.Dictionary;
 import java.util.Enumeration;
+import java.util.HashSet;
 import java.util.NoSuchElementException;
+import java.util.Set;
 import java.util.Stack;
+import java.util.StringTokenizer;
 
 import org.apache.felix.cm.PersistenceManager;
 import org.osgi.framework.BundleContext;
@@ -135,6 +138,22 @@
      */
     private final File location;
 
+    /**
+     * Flag indicating whether this instance is running on a Windows
+     * platform or not.
+     */
+    private final boolean isWin;
+
+    /**
+     * A set of three character names (prefixes) considered reserved
+     * on Windows platform systems. This set consists of names such as
+     * LPT, CON, COM, etc., which cause weird behaviour on Windows systems
+     * if used as prefixes on path segments. See .
+     *
+     * @see <a href="https://issues.apache.org/jira/browse/FELIX-4302">FELIX-4302</a>
+     */
+    private final Set winDevNames;
+
     // sets up this class defining the set of valid characters in path
     // set getFile(String) for details.
     static
@@ -160,61 +179,29 @@
     }
 
 
-    /**
-     * Encodes a Service PID to a filesystem path as described in the class
-     * JavaDoc above.
-     * <p>
-     * This method is not part of the API of this class and is declared package
-     * private to enable JUnit testing on it. This method may be removed or
-     * modified at any time without notice.
-     *
-     * @param pid The Service PID to encode into a relative path name.
-     *
-     * @return The relative path name corresponding to the Service PID.
-     */
-    static String encodePid( String pid )
     {
-
-        // replace dots by File.separatorChar
-        pid = pid.replace( '.', File.separatorChar );
-
-        // replace slash by File.separatorChar if different
-        if ( File.separatorChar != '/' )
+        // Windows Specific Preparation (FELIX-4302)
+        // According to the GetJavaProperties method in
+        // jdk/src/windows/native/java/lang/java_props_md.c the os.name
+        // system property on all Windows platforms start with the string
+        // "Windows" hence we assume a Windows platform in thus case.
+        final String osName = System.getProperty( "os.name" );
+        isWin = osName != null && osName.startsWith( "Windows" );
+        if ( isWin )
         {
-            pid = pid.replace( '/', File.separatorChar );
+            winDevNames = new HashSet();
+            winDevNames.add( "CON" ); // keyboard and display
+            winDevNames.add( "PRN" ); // system list; generally par. port
+            winDevNames.add( "AUX" ); // auxiliary; generally ser. port
+            winDevNames.add( "CLO" ); // CLOCK$; system real time clock
+            winDevNames.add( "NUL" ); // Bit-bucket
+            winDevNames.add( "COM" ); // COM1..COMn; serial ports
+            winDevNames.add( "LPT" ); // LPT1..LPTn; parallel ports
         }
-
-        // scan for first non-valid character (if any)
-        int first = 0;
-        while ( first < pid.length() && VALID_PATH_CHARS.get( pid.charAt( first ) ) )
+        else
         {
-            first++;
+            winDevNames = null;
         }
-
-        // check whether we exhausted
-        if ( first < pid.length() )
-        {
-            StringBuffer buf = new StringBuffer( pid.substring( 0, first ) );
-
-            for ( int i = first; i < pid.length(); i++ )
-            {
-                char c = pid.charAt( i );
-                if ( VALID_PATH_CHARS.get( c ) )
-                {
-                    buf.append( c );
-                }
-                else
-                {
-                    String val = "000" + Integer.toHexString( c );
-                    buf.append( '%' );
-                    buf.append( val.substring( val.length() - 4 ) );
-                }
-            }
-
-            pid = buf.toString();
-        }
-
-        return pid;
     }
 
 
@@ -357,6 +344,91 @@
 
 
     /**
+     * Encodes a Service PID to a filesystem path as described in the class
+     * JavaDoc above.
+     * <p>
+     * This method is not part of the API of this class and is declared package
+     * private to enable JUnit testing on it. This method may be removed or
+     * modified at any time without notice.
+     *
+     * @param pid The Service PID to encode into a relative path name.
+     *
+     * @return The relative path name corresponding to the Service PID.
+     */
+    String encodePid( String pid )
+    {
+
+        // replace dots by File.separatorChar
+        pid = pid.replace( '.', File.separatorChar );
+
+        // replace slash by File.separatorChar if different
+        if ( File.separatorChar != '/' )
+        {
+            pid = pid.replace( '/', File.separatorChar );
+        }
+
+        // scan for first non-valid character (if any)
+        int first = 0;
+        while ( first < pid.length() && VALID_PATH_CHARS.get( pid.charAt( first ) ) )
+        {
+            first++;
+        }
+
+        // check whether we exhausted
+        if ( first < pid.length() )
+        {
+            StringBuffer buf = new StringBuffer( pid.substring( 0, first ) );
+
+            for ( int i = first; i < pid.length(); i++ )
+            {
+                char c = pid.charAt( i );
+                if ( VALID_PATH_CHARS.get( c ) )
+                {
+                    buf.append( c );
+                }
+                else
+                {
+                    appendEncoded( buf, c );
+                }
+            }
+
+            pid = buf.toString();
+        }
+
+        // Prefix special device names on Windows (FELIX-4302)
+        if ( isWin )
+        {
+            final StringTokenizer segments = new StringTokenizer( pid, File.separator, true );
+            final StringBuffer pidBuffer = new StringBuffer( pid.length() );
+            while ( segments.hasMoreTokens() )
+            {
+                final String segment = segments.nextToken();
+                if ( segment.length() >= 3 && winDevNames.contains( segment.substring( 0, 3 ).toUpperCase() ) )
+                {
+                    appendEncoded( pidBuffer, segment.charAt( 0 ) );
+                    pidBuffer.append( segment.substring( 1 ) );
+                }
+                else
+                {
+                    pidBuffer.append( segment );
+                }
+            }
+            pid = pidBuffer.toString();
+        }
+
+        return pid;
+    }
+
+
+    private void appendEncoded( StringBuffer buf, final char c )
+    {
+        String val = "000" + Integer.toHexString( c );
+        buf.append( '%' );
+        buf.append( val.substring( val.length() - 4 ) );
+    }
+
+
+    /**
      * Returns the directory in which the configuration files are written as
      * a <code>File</code> object.
      *
diff --git a/configadmin/src/main/resources/OSGI-INF/permissions.perm b/configadmin/src/main/resources/OSGI-INF/permissions.perm
index 98ba165..1f3fb33 100644
--- a/configadmin/src/main/resources/OSGI-INF/permissions.perm
+++ b/configadmin/src/main/resources/OSGI-INF/permissions.perm
@@ -39,6 +39,7 @@
 
 # Handle persistent configuration files
 # -> FilePersistenceManager
+(java.util.PropertyPermission "os.name" "read")
 (java.util.PropertyPermission "user.dir" "read")
 (java.io.FilePermission "-" "read,write,execute,delete")
 
diff --git a/configadmin/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java b/configadmin/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java
index 63b627f..b8a03a1 100644
--- a/configadmin/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java
+++ b/configadmin/src/test/java/org/apache/felix/cm/file/FilePersistenceManagerTest.java
@@ -62,16 +62,69 @@
 
     public void testPidPlain()
     {
-        assertEquals( "plain", FilePersistenceManager.encodePid( "plain" ) );
-        assertEquals( "plain" + File.separatorChar + "path", FilePersistenceManager.encodePid( "plain.path" ) );
-        assertEquals( "encod%00e8", FilePersistenceManager.encodePid( "encod\u00E8" ) );
-        assertEquals( "encod%00e8" + File.separatorChar + "path", FilePersistenceManager.encodePid( "encod\u00E8/path" ) );
-        assertEquals( "encode" + File.separatorChar + "%1234" + File.separatorChar + "path", FilePersistenceManager
-            .encodePid( "encode/\u1234/path" ) );
-        assertEquals( "encode" + File.separatorChar + " %0025 " + File.separatorChar + "path", FilePersistenceManager
-            .encodePid( "encode/ % /path" ) );
+        assertEquals( "plain", fpm.encodePid( "plain" ) );
+        assertEquals( "plain" + File.separatorChar + "path", fpm.encodePid( "plain.path" ) );
+        assertEquals( "encod%00e8", fpm.encodePid( "encod\u00E8" ) );
+        assertEquals( "encod%00e8" + File.separatorChar + "path", fpm.encodePid( "encod\u00E8/path" ) );
+        assertEquals( "encode" + File.separatorChar + "%1234" + File.separatorChar + "path",
+            fpm.encodePid( "encode/\u1234/path" ) );
+        assertEquals( "encode" + File.separatorChar + " %0025 " + File.separatorChar + "path",
+            fpm.encodePid( "encode/ % /path" ) );
     }
 
+    public void testPidEncodingCollision() {
+        // assert a == encode(a) ==> encode(a) == encode(encode(a))
+        final String plain = "plain";
+        assertEquals( plain, fpm.encodePid( plain ) );
+        assertEquals( fpm.encodePid( plain ), fpm.encodePid( fpm.encodePid( plain ) ) );
+        assertEquals( plain, fpm.encodePid( fpm.encodePid( plain ) ) );
+
+        // assert a != encode(a) ==> encode(a) != encode(encode(a))
+        final String encode = "encod\u00E8";
+        final String encoded = "encod%00e8";
+        assertEquals( encoded, fpm.encodePid( encode ) );
+        assertFalse( encode.equals( fpm.encodePid( encode ) ) );
+        assertFalse( fpm.encodePid( encode ).equals( fpm.encodePid( fpm.encodePid( encode ) ) ) );
+        assertFalse( encode.equals( fpm.encodePid( fpm.encodePid( encode ) ) ) );
+    }
+
+    public void testPidDeviceNameEncodingWindows()  {
+        // assert proper encoding of windows device file names (FELIX-4302)
+        String oldOsName = System.getProperty( "os.name" );
+        try {
+            System.setProperty("os.name", "Windows for testing");
+            FilePersistenceManager winFpm = new FilePersistenceManager( file.getAbsolutePath() );
+            assertEquals("%004cPT1", winFpm.encodePid( "LPT1" ));
+            assertEquals("%006cpt1", winFpm.encodePid( "lpt1" ));
+            assertEquals("%0043ON", winFpm.encodePid( "CON" ));
+            assertEquals("%0050RN", winFpm.encodePid( "PRN" ));
+            assertEquals("%0041UX", winFpm.encodePid( "AUX" ));
+            assertEquals("%0043LOCK%0024", winFpm.encodePid( "CLOCK$" ));
+            assertEquals("%004eUL", winFpm.encodePid( "NUL" ));
+            assertEquals("%0043OM6", winFpm.encodePid( "COM6" ));
+        } finally {
+            System.setProperty( "os.name", oldOsName );
+        }
+    }
+
+    public void testPidDeviceNameEncodingNonWindows()  {
+        // assert no encoding of windows device file names (FELIX-4302)
+        String oldOsName = System.getProperty( "os.name" );
+        try {
+            System.setProperty("os.name", "Unix for testing");
+            FilePersistenceManager winFpm = new FilePersistenceManager( file.getAbsolutePath() );
+            assertEquals("LPT1", winFpm.encodePid( "LPT1" ));
+            assertEquals("lpt1", winFpm.encodePid( "lpt1" ));
+            assertEquals("CON", winFpm.encodePid( "CON" ));
+            assertEquals("PRN", winFpm.encodePid( "PRN" ));
+            assertEquals("AUX", winFpm.encodePid( "AUX" ));
+            assertEquals("CLOCK%0024", winFpm.encodePid( "CLOCK$" ));
+            assertEquals("NUL", winFpm.encodePid( "NUL" ));
+            assertEquals("COM6", winFpm.encodePid( "COM6" ));
+        } finally {
+            System.setProperty( "os.name", oldOsName );
+        }
+    }
 
     public void testCreateDir()
     {
@@ -187,6 +240,20 @@
         check( "=leading equals", "leading equals" );
     }
 
+
+    // Test expected to always succeed on non-Windows platforms. It may
+    // break if FilePersistenceManager.encode does not cope properly
+    // with Windows device names (see FELIX-4302)
+    public void testWindowsSpecialNames() throws IOException
+    {
+        check( "prefixLPT1", "lpt1" );
+        check( "prefix.prefix2.LPT1.suffix", "lpt1" );
+        check( "prefix.LPT1.suffix", "lpt1" );
+        check( "prefix.LPT1", "lpt1" );
+        check( "LPT1", "lpt1" );
+    }
+
+
     private void check( String name, Object value ) throws IOException
     {
         Dictionary props = new Hashtable();
@@ -200,7 +267,7 @@
     {
         fpm.store( pid, props );
 
-        assertTrue( new File( file, FilePersistenceManager.encodePid( pid ) + ".config" ).exists() );
+        assertTrue( new File( file, fpm.encodePid( pid ) + ".config" ).exists() );
 
         Dictionary loaded = fpm.load( pid );
         assertNotNull( loaded );