8242330: Arrays should be cloned in several JAAS Callback classes

Reviewed-by: mullan

Author: wangweij

src/java.base/share/classes/javax/security/auth/callback/ChoiceCallback.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2020, 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
@@ -43,23 +43,23 @@ public class ChoiceCallback implements Callback, java.io.Serializable {
      * @serial
      * @since 1.4
      */
-    private String prompt;
+    private final String prompt;
     /**
      * @serial the list of choices
      * @since 1.4
      */
-    private String[] choices;
+    private final String[] choices;
     /**
      * @serial the choice to be used as the default choice
      * @since 1.4
      */
-    private int defaultChoice;
+    private final int defaultChoice;
     /**
      * @serial whether multiple selections are allowed from the list of
      * choices
      * @since 1.4
      */
-    private boolean multipleSelectionsAllowed;
+    private final boolean multipleSelectionsAllowed;
     /**
      * @serial the selected choices, represented as indexes into the
      *          {@code choices} list.
@@ -75,7 +75,8 @@ public class ChoiceCallback implements Callback, java.io.Serializable {
      *
      * @param prompt the prompt used to describe the list of choices.
      *
-     * @param choices the list of choices.
+     * @param choices the list of choices. The array is cloned to protect
+     *                  against subsequent modification.
      *
      * @param defaultChoice the choice to be used as the default choice
      *                  when the list of choices are displayed.  This value
@@ -110,7 +111,7 @@ public ChoiceCallback(String prompt, String[] choices,
         }
 
         this.prompt = prompt;
-        this.choices = choices;
+        this.choices = choices.clone();
         this.defaultChoice = defaultChoice;
         this.multipleSelectionsAllowed = multipleSelectionsAllowed;
     }
@@ -127,10 +128,10 @@ public String getPrompt() {
     /**
      * Get the list of choices.
      *
-     * @return the list of choices.
+     * @return a copy of the list of choices.
      */
     public String[] getChoices() {
-        return choices;
+        return choices.clone();
     }
 
     /**
@@ -170,7 +171,8 @@ public void setSelectedIndex(int selection) {
      * Set the selected choices.
      *
      * @param selections the selections represented as indexes into the
-     *          {@code choices} list.
+     *          {@code choices} list. The array is cloned to protect
+     *          against subsequent modification.
      *
      * @exception UnsupportedOperationException if multiple selections are
      *          not allowed, as determined by
@@ -181,18 +183,18 @@ public void setSelectedIndex(int selection) {
     public void setSelectedIndexes(int[] selections) {
         if (!multipleSelectionsAllowed)
             throw new UnsupportedOperationException();
-        this.selections = selections;
+        this.selections = selections == null ? null : selections.clone();
     }
 
     /**
      * Get the selected choices.
      *
-     * @return the selected choices, represented as indexes into the
+     * @return a copy of the selected choices, represented as indexes into the
      *          {@code choices} list.
      *
      * @see #setSelectedIndexes
      */
     public int[] getSelectedIndexes() {
-        return selections;
+        return selections == null ? null : selections.clone();
     }
 }

