8075156: (prefs) get*() and remove() should disallow the use of the null control character '\u0000' as key

Extend disallowing null control character key to remove()

Reviewed-by: rriggs, alanb

jdk/src/java.prefs/share/classes/java/util/prefs/AbstractPreferences.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -78,10 +78,9 @@ import java.lang.Double;
  * under which these calls cannot even enqueue the requested operation for
  * later processing.  Even under these circumstances it is generally better to
  * simply ignore the invocation and return, rather than throwing an
- * exception.  Under these circumstances, however, all subsequent invocations
+ * exception.  Under these circumstances, however, subsequently invoking
- * of <tt>flush()</tt> and <tt>sync</tt> should return <tt>false</tt>, as
+ * <tt>flush()</tt> or <tt>sync</tt> would not imply that all previous
- * returning <tt>true</tt> would imply that all previous operations had
+ * operations had successfully been made permanent.
- * successfully been made permanent.
  *
  * <p>There is one circumstance under which <tt>putSpi, removeSpi and
  * childSpi</tt> <i>should</i> throw an exception: if the caller lacks
@@ -122,6 +121,13 @@ import java.lang.Double;
  * @since   1.4
  */
 public abstract class AbstractPreferences extends Preferences {
+    /**
+     * The code point U+0000, assigned to the null control character, is the
+     * only character encoded in Unicode and ISO/IEC 10646 that is always
+     * invalid in any XML 1.0 and 1.1 document.
+     */
+    static final int CODE_POINT_U0000 = '\u0000';
+
     /**
      * Our name relative to parent.
      */
@@ -234,6 +240,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws IllegalArgumentException if <tt>key.length()</tt> exceeds
      *       <tt>MAX_KEY_LENGTH</tt> or if <tt>value.length</tt> exceeds
      *       <tt>MAX_VALUE_LENGTH</tt>.
+     * @throws IllegalArgumentException if either key or value contain
+     *         the null control character, code point U+0000.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      */
@@ -244,6 +252,10 @@ public abstract class AbstractPreferences extends Preferences {
             throw new IllegalArgumentException("Key too long: "+key);
         if (value.length() > MAX_VALUE_LENGTH)
             throw new IllegalArgumentException("Value too long: "+value);
+        if (key.indexOf(CODE_POINT_U0000) != -1)
+            throw new IllegalArgumentException("Key contains code point U+0000");
+        if (value.indexOf(CODE_POINT_U0000) != -1)
+            throw new IllegalArgumentException("Value contains code point U+0000");
 
         synchronized(lock) {
             if (removed)
@@ -275,10 +287,14 @@ public abstract class AbstractPreferences extends Preferences {
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if key is <tt>null</tt>.  (A
      *         <tt>null</tt> default <i>is</i> permitted.)
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public String get(String key, String def) {
         if (key==null)
             throw new NullPointerException("Null key");
+        if (key.indexOf(CODE_POINT_U0000) != -1)
+            throw new IllegalArgumentException("Key contains code point U+0000");
         synchronized(lock) {
             if (removed)
                 throw new IllegalStateException("Node has been removed.");
@@ -306,10 +322,14 @@ public abstract class AbstractPreferences extends Preferences {
      * @param key key whose mapping is to be removed from the preference node.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      * @throws NullPointerException {@inheritDoc}.
      */
     public void remove(String key) {
         Objects.requireNonNull(key, "Specified key cannot be null");
+        if (key.indexOf(CODE_POINT_U0000) != -1)
+            throw new IllegalArgumentException("Key contains code point U+0000");
         synchronized(lock) {
             if (removed)
                 throw new IllegalStateException("Node has been removed.");
@@ -353,6 +373,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws NullPointerException if key is <tt>null</tt>.
      * @throws IllegalArgumentException if <tt>key.length()</tt> exceeds
      *         <tt>MAX_KEY_LENGTH</tt>.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      */
@@ -381,6 +403,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public int getInt(String key, int def) {
         int result = def;
@@ -408,6 +432,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws NullPointerException if key is <tt>null</tt>.
      * @throws IllegalArgumentException if <tt>key.length()</tt> exceeds
      *         <tt>MAX_KEY_LENGTH</tt>.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      */
@@ -436,6 +462,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public long getLong(String key, long def) {
         long result = def;
@@ -463,6 +491,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws NullPointerException if key is <tt>null</tt>.
      * @throws IllegalArgumentException if <tt>key.length()</tt> exceeds
      *         <tt>MAX_KEY_LENGTH</tt>.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      */
@@ -494,6 +524,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public boolean getBoolean(String key, boolean def) {
         boolean result = def;
@@ -521,6 +553,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws NullPointerException if key is <tt>null</tt>.
      * @throws IllegalArgumentException if <tt>key.length()</tt> exceeds
      *         <tt>MAX_KEY_LENGTH</tt>.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      */
@@ -549,6 +583,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public float getFloat(String key, float def) {
         float result = def;
@@ -576,6 +612,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws NullPointerException if key is <tt>null</tt>.
      * @throws IllegalArgumentException if <tt>key.length()</tt> exceeds
      *         <tt>MAX_KEY_LENGTH</tt>.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      */
@@ -604,6 +642,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public double getDouble(String key, double def) {
         double result = def;
@@ -627,6 +667,8 @@ public abstract class AbstractPreferences extends Preferences {
      * @throws NullPointerException if key or value is <tt>null</tt>.
      * @throws IllegalArgumentException if key.length() exceeds MAX_KEY_LENGTH
      *         or if value.length exceeds MAX_VALUE_LENGTH*3/4.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      */
@@ -650,6 +692,8 @@ public abstract class AbstractPreferences extends Preferences {
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.  (A
      *         <tt>null</tt> value for <tt>def</tt> <i>is</i> permitted.)
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public byte[] getByteArray(String key, byte[] def) {
         byte[] result = def;

jdk/src/java.prefs/share/classes/java/util/prefs/Preferences.java

@@ -489,7 +489,7 @@ public abstract class Preferences {
      *       <tt>MAX_VALUE_LENGTH</tt>.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
-     * @throws IllegalArgumentException if either the key or the value contain
+     * @throws IllegalArgumentException if either key or value contain
      *         the null control character, code point U+0000.
      */
     public abstract void put(String key, String value);
@@ -514,6 +514,8 @@ public abstract class Preferences {
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.  (A
      *         <tt>null</tt> value for <tt>def</tt> <i>is</i> permitted.)
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public abstract String get(String key, String def);
 
@@ -530,6 +532,8 @@ public abstract class Preferences {
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      */
     public abstract void remove(String key);
 
@@ -566,6 +570,8 @@ public abstract class Preferences {
      *         <tt>MAX_KEY_LENGTH</tt>.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @see #getInt(String,int)
      */
     public abstract void putInt(String key, int value);
@@ -597,6 +603,8 @@ public abstract class Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      * @see #putInt(String,int)
      * @see #get(String,String)
      */
@@ -616,6 +624,8 @@ public abstract class Preferences {
      *         <tt>MAX_KEY_LENGTH</tt>.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @see #getLong(String,long)
      */
     public abstract void putLong(String key, long value);
@@ -647,6 +657,8 @@ public abstract class Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      * @see #putLong(String,long)
      * @see #get(String,String)
      */
@@ -666,6 +678,8 @@ public abstract class Preferences {
      *         <tt>MAX_KEY_LENGTH</tt>.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @see #getBoolean(String,boolean)
      * @see #get(String,String)
      */
@@ -702,6 +716,8 @@ public abstract class Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      * @see #get(String,String)
      * @see #putBoolean(String,boolean)
      */
@@ -721,6 +737,8 @@ public abstract class Preferences {
      *         <tt>MAX_KEY_LENGTH</tt>.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @see #getFloat(String,float)
      */
     public abstract void putFloat(String key, float value);
@@ -751,6 +769,8 @@ public abstract class Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      * @see #putFloat(String,float)
      * @see #get(String,String)
      */
@@ -770,6 +790,8 @@ public abstract class Preferences {
      *         <tt>MAX_KEY_LENGTH</tt>.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @see #getDouble(String,double)
      */
     public abstract void putDouble(String key, double value);
@@ -800,6 +822,8 @@ public abstract class Preferences {
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      * @see #putDouble(String,double)
      * @see #get(String,String)
      */
@@ -825,6 +849,8 @@ public abstract class Preferences {
      *         or if value.length exceeds MAX_VALUE_LENGTH*3/4.
      * @throws IllegalStateException if this node (or an ancestor) has been
      *         removed with the {@link #removeNode()} method.
+     * @throws IllegalArgumentException if key contains
+     *         the null control character, code point U+0000.
      * @see #getByteArray(String,byte[])
      * @see #get(String,String)
      */
@@ -864,6 +890,8 @@ public abstract class Preferences {
      *         removed with the {@link #removeNode()} method.
      * @throws NullPointerException if <tt>key</tt> is <tt>null</tt>.  (A
      *         <tt>null</tt> value for <tt>def</tt> <i>is</i> permitted.)
+     * @throws IllegalArgumentException if key contains the null control
+     *         character, code point U+0000.
      * @see #get(String,String)
      * @see #putByteArray(String,byte[])
      */

jdk/src/java.prefs/unix/classes/java/util/prefs/FileSystemPreferences.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -49,13 +49,6 @@ import sun.util.logging.PlatformLogger;
  */
 class FileSystemPreferences extends AbstractPreferences {
 
-    /**
-     * The code point U+0000, assigned to the null control character, is the
-     * only character encoded in Unicode and ISO/IEC 10646 that is always
-     * invalid in any XML 1.0 and 1.1 document.
-     */
-    private static final String CODE_POINT_U0000 = String.valueOf('\u0000');
-
     static {
         PrivilegedAction<Void> load = () -> {
             System.loadLibrary("prefs");
@@ -532,11 +525,6 @@ class FileSystemPreferences extends AbstractPreferences {
     }
 
     protected void putSpi(String key, String value) {
-        if (key.indexOf(CODE_POINT_U0000) != -1) {
-            throw new IllegalArgumentException("Key contains code point U+0000");
-        } else if (value.indexOf(CODE_POINT_U0000) != -1) {
-            throw new IllegalArgumentException("Value contains code point U+0000");
-        }
         initCacheIfNecessary();
         changeLog.add(new Put(key, value));
         prefsCache.put(key, value);

jdk/src/java.prefs/windows/classes/java/util/prefs/WindowsPreferences.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2002, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -25,8 +25,6 @@
 
 package java.util.prefs;
 
-import java.util.Map;
-import java.util.TreeMap;
 import java.util.StringTokenizer;
 import java.io.ByteArrayOutputStream;
 import java.security.AccessController;
@@ -46,7 +44,7 @@ import sun.util.logging.PlatformLogger;
  * @since 1.4
  */
 
-class WindowsPreferences extends AbstractPreferences{
+class WindowsPreferences extends AbstractPreferences {
 
     static {
         PrivilegedAction<Void> load = () -> {

jdk/test/java/util/prefs/CodePointZeroPrefsTest.java

@@ -26,9 +26,8 @@ import java.util.prefs.PreferencesFactory;
 
 /*
  * @test
- * @bug 8068373
+ * @bug 8068373 8075110 8075156
- * @requires os.family == "linux" | os.family == "solaris"
+ * @summary Ensure a code point U+0000 null control character is detected.
- * @summary Ensure writing a code point U+0000 null control character is detected.
  */
 public class CodePointZeroPrefsTest
 {
@@ -36,52 +35,70 @@ public class CodePointZeroPrefsTest
     {
         int failures = 0;
 
-        // Deliberately reflect so you can reproduce it on any platform.
+        Preferences node = Preferences.userRoot().node("com/acme/testing");
-        Constructor<? extends PreferencesFactory> constructor =
-            Class.forName("java.util.prefs.FileSystemPreferencesFactory").asSubclass(PreferencesFactory.class).getDeclaredConstructor();
-        constructor.setAccessible(true);
-        PreferencesFactory factory = constructor.newInstance();
 
-        Preferences node = factory.userRoot().node("com/acme/testing");
+        // --- put() ---
 
         // legal key and value
         try {
             node.put("a", "1");
         } catch (IllegalArgumentException iae) {
-            System.err.println("Unexpected IllegalArgumentException for legal key");
+            System.err.println("Unexpected IllegalArgumentException for legal put() key");
             failures++;
         }
 
         // illegal key only
-        int numIAEs = 0;
         try {
             node.put("a\u0000b", "1");
-            System.err.println("IllegalArgumentException not thrown for illegal key");
+            System.err.println("IllegalArgumentException not thrown for illegal put() key");
             failures++;
         } catch (IllegalArgumentException iae) {
             // do nothing
         }
 
         // illegal value only
-        numIAEs = 0;
         try {
             node.put("ab", "2\u00003");
-            System.err.println("IllegalArgumentException not thrown for illegal value");
+            System.err.println("IllegalArgumentException not thrown for illegal put() value");
             failures++;
         } catch (IllegalArgumentException iae) {
             // do nothing
         }
 
         // illegal key and value
-        numIAEs = 0;
         try {
             node.put("a\u0000b", "2\u00003");
-            System.err.println("IllegalArgumentException not thrown for illegal entry");
+            System.err.println("IllegalArgumentException not thrown for illegal put() entry");
             failures++;
         } catch (IllegalArgumentException iae) {
             // do nothing
         }
 
+        // --- get ---
+
+        // illegal key only
+        try {
+            String theDefault = "default";
+            String value = node.get("a\u0000b", theDefault);
+            System.err.println("IllegalArgumentException not thrown for illegal get() key");
+            failures++;
+        } catch (IllegalArgumentException iae) {
+            // do nothing
+        }
+
+        // --- remove ---
+
+        // illegal key only
+        try {
+            node.remove("a\u0000b");
+            System.err.println("IllegalArgumentException not thrown for illegal remove() key");
+            failures++;
+        } catch (IllegalArgumentException iae) {
+            // do nothing
+        }
+
+        node.removeNode();
+
         if (failures != 0) {
             throw new RuntimeException("CodePointZeroPrefsTest failed with "
                 + failures + " errors!");