MONDRIAN: Fix bug MONDRIAN-983, "Unable to execute MDX statement with…

… native

    MATCHES"; tighten up the regex implementation in other ways.

[git-p4: depot-paths = "//open/mondrian/": change = 14590]

src/main/mondrian/spi/impl/MySqlDialect.java

@@ -10,11 +10,10 @@
 
 import mondrian.olap.Util;
 
+import java.sql.*;
 import java.util.List;
 import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import java.sql.*;
+import java.util.regex.*;
 
 /**
  * Implementation of {@link mondrian.spi.Dialect} for the MySQL database.
@@ -271,31 +270,34 @@ public boolean allowsRegularExpressionInWhereClause() {
 
     public String generateRegularExpression(
         String source,
-        String javaRegExp)
+        String javaRegex)
     {
-        final Matcher flagsMatcher = flagsPattern.matcher(javaRegExp);
+        try {
+            Pattern.compile(javaRegex);
+        } catch (PatternSyntaxException e) {
+            // Not a valid Java regex. Too risky to continue.
+            return null;
+        }
+        final Matcher flagsMatcher = flagsPattern.matcher(javaRegex);
         if (flagsMatcher.matches()) {
-            javaRegExp =
-                javaRegExp.replace(
-                    flagsMatcher.group(1),
-                    "");
+            javaRegex =
+                javaRegex.substring(0, flagsMatcher.start(1))
+                + javaRegex.substring(flagsMatcher.end(1));
         }
-        final Matcher escapeMatcher = escapePattern.matcher(javaRegExp);
+        final Matcher escapeMatcher = escapePattern.matcher(javaRegex);
         if (escapeMatcher.matches()) {
             String sequence = escapeMatcher.group(2);
             sequence = sequence.replaceAll("\\\\", "\\\\");
-            javaRegExp =
-                javaRegExp.replace(
-                    escapeMatcher.group(1),
-                    sequence.toUpperCase());
+            javaRegex =
+                javaRegex.substring(0, escapeMatcher.start(1))
+                + sequence.toUpperCase()
+                + javaRegex.substring(escapeMatcher.end(1));
         }
         final StringBuilder sb = new StringBuilder();
         sb.append("UPPER(");
         sb.append(source);
         sb.append(") REGEXP ");
-        sb.append("'");
-        sb.append(javaRegExp);
-        sb.append("'");
+        quoteStringLiteral(sb, javaRegex);
         return sb.toString();
     }
 }

src/main/mondrian/spi/impl/OracleDialect.java

@@ -8,13 +8,12 @@
 */
 package mondrian.spi.impl;
 
-import java.util.List;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import java.sql.Connection;
 import java.sql.Date;
 import java.sql.SQLException;
 import java.sql.Timestamp;