src/java.base/share/classes/javax/security/auth/callback/ConfirmationCallback.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2020, 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
@@ -122,31 +122,32 @@ public class ConfirmationCallback implements Callback, java.io.Serializable {
 
     /** ERROR message type. */
     public static final int ERROR                       = 2;
+
     /**
      * @serial
      * @since 1.4
      */
-    private String prompt;
+    private final String prompt;
     /**
      * @serial
      * @since 1.4
      */
-    private int messageType;
+    private final int messageType;
     /**
      * @serial
      * @since 1.4
      */
-    private int optionType = UNSPECIFIED_OPTION;
+    private final int optionType;
     /**
      * @serial
      * @since 1.4
      */
-    private int defaultOption;
+    private final int defaultOption;
     /**
      * @serial
      * @since 1.4
      */
-    private String[] options;
+    private final String[] options;
     /**
      * @serial
      * @since 1.4
@@ -206,8 +207,10 @@ public ConfirmationCallback(int messageType,
             break;
         }
 
+        this.prompt = null;
         this.messageType = messageType;
         this.optionType = optionType;
+        this.options = null;
         this.defaultOption = defaultOption;
     }
 
@@ -225,7 +228,8 @@ public ConfirmationCallback(int messageType,
      * @param messageType the message type ({@code INFORMATION},
      *                  {@code WARNING} or {@code ERROR}).
      *
-     * @param options the list of confirmation options.
+     * @param options the list of confirmation options. The array is cloned
+     *                  to protect against subsequent modification.
      *
      * @param defaultOption the default option, represented as an index
      *                  into the {@code options} array.
@@ -253,8 +257,10 @@ public ConfirmationCallback(int messageType,
                 throw new IllegalArgumentException();
         }
 
+        this.prompt = null;
         this.messageType = messageType;
-        this.options = options;
+        this.optionType = UNSPECIFIED_OPTION;
+        this.options = options.clone();
         this.defaultOption = defaultOption;
     }
 
@@ -319,6 +325,7 @@ public ConfirmationCallback(String prompt, int messageType,
         this.prompt = prompt;
         this.messageType = messageType;
         this.optionType = optionType;
+        this.options = null;
         this.defaultOption = defaultOption;
     }
 
@@ -338,7 +345,8 @@ public ConfirmationCallback(String prompt, int messageType,
      * @param messageType the message type ({@code INFORMATION},
      *                  {@code WARNING} or {@code ERROR}).
      *
-     * @param options the list of confirmation options.
+     * @param options the list of confirmation options. The array is cloned
+     *                  to protect against subsequent modification.
      *
      * @param defaultOption the default option, represented as an index
      *                  into the {@code options} array.
@@ -371,7 +379,8 @@ public ConfirmationCallback(String prompt, int messageType,
 
         this.prompt = prompt;
         this.messageType = messageType;
-        this.options = options;
+        this.optionType = UNSPECIFIED_OPTION;
+        this.options = options.clone();
         this.defaultOption = defaultOption;
     }
 
@@ -418,12 +427,12 @@ public int getOptionType() {
     /**
      * Get the confirmation options.
      *
-     * @return the list of confirmation options, or null if this
+     * @return a copy of the list of confirmation options, or null if this
      *          {@code ConfirmationCallback} was instantiated with
      *          an {@code optionType} instead of {@code options}.
      */
     public String[] getOptions() {
-        return options;
+        return options == null ? null : options.clone();
     }
 
     /**

test/jdk/javax/security/auth/callback/Mutability.java

@@ -0,0 +1,96 @@
+/*
+ * Copyright (c) 2020, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8242330
+ * @library /test/lib
+ * @summary Arrays should be cloned in several JAAS Callback classes
+ */
+
+import javax.security.auth.callback.ChoiceCallback;
+import javax.security.auth.callback.ConfirmationCallback;
+
+import static jdk.test.lib.Asserts.assertEQ;
+
+public class Mutability {
+    public static void main(String[] args) {
+
+        // #1. ConfirmationCallback.new(3)
+        String[] i11 = {"1", "2"};
+        ConfirmationCallback c1 = new ConfirmationCallback(
+                ConfirmationCallback.INFORMATION,
+                i11,
+                0);
+
+        // Modify argument of constructor
+        i11[0] = "x";
+        String[] o11 = c1.getOptions();
+        assertEQ(o11[0], "1");
+        // Modify output
+        o11[0] = "y";
+        String[] o12 = c1.getOptions();
+        assertEQ(o12[0], "1");
+
+        // #2. ConfirmationCallback.new(4)
+        String[] i21 = {"1", "2"};
+        ConfirmationCallback c2 = new ConfirmationCallback(
+                "Hi",
+                ConfirmationCallback.INFORMATION,
+                i21,
+                0);
+
+        // Modify argument of constructor
+        i21[0] = "x";
+        assertEQ(c2.getOptions()[0], "1");
+
+        // #3. ChoiceCallback.new
+        String[] i31 = {"1", "2"};
+        ChoiceCallback c3 = new ChoiceCallback(
+                "Hi",
+                i31,
+                0,
+                true);
+
+        // Modify argument of constructor
+        i31[0] = "x";
+        String[] o31 = c3.getChoices();
+        assertEQ(o31[0], "1");
+        // Modify output of getChoices
+        o31[0] = "y";
+        String[] o32 = c3.getChoices();
+        assertEQ(o32[0], "1");
+
+        int[] s31 = {0, 1};
+        c3.setSelectedIndexes(s31);
+
+        // Modify argument of setSelectedIndexes
+        s31[0] = 1;
+        int[] s32 = c3.getSelectedIndexes();
+        assertEQ(s32[0], 0);
+        // Modify output of getSelectedIndexes
+        s32[1] = 0;
+        int[] s33 = c3.getSelectedIndexes();
+        assertEQ(s33[1], 1);
+    }
+}