Make system properties read only.

The issue is that some parts of the system libraries
(and the runtime) assume that these properties are
immutable. This change enforces that notion and makes
behaviour a lot more predictable.

This is not a backwards compatible change, but based
on analysis of code available to me I couldn't find any
instances where this change would break any application.

Has the side effect of fixing..

bug: 12491204
bug: 12491159

Change-Id: I60c73713c97a4721d0b9781010333a9e9c883534

2 files changed, 87 insertions, 27 deletions

diff --git a/luni/src/main/java/java/lang/System.java b/luni/src/main/java/java/lang/System.java
index 33c9c03..7cf3993 100644
--- a/luni/src/main/java/java/lang/System.java
+++ b/luni/src/main/java/java/lang/System.java
@@ -55,7 +55,6 @@ import libcore.io.ErrnoException;
 import libcore.io.Libcore;
 import libcore.io.StructPasswd;
 import libcore.io.StructUtsname;
-import libcore.util.ZoneInfoDB;
 
 /**
  * Provides access to system-related information and resources including
@@ -83,6 +82,7 @@ public final class System {
     public static final PrintStream err;
 
     private static final String lineSeparator;
+    private static final Properties unchangeableSystemProperties;
     private static Properties systemProperties;
 
     /**
@@ -105,6 +105,8 @@ public final class System {
         err = new PrintStream(new FileOutputStream(FileDescriptor.err));
         out = new PrintStream(new FileOutputStream(FileDescriptor.out));
         in = new BufferedInputStream(new FileInputStream(FileDescriptor.in));
+        unchangeableSystemProperties = initUnchangeableSystemProperties();
+        systemProperties = createSystemProperties();
         lineSeparator = System.getProperty("line.separator");
     }
 
@@ -326,13 +328,10 @@ public final class System {
      * @return the system properties.
      */
     public static Properties getProperties() {
-        if (systemProperties == null) {
-            initSystemProperties();
-        }
         return systemProperties;
     }
 
