From 6d2fd0407a0a5ce553cc04f3ca4671ef171901c8 Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Thu, 13 Apr 2017 12:14:53 +0100 Subject: [PATCH 1/2] JENA-1319: Use ExprEvalException; don't produce a stacktrace. --- .../org/apache/jena/sparql/expr/E_Regex.java | 26 ++++++++++++++++--- .../apache/jena/sparql/expr/RegexJava.java | 9 +++---- .../apache/jena/sparql/expr/RegexXerces.java | 4 +-- .../sse/builders/ExprBuildException.java | 11 +++----- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java b/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java index 3e8046c74c6..f6b6743f1ff 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java @@ -71,11 +71,19 @@ public E_Regex(Expr expr, String pattern, String flags) private void init(Expr pattern, Expr flags) { + try { if ( pattern.isConstant() && pattern.getConstant().isString() && ( flags==null || flags.isConstant() ) ) regexEngine = makeRegexEngine(pattern.getConstant(), (flags==null)?null:flags.getConstant()) ; + } catch (ExprEvalException ex) { + // Here, we are doing static compilation of the pattern. + // ExprEvalException does not have a stacktrace. + // We could throw a non-eval exception. + throw ex; //new ExprException(ex.getMessage(), ex.getCause()); + } } - + private String currentFailMessage = null; + @Override public NodeValue eval(List args) { @@ -84,8 +92,20 @@ public NodeValue eval(List args) NodeValue vFlags = ( args.size() == 2 ? null : args.get(2) ) ; RegexEngine regex = regexEngine ; - if ( regex == null ) - regex = makeRegexEngine(vPattern, vFlags) ; + if ( regex == null ) { + // Execution time regex compile (not a constant pattern). + try { + regex = makeRegexEngine(vPattern, vFlags) ; + } catch (ExprEvalException ex) { + // Avoid multiple logging of the same message. + String m = ex.getMessage(); + if ( m != null && ! m.equals(currentFailMessage) ) + Log.warn(this, m); + currentFailMessage = m; + // This becomes an eval error, the FILTER is false and the current row rejected. + throw ex; + } + } boolean b = regex.match(arg.getLiteralLexicalForm()) ; diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexJava.java b/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexJava.java index 6cd16f5996a..4a85b2f0b4c 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexJava.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexJava.java @@ -22,8 +22,6 @@ import java.util.regex.Pattern ; import java.util.regex.PatternSyntaxException ; -import org.apache.jena.query.QueryParseException ; - public class RegexJava implements RegexEngine { Pattern regexPattern ; @@ -49,7 +47,7 @@ private Pattern makePattern(String patternStr, String flags) return Pattern.compile(patternStr, mask) ; } catch (PatternSyntaxException pEx) - { throw new ExprException("Regex: Pattern exception: "+pEx) ; } + { throw new ExprEvalException("Regex: Pattern exception: "+pEx) ; } } @@ -63,7 +61,6 @@ public static int makeMask(String modifiers) { switch(modifiers.charAt(i)) { - //case 'i' : newMask |= Pattern.CASE_INSENSITIVE; break ; case 'i' : // Need both (Java 1.4) newMask |= Pattern.UNICODE_CASE ; @@ -73,8 +70,8 @@ public static int makeMask(String modifiers) case 's' : newMask |= Pattern.DOTALL ; break ; //case 'x' : newMask |= Pattern.; break ; - default : - throw new QueryParseException("Illegal flag in regex modifiers: "+modifiers.charAt(i), -1, -1) ; + default: + throw new ExprEvalException("Illegal flag in regex modifiers: "+modifiers.charAt(i)) ; } } return newMask ; diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexXerces.java b/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexXerces.java index 491fbda2c0b..890f4f3663a 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexXerces.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/RegexXerces.java @@ -41,9 +41,9 @@ private RegularExpression makePattern(String patternStr, String flags) // Input : only m s i x // Check/modify flags. // Always "u", never patternStr - //x: Remove whitespace characters (#x9, #xA, #xD and #x20) unless in [] classes + // x: Remove whitespace characters (#x9, #xA, #xD and #x20) unless in [] classes try { return new RegularExpression(patternStr, flags) ; } catch (ParseException pEx) - { throw new ExprException("Regex: Pattern exception: "+pEx) ; } + { throw new ExprEvalException("Regex: Pattern exception: "+pEx) ; } } } diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/sse/builders/ExprBuildException.java b/jena-arq/src/main/java/org/apache/jena/sparql/sse/builders/ExprBuildException.java index 802d483913f..d2bbd2b62b9 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/sse/builders/ExprBuildException.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/sse/builders/ExprBuildException.java @@ -20,10 +20,7 @@ public class ExprBuildException extends BuildException - { - -// public OpBuildException(Throwable cause) { super(cause) ; } -// public OpBuildException() { super() ; } - public ExprBuildException (String msg) { super(msg) ; } -// public OpBuildException (String msg, Throwable cause) { super(msg, cause) ; } - } +{ + public ExprBuildException (String msg) { super(msg) ; } + public ExprBuildException (String msg, Throwable cause) { super(msg, cause) ; } +} From a9dbd9e7ae13c71a9e4e4e2283d28192ebe9f804 Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Thu, 13 Apr 2017 17:32:05 +0100 Subject: [PATCH 2/2] Tests for regex errors. --- .../src/main/java/org/apache/jena/sparql/expr/E_Regex.java | 6 ++---- .../test/java/org/apache/jena/sparql/expr/TestRegex.java | 7 +++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java b/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java index f6b6743f1ff..a4d89d4427c 100644 --- a/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java +++ b/jena-arq/src/main/java/org/apache/jena/sparql/expr/E_Regex.java @@ -97,7 +97,7 @@ public NodeValue eval(List args) try { regex = makeRegexEngine(vPattern, vFlags) ; } catch (ExprEvalException ex) { - // Avoid multiple logging of the same message. + // Avoid multiple logging of the same message (at least if adjacent) String m = ex.getMessage(); if ( m != null && ! m.equals(currentFailMessage) ) Log.warn(this, m); @@ -106,10 +106,8 @@ public NodeValue eval(List args) throw ex; } } - boolean b = regex.match(arg.getLiteralLexicalForm()) ; - - return b ? NodeValue.TRUE : NodeValue.FALSE ; + return b ? NodeValue.TRUE : NodeValue.FALSE ; } public static RegexEngine makeRegexEngine(NodeValue vPattern, NodeValue vFlags) diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestRegex.java b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestRegex.java index 547732c97ad..53e883b1400 100644 --- a/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestRegex.java +++ b/jena-arq/src/test/java/org/apache/jena/sparql/expr/TestRegex.java @@ -63,5 +63,12 @@ private String fmtTest(String value, String pattern, String flags) tmp = tmp + ")" ; return tmp ; } + + // Bad regex + @Test(expected=ExprEvalException.class) + public void testRegexErr1() { regexTest("ABC", "(", null, false) ; } + // No such flag + @Test(expected=ExprEvalException.class) + public void testRegexErr2() { regexTest("ABC", "abc", "g", false) ; } }