+import java.util.List;
+import java.util.regex.*;
 
 /**
  * Implementation of {@link mondrian.spi.Dialect} for the Oracle database.
@@ -84,13 +83,20 @@ public boolean allowsRegularExpressionInWhereClause() {
     @Override
     public String generateRegularExpression(
         String source,
-        String javaRegExp)
+        String javaRegex)
     {
-        final Matcher flagsMatcher = flagsPattern.matcher(javaRegExp);
-        final StringBuilder suffixSb = new StringBuilder();
+        try {
+            Pattern.compile(javaRegex);
+        } catch (PatternSyntaxException e) {
+            // Not a valid Java regex. Too risky to continue.
+            return null;
+        }
+        final Matcher flagsMatcher = flagsPattern.matcher(javaRegex);
+        final String suffix;
         if (flagsMatcher.matches()) {
             // We need to convert leading flags into oracle
             // specific flags
+            final StringBuilder suffixSb = new StringBuilder();
             final String flags = flagsMatcher.group(2);
             if (flags.contains("i")) {
                 suffixSb.append("i");
@@ -101,29 +107,31 @@ public String generateRegularExpression(
             if (flags.contains("m")) {
                 suffixSb.append("m");
             }
-            javaRegExp =
-                javaRegExp.replace(
-                    flagsMatcher.group(1),
-                    "");
+            suffix = suffixSb.toString();
+            javaRegex =
+                javaRegex.substring(0, flagsMatcher.start(1))
+                + javaRegex.substring(flagsMatcher.end(1));
+        } else {
+            suffix = "";
         }
-        final Matcher escapeMatcher = escapePattern.matcher(javaRegExp);
+        final Matcher escapeMatcher = escapePattern.matcher(javaRegex);
         if (escapeMatcher.matches()) {
             // We need to convert escape characters \E and \Q into
             // oracle compatible escapes.
             String sequence = escapeMatcher.group(2);
             sequence = sequence.replaceAll("\\\\", "\\\\");
-            javaRegExp =
-                javaRegExp.replace(
+            javaRegex =
+                javaRegex.replace(
                     escapeMatcher.group(1),
                     sequence);
         }
         final StringBuilder sb = new StringBuilder();
         sb.append("REGEXP_LIKE(");
         sb.append(source);
         sb.append(", ");
-        quoteStringLiteral(sb, javaRegExp);
+        quoteStringLiteral(sb, javaRegex);
         sb.append(", ");
-        quoteStringLiteral(sb, suffixSb.toString());
+        quoteStringLiteral(sb, suffix);
         sb.append(")");
         return sb.toString();
     }

src/main/mondrian/spi/impl/PostgreSqlDialect.java

@@ -12,6 +12,7 @@
 import org.apache.log4j.Logger;
 
 import java.sql.*;
+import java.util.regex.*;
 
 /**
  * Implementation of {@link mondrian.spi.Dialect} for the PostgreSQL database.
@@ -139,13 +140,19 @@ public boolean allowsRegularExpressionInWhereClause() {
         return true;
     }
 
-    public String generateRegularExpression(String source, String javaRegExp) {
-        javaRegExp = javaRegExp.replace("\\Q", "");
-        javaRegExp = javaRegExp.replace("\\E", "");
+    public String generateRegularExpression(String source, String javaRegex) {
+        try {
+            Pattern.compile(javaRegex);
+        } catch (PatternSyntaxException e) {
+            // Not a valid Java regex. Too risky to continue.
+            return null;
+        }
+        javaRegex = javaRegex.replace("\\Q", "");
+        javaRegex = javaRegex.replace("\\E", "");
         final StringBuilder sb = new StringBuilder();
         sb.append(source);
         sb.append(" ~ ");
-        quoteStringLiteral(sb, javaRegExp);
+        quoteStringLiteral(sb, javaRegex);
         return sb.toString();
     }
 }

testsrc/main/mondrian/rolap/NativeFilterMatchingTest.java

@@ -9,12 +9,16 @@
 */
 package mondrian.rolap;
 
-import mondrian.olap.MondrianProperties;
 import mondrian.olap.Result;
 import mondrian.spi.Dialect;
 import mondrian.test.SqlPattern;
 import mondrian.test.TestContext;
 
+/**
+ * Test case for pushing MDX filter conditions down to SQL.
+ *
+ * @version $Id$
+ */
 public class NativeFilterMatchingTest extends BatchTestCase {
     public void testPositiveMatching() throws Exception {
         final String sqlOracle =
@@ -131,5 +135,38 @@ public void testNegativeMatching() throws Exception {
         final String resultString = TestContext.toString(result);
         assertFalse(resultString.contains("Jeanne"));
     }
+
+    /**
+     * <p>System test case for bug
+     * <a href="http://jira.pentaho.com/browse/MONDRIAN-983">MONDRIAN-983,
+     * "Regression: Unable to execute MDX statement with native MATCHES"</a>.
+     *
+     * @see mondrian.test.DialectTest#testRegularExpressionSqlInjection()
+     */
+    public void testMatchBugMondrian983() {
+        assertQueryReturns(
+            "With\n"
+            + "Set [*NATIVE_CJ_SET] as 'Filter([*BASE_MEMBERS_Product], Not IsEmpty ([Measures].[Unit Sales]))' \n"
+            + "Set [*SORTED_ROW_AXIS] as 'Order([*CJ_ROW_AXIS],[Product].CurrentMember.OrderKey,BASC,Ancestor([Product].CurrentMember,[Product].[Product Department]).OrderKey,BASC)' \n"
+            + "Set [*NATIVE_MEMBERS_Product] as 'Generate([*NATIVE_CJ_SET], {[Product].CurrentMember})' \n"
+            + "Set [*BASE_MEMBERS_Product] as 'Filter([Product].[Product Category].Members,[Product].CurrentMember.Caption Matches (\"(?i).*\\Qa\"\"); window.alert(\"\"woot'');\\E.*\"))' \n"
+            + "Set [*BASE_MEMBERS_Measures] as '{[Measures].[*FORMATTED_MEASURE_0]}' \n"
+            + "Set [*CJ_ROW_AXIS] as 'Generate([*NATIVE_CJ_SET], {([Product].currentMember)})' \n"
+            + "Set [*CJ_COL_AXIS] as '[*NATIVE_CJ_SET]' \n"
+            + "Member [Product].[*TOTAL_MEMBER_SEL~SUM] as 'Sum([*NATIVE_MEMBERS_Product])', SOLVE_ORDER=-100 \n"
+            + "Member [Measures].[*FORMATTED_MEASURE_0] as '[Measures].[Unit Sales]', FORMAT_STRING = 'Standard', SOLVE_ORDER=400 \n"
+            + "Select\n"
+            + "[*BASE_MEMBERS_Measures] on columns,\n"
+            + "Union({[Product].[*TOTAL_MEMBER_SEL~SUM]},[*SORTED_ROW_AXIS]) on rows\n"
+            + "From [Sales]",
+            "Axis #0:\n"
+            + "{}\n"
+            + "Axis #1:\n"
+            + "{[Measures].[*FORMATTED_MEASURE_0]}\n"
+            + "Axis #2:\n"
+            + "{[Product].[*TOTAL_MEMBER_SEL~SUM]}\n"
+            + "Row #0: \n");
+    }
 }
-// End NativeFilterMatchingTest.java
+
+// End NativeFilterMatchingTest.java

testsrc/main/mondrian/test/DialectTest.java

@@ -937,6 +937,82 @@ public void testAllowsRegularExpressionInWhereClause() throws Exception {
                     "(?i).*\\QBar\\E.*"));
         }
     }