-    private static void initSystemProperties() {
+    private static Properties initUnchangeableSystemProperties() {
         VMRuntime runtime = VMRuntime.getRuntime();
         Properties p = new Properties();
 
@@ -357,10 +356,6 @@ public final class System {
         }
         p.put("java.home", javaHome);
 
-        // On Android, each app gets its own temporary directory. This is just a fallback
-        // default, useful only on the host.
-        p.put("java.io.tmpdir", "/tmp");
-
         String ldLibraryPath = getenv("LD_LIBRARY_PATH");
         if (ldLibraryPath != null) {
             p.put("java.library.path", ldLibraryPath);
@@ -413,8 +408,16 @@ public final class System {
 
         // Override built-in properties with settings from the command line.
         parsePropertyAssignments(p, runtime.properties());
+        return p;
+    }
 
-        systemProperties = p;
+    private static Properties createSystemProperties() {
+        Properties p = new PropertiesWithNonOverrideableDefaults(unchangeableSystemProperties);
+        // On Android, each app gets its own temporary directory.
+        // (See android.app.ActivityThread.) This is just a fallback default,
+        // useful only on the host.
+        p.put("java.io.tmpdir", "/tmp");
+        return p;
     }
 
     /**
@@ -440,7 +443,8 @@ public final class System {
      * Returns the value of a particular system property or {@code null} if no
      * such property exists.
      *
-     * <p>The following properties are always provided by the Dalvik VM:
+     * <p>The following properties are always provided by the Dalvik VM <b>and
+     * cannot be modified</b>:
      * <p><table BORDER="1" WIDTH="100%" CELLPADDING="3" CELLSPACING="0" SUMMARY="">
      * <tr BGCOLOR="#CCCCFF" CLASS="TableHeadingColor">
      *     <td><b>Name</b></td>        <td><b>Meaning</b></td>                    <td><b>Example</b></td></tr>
@@ -481,7 +485,8 @@ public final class System {
      *
      * </table>
      *
-     * <p>It is a mistake to try to override any of these. Doing so will have unpredictable results.
+     * <p>It is an error to override anyone of these properties. Any attempt to
+     * do so will leave their values unchanged.
      *
      * @param propertyName
      *            the name of the system property to look up.
@@ -498,22 +503,26 @@ public final class System {
      */
     public static String getProperty(String name, String defaultValue) {
         checkPropertyName(name);
-        return getProperties().getProperty(name, defaultValue);
+        return systemProperties.getProperty(name, defaultValue);
     }
 
     /**
-     * Sets the value of a particular system property.
+     * Sets the value of a particular system property. Most system properties
+     * are read only and cannot be cleared or modified. See {@link #setProperty} for a
+     * list of such properties.
      *
      * @return the old value of the property or {@code null} if the property
      *         didn't exist.
      */
     public static String setProperty(String name, String value) {
         checkPropertyName(name);
-        return (String) getProperties().setProperty(name, value);
+        return (String) systemProperties.setProperty(name, value);
     }
 
     /**
-     * Removes a specific system property.
+     * Removes a specific system property. Most system properties
+     * are read only and cannot be cleared or modified. See {@link #setProperty} for a
+     * list of such properties.
      *
      * @return the property value or {@code null} if the property didn't exist.
      * @throws NullPointerException
@@ -523,7 +532,7 @@ public final class System {
      */
     public static String clearProperty(String name) {
         checkPropertyName(name);
-        return (String) getProperties().remove(name);
+        return (String) systemProperties.remove(name);
     }
 
     private static void checkPropertyName(String name) {
@@ -685,12 +694,18 @@ public final class System {
     }
 
     /**
-     * Sets all system properties. This does not take a copy; the passed-in object is used
-     * directly. Passing null causes the VM to reinitialize the properties to how they were
-     * when the VM was started.
+     * Attempts to set all system properties. Copies all properties from
+     * {@code p} and discards system properties that are read only and cannot
+     * be modified. See {@link #setProperty} for a list of such properties.
      */
     public static void setProperties(Properties p) {
-        systemProperties = p;
+        PropertiesWithNonOverrideableDefaults userProperties =
+                new PropertiesWithNonOverrideableDefaults(unchangeableSystemProperties);
+        if (p != null) {
+            userProperties.putAll(p);
+        }
+
+        systemProperties = userProperties;
     }
 
     /**
@@ -731,6 +746,36 @@ public final class System {
      */
     private static native void setFieldImpl(String fieldName, String signature, Object stream);
 
+    /**
+     * A properties class that prohibits changes to any of the properties
+     * contained in its defaults.
+     */
+    static final class PropertiesWithNonOverrideableDefaults extends Properties {
+        PropertiesWithNonOverrideableDefaults(Properties defaults) {
+            super(defaults);
+        }
+
+        @Override
+        public Object put(Object key, Object value) {
+            if (defaults.containsKey(key)) {
+                logE("Ignoring attempt to set property \"" + key +
+                        "\" to value \"" + value + "\".");
+                return defaults.get(key);
+            }
+
+            return super.put(key, value);
+        }
+
+        @Override
+        public Object remove(Object key) {
+            if (defaults.containsKey(key)) {
+                logE("Ignoring attempt to remove property \"" + key + "\".");
+                return null;
+            }
+
+            return super.remove(key);
+        }
+    }
 
     /**
      * The unmodifiable environment variables map. System.getenv() specifies
diff --git a/luni/src/test/java/libcore/java/lang/SystemTest.java b/luni/src/test/java/libcore/java/lang/SystemTest.java
index 9fc5e8b..4efecd7 100644
--- a/luni/src/test/java/libcore/java/lang/SystemTest.java
+++ b/luni/src/test/java/libcore/java/lang/SystemTest.java
@@ -22,6 +22,7 @@ import java.io.PrintStream;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.Formatter;
+import java.util.Properties;
 import java.util.concurrent.atomic.AtomicBoolean;
 import junit.framework.TestCase;
 
@@ -33,11 +34,11 @@ public class SystemTest extends TestCase {
             // use System.getProperty. Now they should use System.lineSeparator instead, and the
             // "line.separator" property has no effect after the VM has started.
 
-            // Test System.lineSeparator directly.
+            // Test that System.lineSeparator is not changed when the corresponding
+            // system property is changed.
             assertEquals("\n", System.lineSeparator());
             System.setProperty("line.separator", "poop");
             assertEquals("\n", System.lineSeparator());
-            assertFalse(System.lineSeparator().equals(System.getProperty("line.separator")));
 
             // java.io.BufferedWriter --- uses System.lineSeparator on Android but not on RI.
             StringWriter sw = new StringWriter();
@@ -45,23 +46,19 @@ public class SystemTest extends TestCase {
             bw.newLine();
             bw.flush();
             assertEquals(System.lineSeparator(), sw.toString());
-            assertFalse(System.lineSeparator().equals(System.getProperty("line.separator")));
 
             // java.io.PrintStream --- uses System.lineSeparator on Android but not on RI.
             ByteArrayOutputStream baos = new ByteArrayOutputStream();
             new PrintStream(baos).println();
             assertEquals(System.lineSeparator(), new String(baos.toByteArray(), "UTF-8"));
-            assertFalse(System.lineSeparator().equals(System.getProperty("line.separator")));
 
             // java.io.PrintWriter --- uses System.lineSeparator on Android but not on RI.
             sw = new StringWriter();
             new PrintWriter(sw).println();
             assertEquals(System.lineSeparator(), sw.toString());
-            assertFalse(System.lineSeparator().equals(System.getProperty("line.separator")));
 
             // java.util.Formatter --- uses System.lineSeparator on both.
             assertEquals(System.lineSeparator(), new Formatter().format("%n").toString());
-            assertFalse(System.lineSeparator().equals(System.getProperty("line.separator")));
         } finally {
             System.setProperty("line.separator", "\n");
         }
@@ -151,4 +148,22 @@ public class SystemTest extends TestCase {
 
         done.set(true);
     }
+
+    public void testSystemProperties_immtuable() {
+        String userDir = System.getProperty("user.dir");
+        System.setProperty("user.dir", "not poop");
+        assertEquals(userDir, System.getProperty("user.dir"));
+
+        System.getProperties().setProperty("user.dir", "hmmph");
+        assertEquals(userDir, System.getProperty("user.dir"));
+
+        System.getProperties().clear();
+        assertEquals(userDir, System.getProperty("user.dir"));
+
+        Properties p = new Properties();
+        p.setProperty("user.dir", "meh");
+        System.setProperties(p);
+
+        assertEquals(userDir, System.getProperty("user.dir"));
+    }
 }