8230407: SocketPermission and FilePermission action list allows leading comma
authorigerasim
Wed, 16 Oct 2019 14:32:17 -0700
changeset 58653 71fef5fae9cc
parent 58652 9b67dd88a931
child 58654 562bf1878089
8230407: SocketPermission and FilePermission action list allows leading comma Reviewed-by: chegar Contributed-by: Ivan Gerasimov <ivan.gerasimov@oracle.com>, Chris Hegarty <chris.hegarty@oracle.com>
src/java.base/share/classes/java/io/FilePermission.java
src/java.base/share/classes/java/net/SocketPermission.java
test/jdk/java/io/FilePermission/SpecTests.java
test/jdk/java/net/SocketPermission/Ctor.java
--- a/src/java.base/share/classes/java/io/FilePermission.java	Mon Oct 14 18:48:10 2019 -0700
+++ b/src/java.base/share/classes/java/io/FilePermission.java	Wed Oct 16 14:32:17 2019 -0700
@@ -480,9 +480,9 @@
      * @param path the pathname of the file/directory.
      * @param actions the action string.
      *
-     * @throws IllegalArgumentException
-     *          If actions is {@code null}, empty or contains an action
-     *          other than the specified possible actions.
+     * @throws IllegalArgumentException if actions is {@code null}, empty,
+     *         malformed or contains an action other than the specified
+     *         possible actions
      */
     public FilePermission(String path, String actions) {
         super(path);
@@ -935,17 +935,18 @@
             }
 
             // make sure we didn't just match the tail of a word
-            // like "ackbarfaccept".  Also, skip to the comma.
+            // like "ackbarfdelete".  Also, skip to the comma.
             boolean seencomma = false;
             while (i >= matchlen && !seencomma) {
-                switch(a[i-matchlen]) {
-                case ',':
-                    seencomma = true;
-                    break;
+                switch (c = a[i-matchlen]) {
                 case ' ': case '\r': case '\n':
                 case '\f': case '\t':
                     break;
                 default:
+                    if (c == ',' && i > matchlen) {
+                        seencomma = true;
+                        break;
+                    }
                     throw new IllegalArgumentException(
                             "invalid permission: " + actions);
                 }
@@ -1141,10 +1142,10 @@
      *
      * @param permission the Permission object to add.
      *
-     * @throws    IllegalArgumentException - if the permission is not a
+     * @throws    IllegalArgumentException   if the permission is not a
      *                                       FilePermission
      *
-     * @throws    SecurityException - if this FilePermissionCollection object
+     * @throws    SecurityException   if this FilePermissionCollection object
      *                                has been marked readonly
      */
     @Override
--- a/src/java.base/share/classes/java/net/SocketPermission.java	Mon Oct 14 18:48:10 2019 -0700
+++ b/src/java.base/share/classes/java/net/SocketPermission.java	Wed Oct 16 14:32:17 2019 -0700
@@ -287,6 +287,11 @@
      * @param host the hostname or IP address of the computer, optionally
      * including a colon followed by a port or port range.
      * @param action the action string.
+     *
+     * @throws NullPointerException if any parameters are null
+     * @throws IllegalArgumentException if the format of {@code host} is
+     *         invalid, or if the {@code action} string is empty, malformed, or
+     *         contains an action other than the specified possible actions
      */
     public SocketPermission(String host, String action) {
         super(getHost(host));
@@ -589,14 +594,15 @@
             // like "ackbarfaccept".  Also, skip to the comma.
             boolean seencomma = false;
             while (i >= matchlen && !seencomma) {
-                switch(a[i-matchlen]) {
-                case ',':
-                    seencomma = true;
-                    break;
+                switch (c = a[i-matchlen]) {
                 case ' ': case '\r': case '\n':
                 case '\f': case '\t':
                     break;
                 default:
+                    if (c == ',' && i > matchlen) {
+                        seencomma = true;
+                        break;
+                    }
                     throw new IllegalArgumentException(
                             "invalid permission: " + action);
                 }
--- a/test/jdk/java/io/FilePermission/SpecTests.java	Mon Oct 14 18:48:10 2019 -0700
+++ b/test/jdk/java/io/FilePermission/SpecTests.java	Wed Oct 16 14:32:17 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2019, 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
@@ -22,11 +22,10 @@
  */
 
 /**
- *
  * @test
- * @bug 4955804
- * @summary Tests for FilePermission constructor spec for null
- *      and empty String parameters
+ * @bug 4955804 8230407
+ * @summary Tests for FilePermission constructor spec for null,
+ *      empty and misformated String parameters
  */
 
 import java.io.*;
@@ -37,10 +36,11 @@
         String ILE = "java.lang.IllegalArgumentException";
         String NPE = "java.lang.NullPointerException";
 
-        String names[] =   {"", null, "foo", "foo", "foo", "foo"};
+        String names[] =   {"", null, "foo", "foo", "foo", "foo", "foo"};
         String actions[] = {"read", "read", "", null, "junk",
-                         "read,write,execute,delete,rename"};
-        String exps[] = { null, NPE, ILE, ILE, ILE, ILE };
+                            "read,write,execute,delete,rename",
+                            ",read"};
+        String exps[] = { null, NPE, ILE, ILE, ILE, ILE, ILE };
 
         FilePermission permit;
         for (int i = 0; i < names.length; i++) {
@@ -54,15 +54,19 @@
                                         " for name:" + names[i] +
                                         " actions:" + actions[i]);
                 } else {
-                   System.out.println(names[i] + ", [" + actions[i] + "] " +
-                         "resulted in " + exps[i] + " as Expected");
+                    System.out.println(names[i] + ", [" + actions[i] + "] " +
+                            "resulted in " + exps[i] + " as Expected");
+                    continue;
                 }
-           }
-           if (exps[i] == null) {
+            }
+            if (exps[i] == null) {
                 System.out.println(names[i] + ", [" + actions[i] + "] " +
-                         "resulted in No Exception as Expected");
+                        "resulted in No Exception as Expected");
+            } else {
+                throw new Exception("Expecting: " + exps[i] +
+                                    " for name:" + names[i] +
+                                    " actions:" + actions[i]);
             }
         }