+
+    public void testRegularExpressionSqlInjection() throws SQLException {
+        // bug mondrian-983
+        // We know that mysql's dialect can handle this regex
+        boolean couldTranslate =
+            checkRegex("(?i).*\\Qa\"\"); window.alert(\"\"woot');\\E.*");
+        switch (getDialect().getDatabaseProduct()) {
+        case MYSQL:
+            assertTrue(couldTranslate);
+            break;
+        }
+
+        // On mysql, this gives error:
+        //   Got error 'repetition-operator operand invalid' from regexp
+        //
+        // Ideally, we would detect that the regex cannot be translated to
+        // SQL (perhaps because it's not a valid java regex). Currently the
+        // database gives an error, and that's better than nothing.
+        Throwable throwable = null;
+        try {
+            couldTranslate =
+                checkRegex(
+                    "\"(?i).*\\Qa\"\"); window.alert(\"\"woot');\\E.*\"");
+        } catch (SQLException e) {
+            throwable = e;
+        }
+        switch (getDialect().getDatabaseProduct()) {
+        case MYSQL:
+            assertNotNull(throwable);
+            assertTrue(couldTranslate);
+            assertTrue(
+                throwable.getMessage().contains(
+                    "Got error 'repetition-operator operand invalid' from "
+                    + "regexp"));
+            break;
+        default:
+            // As far as we know, all other databases either handle this regex
+            // just fine or our dialect for that database refuses to translate
+            // the regex to SQL.
+            assertNull(throwable);
+        }
+
+        // Every dialect should refuse to translate an invalid regex.
+        couldTranslate = checkRegex("ab[cd");
+        assertFalse(couldTranslate);
+    }
+
+    /**
+     * Translates a regular expression into SQL and executes the query.
+     * Returns whether the dialect was able to translate the regex.
+     *
+     * @param regex Java regular expression string
+     * @return Whether dialect could translate regex to SQL.
+     * @throws SQLException on error
+     */
+    private boolean checkRegex(String regex) throws SQLException {
+        Dialect dialect = getDialect();
+        final String sqlRegex =
+            dialect.generateRegularExpression(
+                dialect.quoteIdentifier("customer", "fname"),
+                regex);
+        if (sqlRegex != null) {
+            String sql =
+                "select * from "
+                + dialect.quoteIdentifier("customer")
+                + " where "
+                + sqlRegex;
+            final ResultSet resultSet =
+                getConnection().createStatement().executeQuery(sql);
+            assertFalse(resultSet.next());
+            resultSet.close();
+            return true;
+        } else {
+            return false;
+        }
+    }
 }
 
 // End DialectTest.java