Adding time limit to PAC script execution

naming changes

few sanitations

Changed default timeout and improved code

platform/core.network/src/org/netbeans/core/network/proxy/pac/impl/NbPacScriptEvaluator.java

@@ -26,6 +26,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.StringTokenizer;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
@@ -46,6 +47,9 @@
 import org.netbeans.core.network.proxy.pac.PacUtils;
 import org.openide.util.Lookup;
 import org.openide.util.NbBundle;
+import org.openide.util.RequestProcessor;
+import org.openide.util.RequestProcessor.Task;
+import org.netbeans.core.ProxySettings;
 
 /**
  * NetBeans implementation of a PAC script evaluator. This implementation
@@ -196,6 +200,8 @@ public class NbPacScriptEvaluator implements PacScriptEvaluator {
     private static final String PAC_SOCKS5_FFEXT = "SOCKS5"; // Mozilla Firefox extension. Not part of original Netscape spec.
     private static final String PAC_HTTP_FFEXT = "HTTP"; // Mozilla Firefox extension. Not part of original Netscape spec.
     private static final String PAC_HTTPS_FFEXT = "HTTPS"; // Mozilla Firefox extension. Not part of original Netscape spec.
+    private static class RPSingleton { private static final RequestProcessor instance = new RequestProcessor(NbPacScriptEvaluator.class.getName(), Runtime.getRuntime().availableProcessors(), true, false); }
+    private static RequestProcessor getRequestProcessor() { return RPSingleton.instance; }
     private final String pacScriptSource;
 
 
@@ -213,7 +219,7 @@ public NbPacScriptEvaluator(String pacSourceCocde) throws PacParsingException {
     @Override
     public List<Proxy> findProxyForURL(URI uri) throws PacValidationException {
 
-        List<Proxy> jsResultAnalyzed;
+        List<Proxy> jsResultAnalyzed = null;
 
         // First try the cache
         if (resultCache != null) {
@@ -222,38 +228,37 @@ public List<Proxy> findProxyForURL(URI uri) throws PacValidationException {
                 return jsResultAnalyzed;
             }
         }
-        try {
-            Object jsResult;
-            synchronized (scriptEngine) {
-                jsResult = scriptEngine.findProxyForURL(PacUtils.toStrippedURLStr(uri), uri.getHost());
-            }
-            jsResultAnalyzed = analyzeResult(uri, jsResult);
-            if (canUseURLCaching && (resultCache != null)) {
-                resultCache.put(uri, jsResultAnalyzed);   // save the result in the cache
-            }
-            return jsResultAnalyzed;
-        } catch (NoSuchMethodException ex) {
-            // If this exception occur at this time it is really, really unexpected.
-            // We already gave the function a test spin in the constructor.
-            Exceptions.printStackTrace(ex);
-            return Collections.singletonList(Proxy.NO_PROXY);
-        } catch (ScriptException ex) {
-            LOGGER.log(Level.WARNING, "Error when executing PAC script function " + scriptEngine.getJsMainFunction().getJsFunctionName() + " : ", ex);
-            return Collections.singletonList(Proxy.NO_PROXY);
-        } catch (Exception ex) {  // for runtime exceptions
-            if (ex.getCause() != null) {
-                if (ex.getCause() instanceof ClassNotFoundException) {
-                    // Is someone trying to break out of the sandbox ?
-                    LOGGER.log(Level.WARNING, "The downloaded PAC script is attempting to access Java class ''{0}'' which may be a sign of maliciousness. You should investigate this with your network administrator.", ex.getCause().getMessage());
-                    return Collections.singletonList(Proxy.NO_PROXY);
+
+        int timeout = ProxySettings.getPacScriptTimeout();
+
+        if (timeout <= 0){
+            jsResultAnalyzed = executeProxyScript(uri);
+        } else {
+            AtomicReference<List<Proxy>> resultHolder = new AtomicReference<>(null);
+            Task task = getRequestProcessor().post(() -> {
+                resultHolder.set(executeProxyScript(uri));
+            });
+
+            try{
+                if(!task.waitFinished(timeout)){
+                    LOGGER.log(Level.WARNING, "Timeout when executing PAC script function: {0}", scriptEngine.getJsMainFunction().getJsFunctionName());
+                }
+            } catch (InterruptedException ex) {
+                LOGGER.log(Level.WARNING, "PAC script execution interrupted: {0}", ex);
+            } finally {
+                if (!task.isFinished()) {
+                    // interruptThread is set true for the RequestProcessor so cancel will interrupt without any setting
+                    task.cancel();
                 }
             }
-            // other unforseen errors
-            LOGGER.log(Level.WARNING, "Error when executing PAC script function " + scriptEngine.getJsMainFunction().getJsFunctionName() + " : ", ex);
-            return Collections.singletonList(Proxy.NO_PROXY);
+            jsResultAnalyzed  = resultHolder.get();
         }
+        if (canUseURLCaching && (resultCache != null) && (jsResultAnalyzed != null)) {
+                resultCache.put(uri, jsResultAnalyzed);   // save the result in the cache
+        }
+        return jsResultAnalyzed != null ? jsResultAnalyzed : Collections.singletonList(Proxy.NO_PROXY);
     }
-
+    
     @Override
     public boolean usesCaching() {
         return (canUseURLCaching && (resultCache != null));
@@ -275,6 +280,32 @@ public String getPacScriptSource() {
         return this.pacScriptSource;
     }
 
+    private List<Proxy> executeProxyScript(URI uri) {
+        try{
+            Object jsResult;
+            synchronized (scriptEngine) {
+                jsResult = scriptEngine.findProxyForURL(PacUtils.toStrippedURLStr(uri), uri.getHost());
+            }
+            return analyzeResult(uri, jsResult);
+
+        } catch (NoSuchMethodException ex) {
+            // If this exception occur at this time it is really, really unexpected.
+            // We already gave the function a test spin in the constructor.
+            Exceptions.printStackTrace(ex);
+        } catch (ScriptException ex) {
+            LOGGER.log(Level.WARNING, "Error when executing PAC script function " + scriptEngine.getJsMainFunction().getJsFunctionName() + " : ", ex);
+        } catch (Exception ex) {  // for runtime exceptions
+            if (ex.getCause() != null) {
+                if (ex.getCause() instanceof ClassNotFoundException) {
+                    // Is someone trying to break out of the sandbox ?
+                    LOGGER.log(Level.WARNING, "The downloaded PAC script is attempting to access Java class ''{0}'' which may be a sign of maliciousness. You should investigate this with your network administrator.", ex.getCause().getMessage());
+                }
+            }
+            // other unforseen errors
+            LOGGER.log(Level.WARNING, "Error when executing PAC script function " + scriptEngine.getJsMainFunction().getJsFunctionName() + " : ", ex);
+        }
+        return null;
+    }
 
 
     private PacScriptEngine getScriptEngine(String pacSource) throws PacParsingException {

platform/core.network/test/unit/data/pacFiles2/pac-test-timeout.js

@@ -0,0 +1,30 @@
+/* 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+
+
+// 
+// A PAC script which takes long time to execute and wastes cpu resources
+//
+
+function FindProxyForURL(url, host)
+{
+    alert("pac-test-timeout.js");
+    const repeatedA = "A".repeat(999);
+    while(true){
+        console.log(repeatedA);
+    }
+    return "DIRECT";
+}

platform/core.network/test/unit/src/org/netbeans/core/network/proxy/pac/PacEngineTest.java

@@ -27,9 +27,12 @@
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.netbeans.core.ProxySettings;
+import static org.netbeans.core.ProxySettings.PAC_SCRIPT_TIMEOUT;
 import org.netbeans.core.network.proxy.pac.impl.NbPacScriptEvaluatorFactory;
 import org.netbeans.junit.NbModuleSuite;
 import org.netbeans.junit.NbTestCase;
+import org.openide.util.NbPreferences;
 
 /**
  *
@@ -73,6 +76,9 @@ public static final junit.framework.Test suite() {
     @Test
     public void testEngine() throws PacParsingException, IOException, URISyntaxException, PacValidationException {
         System.out.println("toSemiColonListStr");
+
+        NbPreferences.forModule(ProxySettings.class)
+                .putInt(PAC_SCRIPT_TIMEOUT, 2000);
 
         PacScriptEvaluatorFactory factory = new NbPacScriptEvaluatorFactory();
 
@@ -81,6 +87,7 @@ public void testEngine() throws PacParsingException, IOException, URISyntaxExcep
         testPacFile("pac-test3.js", factory, 1, false);
         testPacFileMalicious("pac-test-sandbox-breakout.js", factory);
         testPacFileMalicious("pac-test-getclass.js", factory);
+        testPacFileMalicious("pac-test-timeout.js", factory);
 
         testPacFile2("pac-test4.js", factory);
     }

platform/o.n.core/src/org/netbeans/core/ProxySettings.java

@@ -49,6 +49,8 @@ public class ProxySettings {
     public static final String USE_PROXY_ALL_PROTOCOLS = "useProxyAllProtocols";    // NOI18N
     public static final String DIRECT = "DIRECT";   // NOI18N
     public static final String PAC = "PAC";     // NOI18N
+    public static final String PAC_SCRIPT_TIMEOUT = "pacScriptTimeout"; // NOI18N
+    public static final int DEFAULT_TIMEOUT = 10000;
 
     public static final String SYSTEM_PROXY_HTTP_HOST = "systemProxyHttpHost";      // NOI18N
     public static final String SYSTEM_PROXY_HTTP_PORT = "systemProxyHttpPort";      // NOI18N
@@ -141,6 +143,10 @@ public static int getProxyType () {
         return type;
     }
 
+    public static int getPacScriptTimeout() {
+        return NbPreferences.forModule(ProxySettings.class)
+                .getInt(PAC_SCRIPT_TIMEOUT, DEFAULT_TIMEOUT);
+    }
 
     public static String getSystemHttpHost() {
         return getPreferences().get(SYSTEM_PROXY_HTTP_HOST, "");