Merge pull request #532 from yakolee/master

improve sql parser & sql wall

src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlLexer.java

@@ -110,7 +110,7 @@ public class MySqlLexer extends Lexer {
             return;
         }
 
-        if (!isAllowComment()) {
+        if (!isAllowComment() && (isEOF() || !isSafeComment(stringVal))) {
             throw new NotAllowCommentException();
         }
     }
@@ -446,7 +446,7 @@ public class MySqlLexer extends Lexer {
                 return;
             }
 
-            if (!isAllowComment()) {
+            if (!isAllowComment() && (isEOF() || !isSafeComment(stringVal))) {
                 throw new NotAllowCommentException();
             }
 
@@ -486,11 +486,38 @@ public class MySqlLexer extends Lexer {
                 return;
             }
 
-            if (!isAllowComment()) {
+            if (!isAllowComment() && (isEOF() || !isSafeComment(stringVal))) {
                 throw new NotAllowCommentException();
             }
 
             return;
         }
     }
+    
+    private boolean isSafeComment(String comment) {
+        if (comment == null) {
+            return true;
+        }
+        comment = comment.toLowerCase();
+        if (comment.indexOf("select") != -1 //
+            || comment.indexOf("delete") != -1 //
+            || comment.indexOf("insert") != -1 //
+            || comment.indexOf("update") != -1 //
+            || comment.indexOf("into") != -1 //
+            || comment.indexOf("where") != -1 //
+            || comment.indexOf("or") != -1 //
+            || comment.indexOf("and") != -1 //
+            || comment.indexOf("union") != -1 //
+            || comment.indexOf('\'') != -1 //
+            || comment.indexOf('=') != -1 //
+            || comment.indexOf('>') != -1 //
+            || comment.indexOf('<') != -1 //
+            || comment.indexOf('&') != -1 //
+            || comment.indexOf('|') != -1 //
+            || comment.indexOf('^') != -1 //
+        ) {
+            return false;
+        }
+        return true;
+    }
 }

src/main/java/com/alibaba/druid/sql/parser/Lexer.java