-
     }
 }
--- a/test/jdk/java/net/SocketPermission/Ctor.java	Mon Oct 14 18:48:10 2019 -0700
+++ b/test/jdk/java/net/SocketPermission/Ctor.java	Wed Oct 16 14:32:17 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, 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
@@ -23,19 +23,64 @@
 
 /*
  * @test
- * @bug 4391898
+ * @bug 4391898 8230407
  * @summary SocketPermission(":",...) throws ArrayIndexOutOfBoundsException
+ *          SocketPermission constructor argument checks
+ * @run testng Ctor
  */
 
-import java.net.*;
+import java.net.SocketPermission;
+import org.testng.annotations.Test;
+import static java.lang.System.out;
+import static org.testng.Assert.*;
 
 public class Ctor {
-    public static void main(String[] args) {
-        try {
-            SocketPermission sp = new java.net.SocketPermission(":", "connect");
-        } catch (java.lang.ArrayIndexOutOfBoundsException e) {
-            throw new RuntimeException(e);
-        }
-        System.out.println("Test passed!!!");
+
+    static final Class<NullPointerException> NPE = NullPointerException.class;
+    static final Class<IllegalArgumentException> IAE = IllegalArgumentException.class;
+
+    @Test
+    public void positive() {
+        // ArrayIndexOutOfBoundsException is the bug, 4391898, exists
+        SocketPermission sp1 =  new SocketPermission(":", "connect");
+    }
+
+    @Test
+    public void npe() {
+        NullPointerException e;
+        e = expectThrows(NPE, () -> new SocketPermission(null, null));
+        out.println("caught expected NPE: " + e);
+        e = expectThrows(NPE, () -> new SocketPermission("foo", null));
+        out.println("caught expected NPE: " + e);
+        e = expectThrows(NPE, () -> new SocketPermission(null, "connect"));
+        out.println("caught expected NPE: " + e);
+    }
+
+    @Test
+    public void iae() {
+        IllegalArgumentException e;
+        // host
+        e = expectThrows(IAE, () -> new SocketPermission("1:2:3:4", "connect"));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo:5-4", "connect"));
+        out.println("caught expected IAE: " + e);
+
+        // actions
+        e = expectThrows(IAE, () -> new SocketPermission("foo", ""));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo", "badAction"));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo", "badAction,connect"));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo", "badAction,,connect"));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo", ",connect"));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo", ",,connect"));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo", "connect,"));
+        out.println("caught expected IAE: " + e);
+        e = expectThrows(IAE, () -> new SocketPermission("foo", "connect,,"));
+        out.println("caught expected IAE: " + e);
     }
 }