8015345: Function("}),print('test'),({") should throw SyntaxError

Reviewed-by: lagergren, hannesw, jlaskey

src/jdk/nashorn/internal/objects/NativeFunction.java

@@ -33,10 +33,14 @@ import jdk.nashorn.internal.objects.annotations.Attribute;
 import jdk.nashorn.internal.objects.annotations.Constructor;
 import jdk.nashorn.internal.objects.annotations.Function;
 import jdk.nashorn.internal.objects.annotations.ScriptClass;
+import jdk.nashorn.internal.parser.Parser;
+import jdk.nashorn.internal.runtime.Context;
 import jdk.nashorn.internal.runtime.JSType;
+import jdk.nashorn.internal.runtime.ParserException;
 import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
+import jdk.nashorn.internal.runtime.Source;
 
 /**
  * ECMA 15.3 Function Objects
@@ -187,16 +191,25 @@ public final class NativeFunction {
 
         sb.append("(function (");
         if (args.length > 0) {
+            final StringBuilder paramListBuf = new StringBuilder();
             for (int i = 0; i < args.length - 1; i++) {
-                sb.append(JSType.toString(args[i]));
+                paramListBuf.append(JSType.toString(args[i]));
                 if (i < args.length - 2) {
-                    sb.append(",");
+                    paramListBuf.append(",");
                 }
             }
+
+            final String paramList = paramListBuf.toString();
+            if (! paramList.isEmpty()) {
+                checkFunctionParameters(paramList);
+                sb.append(paramList);
+            }
         }
         sb.append(") {\n");
         if (args.length > 0) {
-            sb.append(JSType.toString(args[args.length - 1]));
+            final String funcBody = JSType.toString(args[args.length - 1]);
+            checkFunctionBody(funcBody);
+            sb.append(funcBody);
             sb.append('\n');
         }
         sb.append("})");
@@ -205,4 +218,24 @@ public final class NativeFunction {
 
         return Global.directEval(global, sb.toString(), global, "<function>", Global.isStrict());
     }
+
+    private static void checkFunctionParameters(final String params) {
+        final Source src = new Source("<function>", params);
+        final Parser parser = new Parser(Global.getEnv(), src, new Context.ThrowErrorManager());
+        try {
+            parser.parseFormalParameterList();
+        } catch (final ParserException pe) {
+            pe.throwAsEcmaException();
+        }
+    }
+
+    private static void checkFunctionBody(final String funcBody) {
+        final Source src = new Source("<function>", funcBody);
+        final Parser parser = new Parser(Global.getEnv(), src, new Context.ThrowErrorManager());
+        try {
+            parser.parseFunctionBody();
+        } catch (final ParserException pe) {
+            pe.throwAsEcmaException();
+        }
+    }
 }

src/jdk/nashorn/internal/parser/Parser.java

@@ -192,36 +192,110 @@ public class Parser extends AbstractParser {
             // Begin parse.
             return program(scriptName);
         } catch (final Exception e) {
-            // Extract message from exception.  The message will be in error
-            // message format.
-            String message = e.getMessage();
-
-            // If empty message.
-            if (message == null) {
-                message = e.toString();
-            }
+            handleParseException(e);
 
-            // Issue message.
-            if (e instanceof ParserException) {
-                errors.error((ParserException)e);
+            return null;
+        } finally {
+            final String end = this + " end '" + scriptName + "'";
+            if (Timing.isEnabled()) {
+                Timing.accumulateTime(toString(), System.currentTimeMillis() - t0);
+                LOG.info(end, "' in ", (System.currentTimeMillis() - t0), " ms");
             } else {
-                errors.error(message);
+                LOG.info(end);
             }
+        }
+    }
 
-            if (env._dump_on_error) {
-                e.printStackTrace(env.getErr());
-            }
+    /**
+     * Parse and return the list of function parameter list. A comma
+     * separated list of function parameter identifiers is expected to be parsed.
+     * Errors will be thrown and the error manager will contain information
+     * if parsing should fail. This method is used to check if parameter Strings
+     * passed to "Function" constructor is a valid or not.
+     *
+     * @return the list of IdentNodes representing the formal parameter list
+     */
+    public List<IdentNode> parseFormalParameterList() {
+        try {
+            stream = new TokenStream();
+            lexer  = new Lexer(source, stream, scripting && !env._no_syntax_extensions);
+
+            // Set up first token (skips opening EOL.)
+            k = -1;
+            next();
 
+            return formalParameterList(TokenType.EOF);
+        } catch (final Exception e) {
+            handleParseException(e);
             return null;
-         } finally {
-             final String end = this + " end '" + scriptName + "'";
-             if (Timing.isEnabled()) {
-                 Timing.accumulateTime(toString(), System.currentTimeMillis() - t0);
-                 LOG.info(end, "' in ", (System.currentTimeMillis() - t0), " ms");
-             } else {
-                 LOG.info(end);
-             }
-         }
+        }
+    }
+
+    /**
+     * Execute parse and return the resulting function node.
+     * Errors will be thrown and the error manager will contain information
+     * if parsing should fail. This method is used to check if code String
+     * passed to "Function" constructor is a valid function body or not.
+     *
+     * @return function node resulting from successful parse
+     */
+    public FunctionNode parseFunctionBody() {
+        try {
+            stream = new TokenStream();
+            lexer  = new Lexer(source, stream, scripting && !env._no_syntax_extensions);
+
+            // Set up first token (skips opening EOL.)
+            k = -1;
+            next();
+
+            // Make a fake token for the function.
+            final long functionToken = Token.toDesc(FUNCTION, 0, source.getLength());
+            // Set up the function to append elements.
+
+            FunctionNode function = newFunctionNode(
+                functionToken,
+                new IdentNode(functionToken, Token.descPosition(functionToken), RUN_SCRIPT.symbolName()),
+                new ArrayList<IdentNode>(),
+                FunctionNode.Kind.NORMAL);
+
+            functionDeclarations = new ArrayList<>();
+            sourceElements();
+            addFunctionDeclarations(function);
+            functionDeclarations = null;
+
+            expect(EOF);
+
+            function.setFinish(source.getLength() - 1);
+
+            function = restoreFunctionNode(function, token); //commit code
+            function = function.setBody(lc, function.getBody().setNeedsScope(lc));
+            return function;
+        } catch (final Exception e) {
+            handleParseException(e);
+            return null;
+        }
+    }
+
+    private void handleParseException(final Exception e) {
+        // Extract message from exception.  The message will be in error
+        // message format.
+        String message = e.getMessage();
+
+        // If empty message.
+        if (message == null) {
+            message = e.toString();
+        }
+
+        // Issue message.
+        if (e instanceof ParserException) {
+            errors.error((ParserException)e);
+        } else {
+            errors.error(message);
+        }
+
+        if (env._dump_on_error) {
+            e.printStackTrace(env.getErr());
+        }
     }
 
     /**
@@ -2424,12 +2498,29 @@ loop:
      * @return List of parameter nodes.
      */
     private List<IdentNode> formalParameterList() {
+        return formalParameterList(RPAREN);
+    }
+
+    /**
+     * Same as the other method of the same name - except that the end
+     * token type expected is passed as argument to this method.
+     *
+     * FormalParameterList :
+     *      Identifier
+     *      FormalParameterList , Identifier
+     *
+     * See 13
+     *
+     * Parse function parameter list.
+     * @return List of parameter nodes.
+     */
+    private List<IdentNode> formalParameterList(final TokenType endType) {
         // Prepare to gather parameters.
         final List<IdentNode> parameters = new ArrayList<>();
         // Track commas.
         boolean first = true;
 
-        while (type != RPAREN) {
+        while (type != endType) {
             // Comma prior to every argument except the first.
             if (!first) {
                 expect(COMMARIGHT);

test/script/basic/JDK-8015345.js

+/*
+ * Copyright (c) 2010, 2013, 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.
+ */
+
+/**
+ * JDK-8015345: Function("}),print('test'),({") should throw SyntaxError
+ *
+ * @test
+ * @run
+ */
+
+function checkFunction(code) {
+    try {
+        Function(code);
+        fail("should have thrown SyntaxError for :" + code);
+    } catch (e) {
+        if (! (e instanceof SyntaxError)) {
+            fail("SyntaxError expected, but got " + e);
+        }
+        print(e);
+    }
+}
+
+// invalid body
+checkFunction("}),print('test'),({");
+
+// invalid param list
+checkFunction("x**y", "print('x')");
+
+// invalid param identifier
+checkFunction("in", "print('hello')");
+//checkFunction("<>", "print('hello')")
+
+// invalid param list and body
+checkFunction("x--y", ")");
+
+// check few valid cases as well
+var f = Function("x", "return x*x");
+print(f(10))
+
+f = Function("x", "y", "return x+y");
+print(f(33, 22));
+
+f = Function("x,y", "return x/y");
+print(f(24, 2));

test/script/basic/JDK-8015345.js.EXPECTED

+SyntaxError: <function>:1:0 Expected eof but found }
+}),print('test'),({
+^
+SyntaxError: <function>:1:2 Expected an operand but found *
+x**y
+  ^
+SyntaxError: <function>:1:0 Expected an operand but found in
+in
+^
+SyntaxError: <function>:1:3 Expected ; but found y
+x--y
+   ^
+100
+55
+12

test/script/basic/funcconstructor.js.EXPECTED

@@ -4,7 +4,7 @@ function (x) {
 print('anon func'); return x*x;
 }
 syntax error? true
-SyntaxError: <function>:2:13 Missing close quote
+SyntaxError: <function>:1:13 Missing close quote
 print('hello)
              ^
 done