@@ -1038,7 +1038,7 @@ public class Lexer {
     }
 
     public BigDecimal decimalValue() {
-        return new BigDecimal(text.toCharArray(), mark, bufPos);
+        return new BigDecimal(subString(mark, bufPos).toCharArray());
     }
 
     public static interface CommentHandler {

src/main/java/com/alibaba/druid/sql/parser/SQLSelectParser.java

@@ -352,7 +352,8 @@ public class SQLSelectParser extends SQLParser {
 
     protected SQLTableSource parseTableSourceRest(SQLTableSource tableSource) {
         if ((tableSource.getAlias() == null) || (tableSource.getAlias().length() == 0)) {
-            if (lexer.token() != Token.LEFT && lexer.token() != Token.RIGHT && lexer.token() != Token.FULL) {
+            if (lexer.token() != Token.LEFT && lexer.token() != Token.RIGHT && lexer.token() != Token.FULL
+                && !identifierEquals("STRAIGHT_JOIN") && !identifierEquals("CROSS")) {
                 String alias = as();
                 if (alias != null) {
                     tableSource.setAlias(alias);

src/main/java/com/alibaba/druid/sql/visitor/functions/If.java

@@ -38,7 +38,10 @@ public class If implements Function {
         SQLExpr condition = parameters.get(0);
         condition.accept(visitor);
         Object itemValue = condition.getAttributes().get(EVAL_VALUE);
-        if (Boolean.TRUE == itemValue || (itemValue != null && !SQLEvalVisitorUtils.eq(itemValue, 0))) {
+        if (itemValue == null) {
+            return null;
+        }
+        if (Boolean.TRUE == itemValue || !SQLEvalVisitorUtils.eq(itemValue, 0)) {
             SQLExpr trueExpr = parameters.get(1);
             trueExpr.accept(visitor);
             return trueExpr.getAttributes().get(EVAL_VALUE);

src/main/java/com/alibaba/druid/wall/WallProvider.java

@@ -632,7 +632,7 @@ public abstract class WallProvider {
                                                                                      + lastToken, sql));
             }
         } catch (NotAllowCommentException e) {
-            violations.add(new SyntaxErrorViolation(e, sql));
+            violations.add(new IllegalSQLObjectViolation(ErrorCode.COMMENT_STATEMENT_NOT_ALLOW, "comment not allow", sql));
             incrementCommentDeniedCount();
         } catch (ParserException e) {
             syntaxErrrorCount.incrementAndGet();

src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java

@@ -347,6 +347,12 @@ public class WallVisitorUtils {
         }
 
         for (SQLExpr part : parts) {
+            if(isFirst(part)) {
+                Object evalValue = part.getAttribute(EVAL_VALUE);
+                if(evalValue != null && evalValue instanceof Boolean && (Boolean)evalValue) {
+                    return true;
+                }
+            }
             boolean isSimpleConstExpr = false;
             if (part == sqlExpr || part instanceof SQLLiteralExpr) {
                 isSimpleConstExpr = true;
@@ -2358,11 +2364,11 @@ public class WallVisitorUtils {
             errorCode = ErrorCode.SHOW_NOT_ALLOW;
         } else if (x instanceof MySqlCommitStatement) {
             allow = config.isCommitAllow();
-            denyMessage = "show not allow";
+            denyMessage = "commit not allow";
             errorCode = ErrorCode.COMMIT_NOT_ALLOW;
         } else if (x instanceof SQLRollbackStatement) {
             allow = config.isRollbackAllow();
-            denyMessage = "show not allow";
+            denyMessage = "rollback not allow";
             errorCode = ErrorCode.ROLLBACK_NOT_ALLOW;
         } else if (x instanceof SQLUseStatement) {
             allow = config.isUseAllow();

src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest.java

@@ -45,5 +45,8 @@ public class MySqlWallTest extends TestCase {
         Assert.assertTrue(WallUtils.isValidateMySql("select 'outfile'"));
 
         Assert.assertFalse(WallUtils.isValidateMySql("select f1, f2 from t union select 1, 2"));
+        
+        Assert.assertTrue(WallUtils.isValidateMySql("select c1 from t where 1=1 or id =1"));
+        Assert.assertFalse(WallUtils.isValidateMySql("select c1 from t where id =1 or 1=1"));
     }
 }

src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest_comment.java

@@ -33,13 +33,37 @@ public class MySqlWallTest_comment extends TestCase {
 
     public void test_true() throws Exception {
         WallProvider provider = new MySqlWallProvider();
-        
+
         provider.getConfig().setCommentAllow(true);
-        
+
         Assert.assertTrue(provider.checkValid(//
         "SELECT * FROM T WHERE FID = ? #AND 1"));
 
         Assert.assertEquals(1, provider.getTableStats().size());
     }
 
+    public void test_false() throws Exception {
+        WallProvider provider = new MySqlWallProvider();
+
+        provider.getConfig().setCommentAllow(false);
+
+        Assert.assertTrue(provider.checkValid("/* this is comment */ SELECT id FROM t "));
+        Assert.assertTrue(provider.checkValid("-- this is comment \n SELECT * FROM t"));
+        Assert.assertTrue(provider.checkValid("#this is comment \n SELECT * FROM t"));
+        
+        Assert.assertFalse(provider.checkValid("/*!40101fff*/ select * from t"));
+
+        Assert.assertTrue(provider.checkValid("SELECT * FROM t where a=1 #this is comment \n and b=1"));
+        Assert.assertTrue(provider.checkValid("SELECT * FROM t where a=1 -- this is comment \n and c=1"));
+        Assert.assertTrue(provider.checkValid("SELECT * FROM t where a=1 /* this is comment */ and d=1"));
+
+        Assert.assertFalse(provider.checkValid("SELECT * FROM t where a=1 #and c=1 \n and e=1"));
+        Assert.assertFalse(provider.checkValid("SELECT * FROM t where a=1 -- AND c=1 \n and f=1"));
+        Assert.assertFalse(provider.checkValid("SELECT * FROM t where a=1 /* and c=1 */ and g=1"));
+
+        Assert.assertFalse(provider.checkValid("SELECT * FROM t where a=1 #and c=1 "));
+        Assert.assertFalse(provider.checkValid("SELECT * FROM t where a=1 -- and c=1"));
+        Assert.assertFalse(provider.checkValid("SELECT * FROM t where a=1 /* and c=1 */"));
+    }
+
 }

src/test/java/com/alibaba/druid/bvt/filter/wall/WallMultiLineCommentTest.java

@@ -22,7 +22,7 @@ import org.junit.Assert;
 import com.alibaba.druid.wall.WallUtils;
 
 public class WallMultiLineCommentTest extends TestCase {
-    private String sql = "select f1 from t /* xx */ ";
+    private String sql = "select f1 from t /* xx */";
     
     public void testOracle() throws Exception {
         Assert.assertFalse(WallUtils.isValidateOracle(sql));

src/test/java/com/alibaba/druid/bvt/sql/mysql/MySqlSelectTest_17.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright 1999-2011 Alibaba Group Holding Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.alibaba.druid.bvt.sql.mysql;
+
+import java.util.List;
+
+import org.junit.Assert;
+
+import com.alibaba.druid.sql.MysqlTest;
+import com.alibaba.druid.sql.ast.SQLStatement;
+import com.alibaba.druid.sql.dialect.mysql.parser.MySqlStatementParser;
+import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlSchemaStatVisitor;
+
+public class MySqlSelectTest_17 extends MysqlTest {
+
+    public void test_0() throws Exception {
+        String sql = "SELECT * FROM t1 STRAIGHT_JOIN t2 ON t2.a=t1.a;";
+
+        MySqlStatementParser parser = new MySqlStatementParser(sql);
+        List<SQLStatement> statementList = parser.parseStatementList();
+        SQLStatement statement = statementList.get(0);
+        print(statementList);
+
+        Assert.assertEquals(1, statementList.size());
+
+        MySqlSchemaStatVisitor visitor = new MySqlSchemaStatVisitor();
+        statement.accept(visitor);
+
+        System.out.println("Tables : " + visitor.getTables());
+        System.out.println("fields : " + visitor.getColumns());
+        System.out.println("coditions : " + visitor.getConditions());
+        System.out.println("orderBy : " + visitor.getOrderByColumns());
+        
+        Assert.assertEquals(2, visitor.getTables().size());
+    }
+    
+    public void test_1() throws Exception {
+        String sql = "SELECT * FROM t1 CROSS JOIN t2";
+
+        MySqlStatementParser parser = new MySqlStatementParser(sql);
+        List<SQLStatement> statementList = parser.parseStatementList();
+        SQLStatement statement = statementList.get(0);
+        print(statementList);
+
+        Assert.assertEquals(1, statementList.size());
+
+        MySqlSchemaStatVisitor visitor = new MySqlSchemaStatVisitor();
+        statement.accept(visitor);
+
+        System.out.println("Tables : " + visitor.getTables());
+        System.out.println("fields : " + visitor.getColumns());
+        System.out.println("coditions : " + visitor.getConditions());
+        System.out.println("orderBy : " + visitor.getOrderByColumns());
+        
+        Assert.assertEquals(2, visitor.getTables().size());
+    }
+}