Fixes #4728. Parser does not seem to terminate when parsing this string.

jruby · Aug 8, 2017 · c59be67 · c59be67
1 parent 33c6439
commit c59be67
Showing 4 changed files with 97 additions and 26 deletions.
diff --git a/core/src/main/java/org/jruby/ext/ripper/RipperLexer.java b/core/src/main/java/org/jruby/ext/ripper/RipperLexer.java
@@ -1040,7 +1040,7 @@ private int yylex() throws IOException {
             }
             case '#': { /* it's a comment */
                 this.tokenSeen = tokenSeen;
-                if (!parseMagicComment(getRuntime(), lexb.makeShared(lex_p, lex_pend - lex_p))) {
+                if (!parser_magic_comment(lexb.makeShared(lex_p, lex_pend - lex_p))) {
                     if (comment_at_top()) set_file_encoding(lex_p, lex_pend);
                 }
                 lex_p = lex_pend;

diff --git a/core/src/main/java/org/jruby/lexer/LexingCommon.java b/core/src/main/java/org/jruby/lexer/LexingCommon.java
@@ -13,6 +13,7 @@
 import org.jruby.Ruby;
 import org.jruby.RubyEncoding;
 import org.jruby.RubyRegexp;
+import org.jruby.javasupport.ext.JavaLang;
 import org.jruby.lexer.yacc.ISourcePosition;
 import org.jruby.lexer.yacc.SimpleSourcePosition;
 import org.jruby.lexer.yacc.StackState;
@@ -815,41 +816,101 @@ public static int magicCommentMarker(ByteList str, int begin) {
     public static final String magicString = "^[^\\S]*([^\\s\'\":;]+)\\s*:\\s*(\"(?:\\\\.|[^\"])*\"|[^\"\\s;]+)[\\s;]*[^\\S]*$";
     public static final Regex magicRegexp = new Regex(magicString.getBytes(), 0, magicString.length(), 0, Encoding.load("ASCII"));
 
-
-    // MRI: parser_magic_comment
-    public boolean parseMagicComment(Ruby runtime, ByteList magicLine) throws IOException {
-        int length = magicLine.length();
+    public boolean parser_magic_comment(ByteList magicLine) {
+        boolean indicator = false;
+        int vbeg, vend;
+        int length = magicLine.realSize();
+        int str = 0;
+        int end;
 
         if (length <= 7) return false;
         int beg = magicCommentMarker(magicLine, 0);
         if (beg >= 0) {
-            int end = magicCommentMarker(magicLine, beg);
+            end = magicCommentMarker(magicLine, beg);
             if (end < 0) return false;
+            indicator = true;
+            str = beg;
             length = end - beg - 3; // -3 is to backup over end just found
-        } else {
-            beg = 0;
         }
 
-        int begin = magicLine.getBegin() + beg;
-        Matcher matcher = magicRegexp.matcher(magicLine.unsafeBytes(), begin, begin + length);
-        int result = RubyRegexp.matcherSearch(runtime, matcher, begin, begin + length, Option.NONE);
+        /* %r"([^\\s\'\":;]+)\\s*:\\s*(\"(?:\\\\.|[^\"])*\"|[^\"\\s;]+)[\\s;]*" */
+        while (length > 0) {
+            int i;
+            long n = 0;
 
-        if (result < 0) return false;
+            for (; length > 0; str++, --length) {
+                char c = magicLine.charAt(str);
 
-        // Regexp is guaranteed to have three matches
-        int begs[] = matcher.getRegion().beg;
-        int ends[] = matcher.getRegion().end;
-        String name = magicLine.subSequence(beg + begs[1], beg + ends[1]).toString().replace('-', '_');
-        ByteList value = magicLine.makeShared(beg + begs[2], ends[2] - begs[2]);
+                switch (c) {
+                    case '\'': case '"': case ':': case ';': continue;
+                }
+                if (!Character.isWhitespace(c)) break;
+            }
 
-        if ("coding".equals(name) || "encoding".equals(name)) {
-            magicCommentEncoding(value);
-        } else if ("frozen_string_literal".equals(name)) {
-            setCompileOptionFlag(name, value);
-        } else if ("warn_indent".equals(name)) {
-            setTokenInfo(name, value);
-        } else {
-            return false;
+            for (beg = str; length > 0; str++, --length) {
+                char c = magicLine.charAt(str);
+
+                switch (c) {
+                    case '\'': case '"': case ':': case ';': break;
+                    default:
+                        if (Character.isWhitespace(c)) break;
+                    continue;
+                }
+                break;
+            }
+            for (end = str; length > 0 && Character.isWhitespace(magicLine.charAt(str)); str++, --length);
+            if (length == 0) break;
+            char c = magicLine.charAt(str);
+            if (c != ':') {
+                if (!indicator) return false;
+                continue;
+            }
+
+            do {
+                str++;
+            } while (--length > 0 && Character.isWhitespace(magicLine.charAt(str)));
+            if (length == 0) break;
+            if (magicLine.charAt(str) == '"') {
+                for (vbeg = ++str; --length > 0 && str < length && magicLine.charAt(str) != '"'; str++) {
+                    if (magicLine.charAt(str) == '\\') {
+                        --length;
+                        ++str;
+                    }
+                }
+                vend = str;
+                if (length > 0) {
+                    --length;
+                    ++str;
+                }
+            } else {
+                for (vbeg = str; length > 0 && magicLine.charAt(str) != '"' && magicLine.charAt(str) != ';' && !Character.isWhitespace(magicLine.charAt(str)); --length, str++);
+                vend = str;
+            }
+            if (indicator) {
+                while (length > 0 && (magicLine.charAt(str) == ';' || Character.isWhitespace(magicLine.charAt(str)))) {
+                    --length;
+                    str++;
+                }
+            } else {
+                while (length > 0 && Character.isWhitespace(magicLine.charAt(str))) {
+                    --length;
+                    str++;
+                }
+                if (length > 0) return false;
+            }
+
+            String name = magicLine.subSequence(beg, end).toString().replace('-', '_');
+            ByteList value = magicLine.makeShared(vbeg, vend - vbeg);
+
+            if ("coding".equals(name) || "encoding".equals(name)) {
+                magicCommentEncoding(value);
+            } else if ("frozen_string_literal".equals(name)) {
+                setCompileOptionFlag(name, value);
+            } else if ("warn_indent".equals(name)) {
+                setTokenInfo(name, value);
+            } else {
+                return false;
+            }
         }
 
         return true;

diff --git a/core/src/main/java/org/jruby/lexer/yacc/RubyLexer.java b/core/src/main/java/org/jruby/lexer/yacc/RubyLexer.java
@@ -874,7 +874,7 @@ private int yylex() throws IOException {
                 continue;
             case '#': {	/* it's a comment */
                 this.tokenSeen = tokenSeen;
-                if (!parseMagicComment(parserSupport.getConfiguration().getRuntime(), lexb.makeShared(lex_p, lex_pend - lex_p))) {
+                if (!parser_magic_comment(lexb.makeShared(lex_p, lex_pend - lex_p))) {
                     if (comment_at_top()) set_file_encoding(lex_p, lex_pend);
                 }
                 lex_p = lex_pend;

diff --git a/spec/regression/GH-4728_magic_string_parsing_finishes_spec.rb b/spec/regression/GH-4728_magic_string_parsing_finishes_spec.rb
@@ -0,0 +1,10 @@
+#     policy: "\"Version\": \"2012-10-17\",\n    \"Id\": \"custom-policy-2016-12-07\",\n    \"Statement\": [\n        {\n            \"Sid\": \"Enable IAM User Permissions\",\n            \"Effect\": \"Allow\",\n            \"Principal\": {\n                \"AWS\": \"arn:aws:iam::111122223333:root\"\n            },\n            \"Action\": \"kms:*\",\n            \"Resource\": \"*\"\n        },\n        {\n            \"Sid\": \"Allow access for Key Administrators\",\n            \"Effect\": \"Allow\",\n            \"Principal\": {\n                \"AWS\": [\n                    \"arn:aws:iam::111122223333:user/ExampleAdminUser\",\n                    \"arn:aws:iam::111122223333:role/ExampleAdminRole\"\n                ]\n            },\n            \"Action\": [\n                \"kms:Create*\",\n                \"kms:Describe*\",\n                \"kms:Enable*\",\n                \"kms:List*\",\n                \"kms:Put*\",\n                \"kms:Update*\",\n               require 'rspec'
+
+describe "A non-magical comment which sort of looks like one will finish" do
+  it "in a sane amount of time" do
+    # The actual spec is the huge jsonish comment at the top of this file.
+    # In the past we used a regexp to process magic comments and this particular
+    # line would send the regexp we used into a frenzy and never finish?
+    expect(1).to eq 1
+  end
+end