8151154: IllegalArgumentException not thrown when wrong syntax value is set for javax.xml.catalog.files

Reviewed-by: lancea

jaxp/src/java.xml/share/classes/javax/xml/catalog/BaseEntry.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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
@@ -24,13 +24,9 @@
  */
 package javax.xml.catalog;
 
-import java.io.File;
 import java.net.MalformedURLException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.Objects;
-import jdk.xml.internal.SecuritySupport;
 
 /**
  * Represents a general Catalog entry.
@@ -237,18 +233,6 @@ abstract class BaseEntry {
         return url;
     }
 
-    /**
-     * Replace backslashes with forward slashes. (URLs always use forward
-     * slashes.)
-     *
-     * @param sysid The input system identifier.
-     * @return The same system identifier with backslashes turned into forward
-     * slashes.
-     */
-    protected String fixSlashes(String sysid) {
-        return sysid.replace('\\', '/');
-    }
-
     /**
      * Construct an absolute URI from a relative one, using the current base
      * URI.
@@ -260,7 +244,7 @@ abstract class BaseEntry {
     protected String makeAbsolute(String sysid) {
         URL local = null;
 
-        sysid = fixSlashes(sysid);
+        sysid = Util.fixSlashes(sysid);
         /**
          * try { local = new URL(base, sysid); } catch (MalformedURLException e)
          * { catalogManager.debug.message(1, "Malformed URL on system

jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogFeatures.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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
@@ -24,6 +24,10 @@
  */
 package javax.xml.catalog;
 
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
+import java.util.HashMap;
+import java.util.Map;
 import jdk.xml.internal.SecuritySupport;
 
 /**
@@ -380,10 +384,7 @@ public class CatalogFeatures {
      */
     CatalogFeatures(Builder builder) {
         init();
-        setProperty(Feature.FILES.ordinal(), State.APIPROPERTY, builder.files);
+        setProperties(builder);
-        setProperty(Feature.PREFER.ordinal(), State.APIPROPERTY, builder.prefer);
-        setProperty(Feature.DEFER.ordinal(), State.APIPROPERTY, builder.defer);
-        setProperty(Feature.RESOLVE.ordinal(), State.APIPROPERTY, builder.resolve);
     }
 
     /**
@@ -409,6 +410,15 @@ public class CatalogFeatures {
         readSystemProperties();
     }
 
+    /**
+     * Sets properties by the Builder.
+     * @param builder the CatalogFeatures builder
+     */
+    private void setProperties(Builder builder) {
+        builder.values.entrySet().stream().forEach((entry) -> {
+            setProperty(entry.getKey().ordinal(), State.APIPROPERTY, entry.getValue());
+        });
+    }
     /**
      * Sets the value of a property by its index, updates only if it shall override.
      *
@@ -432,11 +442,24 @@ public class CatalogFeatures {
                          && !value.equals(RESOLVE_IGNORE)) {
                     CatalogMessages.reportIAE(new Object[]{value, Feature.RESOLVE.name()}, null);
                 }
+            } else if (index == Feature.FILES.ordinal()) {
+                try {
+                    if (Util.verifyAndGetURI(value, null) == null) {
+                        CatalogMessages.reportIAE(new Object[]{value, Feature.FILES.name()}, null);
+                    }
+                }catch (MalformedURLException | URISyntaxException | IllegalArgumentException ex) {
+                    CatalogMessages.reportIAE(new Object[]{value, Feature.FILES.name()}, ex);
+                }
+
             }
             if (states[index] == null || state.compareTo(states[index]) >= 0) {
                 values[index] = value;
                 states[index] = state;
             }
+        } else {
+            if (state == State.SYSTEMPROPERTY || state == State.JAXPDOTPROPERTIES) {
+                CatalogMessages.reportIAE(new Object[]{value, Feature.values()[index].name()}, null);
+            }
         }
     }
 
@@ -486,9 +509,9 @@ public class CatalogFeatures {
      */
     public static class Builder {
         /**
-         * Variables for the features supported by CatalogFeatures.
+         * Values of the features supported by CatalogFeatures.
          */
-        String files, prefer, defer, resolve;
+        Map<Feature, String> values = new HashMap<>();
 
         /**
          * Instantiation of Builder is not allowed.
@@ -505,20 +528,10 @@ public class CatalogFeatures {
          * property
          */
         public Builder with(Feature feature, String value) {
-            switch (feature) {
+            if (value == null || value.length() == 0) {
-                case FILES :
+                CatalogMessages.reportIAE(new Object[]{value, feature.name()}, null);
-                    files = value;
-                    break;
-                case PREFER :
-                    prefer = value;
-                    break;
-                case DEFER :
-                    defer = value;
-                    break;
-                case RESOLVE :
-                    resolve = value;
-                    break;
             }
+            values.put(feature, value);
             return this;
         }
 

jaxp/src/java.xml/share/classes/javax/xml/catalog/CatalogImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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,7 +25,6 @@
 package javax.xml.catalog;
 
 import com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl;
-import java.io.File;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URI;
@@ -141,6 +140,12 @@ class CatalogImpl extends GroupEntry implements Catalog {
                 start++;
                 if (verifyCatalogFile(uri)) {
                     systemId = uri.toASCIIString();
+                    try {
+                        baseURI = new URL(systemId);
+                    } catch (MalformedURLException e) {
+                        CatalogMessages.reportRunTimeError(CatalogMessages.ERR_INVALID_PATH,
+                                new Object[]{temp}, e);
+                    }
                     break;
                 }
             }
@@ -291,59 +296,15 @@ class CatalogImpl extends GroupEntry implements Catalog {
      * to a system id
      */
     private URI getSystemId(String file) {
-        URL filepath;
+        URI temp = null;
-        if (file != null && file.length() > 0) {
+
         try {
-                File f = new File(file);
+            temp = Util.verifyAndGetURI(file, baseURI);
-                if (baseURI != null && !f.isAbsolute()) {
+        } catch (MalformedURLException | URISyntaxException | IllegalArgumentException e) {
-                    filepath = new URL(baseURI, fixSlashes(file));
-                    return filepath.toURI();
-                } else {
-                    return resolveURI(file);
-                }
-            } catch (MalformedURLException | URISyntaxException e) {
             CatalogMessages.reportRunTimeError(CatalogMessages.ERR_INVALID_PATH,
                     new Object[]{file}, e);
         }
-        }
-        return null;
-    }
 
-    /**
-     * Resolves the specified uri. If the uri is relative, makes it absolute by
-     * the user.dir directory.
-     *
-     * @param uri The specified URI.
-     * @return The resolved URI
-     */
-    private URI resolveURI(String uri) throws MalformedURLException {
-        if (uri == null) {
-            uri = "";
-        }
-
-        URI temp = toURI(uri);
-        String str = temp.toASCIIString();
-        String base = str.substring(0, str.lastIndexOf('/') + 1);
-        baseURI = new URL(str);
-
-        return temp;
-    }
-
-    /**
-     * Converts an URI string or file path to URI.
-     *
-     * @param uri an URI string or file path
-     * @return an URI
-     */
-    private URI toURI(String uri) {
-        URI temp = null;
-        try {
-            URL url = new URL(uri);
-            temp = url.toURI();
-        } catch (MalformedURLException | URISyntaxException mue) {
-            File file = new File(uri);
-            temp = file.toURI();
-        }
         return temp;
     }
 

jaxp/src/java.xml/share/classes/javax/xml/catalog/Util.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, 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
@@ -24,6 +24,13 @@
  */
 package javax.xml.catalog;
 
+import java.io.File;
+import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import jdk.xml.internal.SecuritySupport;
 
 /**
@@ -31,6 +38,76 @@ import jdk.xml.internal.SecuritySupport;
  * @since 9
  */
 class Util {
+
+    /**
+     * Resolves the specified file path to an absolute systemId. If it is
+     * relative, it shall be resolved using the base or user.dir property if
+     * base is not specified.
+     *
+     * @param file The specified file path
+     * @param baseURI the base URI
+     * @return The URI
+     * @throws CatalogException if the specified file path can not be converted
+     * to a system id
+     */
+    static URI verifyAndGetURI(String file, URL baseURI)
+            throws MalformedURLException, URISyntaxException, IllegalArgumentException {
+        URL filepath;
+        URI temp;
+        if (file != null && file.length() > 0) {
+            File f = new File(file);
+
+            if (baseURI != null && !f.isAbsolute()) {
+                filepath = new URL(baseURI, fixSlashes(file));
+                temp = filepath.toURI();
+            } else {
+                temp = resolveURI(file);
+            }
+            //Paths.get may throw IllegalArgumentException
+            Path path = Paths.get(temp);
+            if (path.toFile().isFile()) {
+                return temp;
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Resolves the specified uri. If the uri is relative, makes it absolute by
+     * the user.dir directory.
+     *
+     * @param uri The specified URI.
+     * @return The resolved URI
+     */
+    static URI resolveURI(String uri) throws MalformedURLException {
+        if (uri == null) {
+            uri = "";
+        }
+
+        URI temp = null;
+        try {
+            URL url = new URL(uri);
+            temp = url.toURI();
+        } catch (MalformedURLException | URISyntaxException mue) {
+            File file = new File(uri);
+            temp = file.toURI();
+        }
+
+        return temp;
+    }
+
+    /**
+     * Replace backslashes with forward slashes. (URLs always use forward
+     * slashes.)
+     *
+     * @param sysid The input system identifier.
+     * @return The same system identifier with backslashes turned into forward
+     * slashes.
+     */
+    static String fixSlashes(String sysid) {
+        return sysid.replace('\\', '/');
+    }
+
     /**
      * Find catalog file paths by reading the system property, and then
      * jaxp.properties if the system property is not specified.
@@ -38,7 +115,7 @@ class Util {
      * @param sysPropertyName the name of system property
      * @return the catalog file paths, or null if not found.
      */
-    static public String[] getCatalogFiles(String sysPropertyName) {
+    static String[] getCatalogFiles(String sysPropertyName) {
         String value = SecuritySupport.getJAXPSystemProperty(sysPropertyName);
         if (value != null && !value.equals("")) {
             return value.split(";");

jaxp/test/javax/xml/jaxp/unittest/catalog/CatalogTest.java

@@ -42,11 +42,23 @@ import org.xml.sax.XMLReader;
 import org.xml.sax.ext.DefaultHandler2;
 
 /*
- * @bug 8081248, 8144966, 8146606, 8146237
+ * @bug 8081248, 8144966, 8146606, 8146237, 8151154
  * @summary Tests basic Catalog functions.
  */
-
 public class CatalogTest {
+    /**
+     * @bug 8151154
+     * Verifies that the CatalogFeatures' builder throws IllegalArgumentException
+     * on invalid file inputs.
+     * @param file the file path
+     */
+    @Test(dataProvider = "invalidPaths", expectedExceptions = IllegalArgumentException.class)
+    public void testFileInput(String file) {
+            CatalogFeatures features = CatalogFeatures.builder()
+                .with(CatalogFeatures.Feature.FILES, file)
+                .build();
+    }
+
     /**
      * @bug 8146237
      * PREFER from Features API taking precedence over catalog file
@@ -201,6 +213,24 @@ public class CatalogTest {
         }
     }
 
+    /*
+       DataProvider: for testing the verification of file paths by
+                     the CatalogFeatures builder
+     */
+    @DataProvider(name = "invalidPaths")
+    Object[][] getFiles() {
+        return new Object[][]{
+            {null},
+            {""},
+            {"file:a/b\\c"},
+            {"file:/../../.."},
+            {"c:/te:t"},
+            {"c:/te?t"},
+            {"c/te*t"},
+            {"in|valid.txt"},
+            {"shema:invalid.txt"},
+        };
+    }
 
     /*
        DataProvider: provides test name, expected string, the catalog, and XML