Skip to content

Commit 7e9bf06

Browse files
eregoneregon
committed
[GR-27631] Handle arbitrary Exceptions from the Joni parser and compiler.
PullRequest: truffleruby/2201
2 parents 96acdac + 98a4943 commit 7e9bf06

File tree

3 files changed

+51
-61
lines changed

3 files changed

+51
-61
lines changed

‎spec/truffle/regexp_spec.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. This
2+
# code is released under a tri EPL/GPL/LGPL license. You can use it,
3+
# redistribute it and/or modify it under the terms of the:
4+
#
5+
# Eclipse Public License version 2.0, or
6+
# GNU General Public License version 2, or
7+
# GNU Lesser General Public License version 2.1.
8+
9+
require_relative '../ruby/spec_helper'
10+
11+
describe "Regexp" do
12+
it "raise a RegexpError and not a fatal error when it fails to parse in Joni" do
13+
# On MRI this actually does not raise, but we need to fix Joni first for that
14+
-> { Regexp.new("\\p{") }.should raise_error(RegexpError)
15+
end
16+
end

‎src/main/java/org/truffleruby/core/regexp/ClassicRegexp.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,12 @@
6060
import org.truffleruby.core.rope.RopeOperations;
6161
import org.truffleruby.core.string.StringSupport;
6262
import org.truffleruby.core.string.StringUtils;
63+
import org.truffleruby.language.backtrace.BacktraceFormatter;
6364
import org.truffleruby.language.control.RaiseException;
6465
import org.truffleruby.parser.ReOptions;
6566

6667
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
68+
import com.oracle.truffle.api.nodes.Node;
6769

