8305529: DefaultProxySelector.select(URI) in certain cases returns a List with null element

jaikiran · jaikiran · commit 3f36dd811e56 · 2023-04-13T01:30:00.000Z
Reviewed-by: dfuchs, djelinski, michaelm
diff --git a/src/java.base/unix/native/libnet/DefaultProxySelector.c b/src/java.base/unix/native/libnet/DefaultProxySelector.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2023, 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
@@ -97,13 +97,15 @@ typedef GSocketConnectable* g_network_address_parse_uri_func();
 typedef const char* g_network_address_get_hostname_func();
 typedef unsigned short g_network_address_get_port_func();
 typedef void g_strfreev_func();
+typedef void g_clear_error_func();
 
 static g_proxy_resolver_get_default_func* g_proxy_resolver_get_default = NULL;
 static g_proxy_resolver_lookup_func* g_proxy_resolver_lookup = NULL;
 static g_network_address_parse_uri_func* g_network_address_parse_uri = NULL;
 static g_network_address_get_hostname_func* g_network_address_get_hostname = NULL;
 static g_network_address_get_port_func* g_network_address_get_port = NULL;
 static g_strfreev_func* g_strfreev = NULL;
+static g_clear_error_func* g_clear_error = NULL;
 
 static void* gconf_client = NULL;
 static int use_gproxyResolver = 0;
@@ -317,13 +319,16 @@ static int initGProxyResolver() {
 
     g_strfreev = (g_strfreev_func*)dlsym(gio_handle, "g_strfreev");
 
+    g_clear_error = (g_clear_error_func*)dlsym(gio_handle, "g_clear_error");
+
     if (!my_g_type_init_func ||
         !g_proxy_resolver_get_default ||
         !g_proxy_resolver_lookup ||
         !g_network_address_parse_uri ||
         !g_network_address_get_hostname ||
         !g_network_address_get_port ||
-        !g_strfreev)
+        !g_strfreev ||
+        !g_clear_error)
     {
         dlclose(gio_handle);
         return 0;
@@ -412,7 +417,13 @@ static jobjectArray getProxyByGProxyResolver(JNIEnv *env, const char *cproto,
                                     proxy_array = NULL;
                                     break;
                                 }
+                            } else {
+                                proxy_array = NULL;
+                                break;
                             }
+                        } else {
+                            proxy_array = NULL;
+                            break;
                         }
                     } else {
                         /* direct connection - no proxy */
@@ -432,6 +443,9 @@ static jobjectArray getProxyByGProxyResolver(JNIEnv *env, const char *cproto,
             }
         }
         (*g_strfreev)(proxies);
+         // as per API doc, g_clear_error doesn't complain if error is NULL, so it's safe to
+         // call without null checks
+        (*g_clear_error)(&error);
     }
 
     return proxy_array;
diff --git a/test/jdk/sun/net/spi/SystemProxyDriver.java b/test/jdk/sun/net/spi/SystemProxyDriver.java
@@ -0,0 +1,56 @@
+/*
+ * Copyright (c) 2023, 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.
+ */
+
+import jdk.test.lib.process.ProcessTools;
+
+/*
+ * @test
+ * @bug 8305529
+ * @summary Verifies that the sun.net.spi.DefaultProxySelector#select(URI) doesn't return a List
+ *          with null elements in it
+ * @modules java.base/sun.net.spi:+open
+ * @library /test/lib
+ * @build jdk.test.lib.process.ProcessTools SystemProxyTest
+ * @run driver SystemProxyDriver
+ */
+public class SystemProxyDriver {
+    // launches the SystemProxyTest as a separate process and verifies that the test passes
+    public static void main(final String[] args) throws Exception {
+        final String[] commandArgs = new String[]{
+                "--add-opens",
+                "java.base/sun.net.spi=ALL-UNNAMED",
+                // trigger use of the http_proxy environment variable that we pass when launching
+                // this Java program
+                "-Djava.net.useSystemProxies=true",
+                "SystemProxyTest"
+        };
+        final ProcessBuilder pb = ProcessTools.createTestJvm(commandArgs);
+        pb.inheritIO();
+        pb.environment().put("http_proxy", "foo://"); // intentionally use a value without host/port
+        final Process p = pb.start();
+        final int exitCode = p.waitFor();
+        if (exitCode != 0) {
+            throw new RuntimeException("Test failed, exitCode: " + exitCode);
+        }
+    }
+}
diff --git a/test/jdk/sun/net/spi/SystemProxyTest.java b/test/jdk/sun/net/spi/SystemProxyTest.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2023, 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.
+ */
+
+import java.net.Proxy;
+import java.net.ProxySelector;
+import java.net.URI;
+import java.util.List;
+
+import sun.net.spi.DefaultProxySelector;
+
+// this test is launched from SystemProxyDriver
+public class SystemProxyTest {
+
+    // calls the DefaultProxySelector.select(URI) and verifies that the returned List is
+    // not null, not empty and doesn't contain null elements.
+    public static void main(final String[] args) throws Exception {
+        final ProxySelector ps = new DefaultProxySelector();
+        final URI uri = new URI("http://example.com"); // the target URL doesn't matter
+        final List<Proxy> proxies = ps.select(uri);
+        if (proxies == null) {
+            // null isn't expected to be returned by the select() API
+            throw new AssertionError("DefaultProxySelector.select(URI) returned null for uri: "
+                    + uri);
+        }
+        if (proxies.isEmpty()) {
+            // empty list isn't expected to be returned by the select() API, instead when
+            // no proxy is configured, the returned list is expected to contain one entry with
+            // a Proxy instance representing direct connection
+            throw new AssertionError("DefaultProxySelector.select(URI) returned empty list" +
+                    " for uri: " + uri);
+        }
+        System.out.println("returned proxies list: " + proxies);
+        for (final Proxy p : proxies) {
+            if (p == null) {
+                throw new AssertionError("null proxy in proxies list for uri: " + uri);
+            }
+        }
+    }
+}