6870
public class ClassicRegexp implements ReOptions {
6971
private final RubyContext context;
@@ -79,25 +81,28 @@ public Encoding getEncoding() {
7981
return pattern.getEncoding();
8082
}
8183

82-
private static Regex makeRegexp(RubyContext context, RopeBuilder bytes, RegexpOptions options, Encoding enc) {
84+
public static Regex makeRegexp(RubyContext context, RopeBuilder processedSource, RegexpOptions options,
85+
Encoding enc, Rope source, Node currentNode) {
8386
try {
8487
return new Regex(
85-
bytes.getUnsafeBytes(),
88+
processedSource.getUnsafeBytes(),
8689
0,
87-
bytes.getLength(),
90+
processedSource.getLength(),
8891
options.toJoniOptions(),
8992
enc,
90-
Syntax.DEFAULT,
93+
Syntax.RUBY,
9194
new RegexWarnCallback(context));
9295
} catch (Exception e) {
93-
throw new RaiseException(context, context.getCoreExceptions().regexpError(e.getMessage(), null));
96+
String errorMessage = BacktraceFormatter.formatJavaThrowableMessage(e) + ": /" +
97+
RopeOperations.decodeRope(source) + "/";
98+
throw new RaiseException(context, context.getCoreExceptions().regexpError(errorMessage, currentNode));
9499
}
95100
}
96101

97102
private static Regex getRegexpFromCache(RubyContext context, RopeBuilder bytes, Encoding encoding,
98-
RegexpOptions options) {
103+
RegexpOptions options, Rope source) {
99104
if (context == null) {
100-
final Regex regex = makeRegexp(null, bytes, options, encoding);
105+
final Regex regex = makeRegexp(null, bytes, options, encoding, source, null);
101106
regex.setUserObject(bytes);
102107
return regex;
103108
}
@@ -114,7 +119,7 @@ private static Regex getRegexpFromCache(RubyContext context, RopeBuilder bytes,
114119
if (regex != null) {
115120
return regex;
116121
} else {
117-
final Regex newRegex = makeRegexp(context, bytes, options, encoding);
122+
final Regex newRegex = makeRegexp(context, bytes, options, encoding, source, null);
118123
newRegex.setUserObject(bytes);
119124
return context.getRegexpCache().addInCacheIfAbsent(cacheKey, newRegex);
120125
}
@@ -133,7 +138,7 @@ public ClassicRegexp(RubyContext context, Rope str, RegexpOptions originalOption
133138
RopeBuilder unescaped = preprocess(context, str, enc, fixedEnc, RegexpSupport.ErrorMode.RAISE);
134139
enc = computeRegexpEncoding(options, enc, fixedEnc, context);
135140

136-
this.pattern = getRegexpFromCache(context, unescaped, enc, options);
141+
this.pattern = getRegexpFromCache(context, unescaped, enc, options, str);
137142
this.str = str;
138143
}
139144

‎src/main/java/org/truffleruby/core/regexp/TruffleRegexpNodes.java

Lines changed: 21 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@
2323
import org.joni.Option;
2424
import org.joni.Regex;
2525
import org.joni.Region;
26-
import org.joni.Syntax;
27-
import org.joni.exception.SyntaxException;
28-
import org.joni.exception.ValueException;
2926
import org.truffleruby.RubyContext;
3027
import org.truffleruby.builtins.CoreMethod;
3128
import org.truffleruby.builtins.CoreMethodArrayArgumentsNode;
@@ -50,7 +47,6 @@
5047
import org.truffleruby.core.string.StringNodes.StringAppendPrimitiveNode;
5148
import org.truffleruby.core.string.StringOperations;
5249
import org.truffleruby.language.RubyContextNode;
53-
import org.truffleruby.language.control.RaiseException;
5450
import org.truffleruby.language.dispatch.DispatchNode;
5551

5652
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -101,17 +97,7 @@ private static Regex makeRegexpForEncoding(RubyContext context, RubyRegexp regex
10197
final RopeBuilder preprocessed = ClassicRegexp
10298
.preprocess(context, sourceRope, enc, fixedEnc, RegexpSupport.ErrorMode.RAISE);
10399
final RegexpOptions options = regexp.options;
104-
try {
105-
return new Regex(
106-
preprocessed.getUnsafeBytes(),
107-
0,
108-
preprocessed.getLength(),
109-
options.toJoniOptions(),
110-
enc,
111-
new RegexWarnCallback(context));
112-
} catch (SyntaxException e) {
113-
throw new RaiseException(context, context.getCoreExceptions().regexpError(e.getMessage(), currentNode));
114-
}
100+
return ClassicRegexp.makeRegexp(context, preprocessed, options, enc, sourceRope, currentNode);
115101
}
116102

117103
@CoreMethod(names = "union", onSingleton = true, required = 2, rest = true)
@@ -433,45 +419,28 @@ public String toString() {
433419
/** WARNING: computeRegexpEncoding() mutates options, so the caller should make sure it's a copy */
434420
@TruffleBoundary
435421
public static Regex compile(RubyContext context, Rope bytes, RegexpOptions options, Node currentNode) {
436-
try {
437-
if (options.isEncodingNone()) {
438-
bytes = RopeOperations.withEncoding(bytes, ASCIIEncoding.INSTANCE);
439-
}
440-
Encoding enc = bytes.getEncoding();
441-
Encoding[] fixedEnc = new Encoding[]{ null };
442-
RopeBuilder unescaped = ClassicRegexp
443-
.preprocess(context, bytes, enc, fixedEnc, RegexpSupport.ErrorMode.RAISE);
444-
enc = ClassicRegexp.computeRegexpEncoding(options, enc, fixedEnc, context);
445-
446-
Regex regexp = new Regex(
447-
unescaped.getUnsafeBytes(),
448-
0,
449-
unescaped.getLength(),
450-
options.toJoniOptions(),
422+
if (options.isEncodingNone()) {
423+
bytes = RopeOperations.withEncoding(bytes, ASCIIEncoding.INSTANCE);
424+
}
425+
Encoding enc = bytes.getEncoding();
426+
Encoding[] fixedEnc = new Encoding[]{ null };
427+
RopeBuilder unescaped = ClassicRegexp
428+
.preprocess(context, bytes, enc, fixedEnc, RegexpSupport.ErrorMode.RAISE);
429+
enc = ClassicRegexp.computeRegexpEncoding(options, enc, fixedEnc, context);
430+
431+
Regex regexp = ClassicRegexp.makeRegexp(context, unescaped, options, enc, bytes, currentNode);
432+
regexp.setUserObject(RopeOperations.withEncoding(bytes, enc));
433+
434+
if (context.getOptions().REGEXP_INSTRUMENT_CREATION) {
435+
final RegexpCacheKey key = new RegexpCacheKey(
436+
bytes,
451437
enc,
452-
Syntax.RUBY,
453-
new RegexWarnCallback(context));
454-
regexp.setUserObject(RopeOperations.withEncoding(bytes, enc));
455-
456-
if (context.getOptions().REGEXP_INSTRUMENT_CREATION) {
457-
final RegexpCacheKey key = new RegexpCacheKey(
458-
bytes,
459-
enc,
460-
options.toJoniOptions(),
461-
context.getHashing(REHASH_COMPILED_REGEXPS));
462-
ConcurrentOperations.getOrCompute(compiledRegexps, key, x -> new AtomicInteger()).incrementAndGet();
463-
}
464-
465-
return regexp;
466-
} catch (ValueException e) {
467-
throw new RaiseException(
468-
context,
469-
context.getCoreExceptions().regexpError(
470-
e.getMessage() + ": " + '/' + RopeOperations.decodeRope(bytes) + '/',
471-
currentNode));
472-
} catch (SyntaxException e) {
473-
throw new RaiseException(context, context.getCoreExceptions().regexpError(e.getMessage(), currentNode));
438+
options.toJoniOptions(),
439+
context.getHashing(REHASH_COMPILED_REGEXPS));
440+
ConcurrentOperations.getOrCompute(compiledRegexps, key, x -> new AtomicInteger()).incrementAndGet();
474441
}
442+
443+
return regexp;
475444
}
476445

477446
}

0 commit comments

Comments
 (0)