From 479dbf42d866ae0095287c8e7cc279247748feeb Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 26 May 2013 22:32:11 +0800 Subject: [PATCH] bug fixed for wallfilter --- .../mysql/parser/MySqlStatementParser.java | 65 +++++++++++-------- .../druid/wall/spi/WallVisitorUtils.java | 38 +++++++++-- .../druid/bvt/filter/wall/WallHavingTest.java | 2 +- .../bvt/filter/wall/WallSelectWhereTest0.java | 4 +- .../bvt/filter/wall/WallSelectWhereTest1.java | 17 +++-- 5 files changed, 86 insertions(+), 40 deletions(-) diff --git a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java index 5f8940daa8..1d43e03f4a 100644 --- a/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java +++ b/src/main/java/com/alibaba/druid/sql/dialect/mysql/parser/MySqlStatementParser.java @@ -35,11 +35,13 @@ import com.alibaba.druid.sql.ast.statement.SQLAlterTableAlterColumn; import com.alibaba.druid.sql.ast.statement.SQLAlterTableDisableConstraint; import com.alibaba.druid.sql.ast.statement.SQLAlterTableDisableKeys; import com.alibaba.druid.sql.ast.statement.SQLAlterTableDropColumnItem; +import com.alibaba.druid.sql.ast.statement.SQLAlterTableDropConstraint; import com.alibaba.druid.sql.ast.statement.SQLAlterTableDropForeinKey; import com.alibaba.druid.sql.ast.statement.SQLAlterTableDropIndex; import com.alibaba.druid.sql.ast.statement.SQLAlterTableDropPrimaryKey; import com.alibaba.druid.sql.ast.statement.SQLAlterTableEnableConstraint; import com.alibaba.druid.sql.ast.statement.SQLAlterTableEnableKeys; +import com.alibaba.druid.sql.ast.statement.SQLAlterTableStatement; import com.alibaba.druid.sql.ast.statement.SQLColumnDefinition; import com.alibaba.druid.sql.ast.statement.SQLCreateDatabaseStatement; import com.alibaba.druid.sql.ast.statement.SQLCreateTableStatement; @@ -2342,34 +2344,6 @@ public class MySqlStatementParser extends SQLStatementParser { } } stmt.getItems().add(item); - } else if (lexer.token() == Token.DROP) { - lexer.nextToken(); - if (lexer.token() == Token.INDEX) { - lexer.nextToken(); - SQLName indexName = this.exprParser.name(); - SQLAlterTableDropIndex item = new SQLAlterTableDropIndex(); - item.setIndexName(indexName); - stmt.getItems().add(item); - } else if (lexer.token() == Token.FOREIGN) { - lexer.nextToken(); - accept(Token.KEY); - SQLName indexName = this.exprParser.name(); - SQLAlterTableDropForeinKey item = new SQLAlterTableDropForeinKey(); - item.setIndexName(indexName); - stmt.getItems().add(item); - } else if (lexer.token() == Token.PRIMARY) { - lexer.nextToken(); - accept(Token.KEY); - SQLAlterTableDropPrimaryKey item = new SQLAlterTableDropPrimaryKey(); - stmt.getItems().add(item); - } else { - if (identifierEquals("COLUMN")) { - lexer.nextToken(); - } - SQLAlterTableDropColumnItem item = new SQLAlterTableDropColumnItem(); - item.setColumnName(this.exprParser.name()); - stmt.getItems().add(item); - } } else if (identifierEquals("DISABLE")) { lexer.nextToken(); @@ -2499,6 +2473,41 @@ public class MySqlStatementParser extends SQLStatementParser { throw new ParserException("TODO " + lexer.token() + " " + lexer.stringVal()); } + public void parseAlterDrop(SQLAlterTableStatement stmt) { + lexer.nextToken(); + if (lexer.token() == Token.INDEX) { + lexer.nextToken(); + SQLName indexName = this.exprParser.name(); + SQLAlterTableDropIndex item = new SQLAlterTableDropIndex(); + item.setIndexName(indexName); + stmt.getItems().add(item); + } else if (lexer.token() == Token.FOREIGN) { + lexer.nextToken(); + accept(Token.KEY); + SQLName indexName = this.exprParser.name(); + SQLAlterTableDropForeinKey item = new SQLAlterTableDropForeinKey(); + item.setIndexName(indexName); + stmt.getItems().add(item); + } else if (lexer.token() == Token.PRIMARY) { + lexer.nextToken(); + accept(Token.KEY); + SQLAlterTableDropPrimaryKey item = new SQLAlterTableDropPrimaryKey(); + stmt.getItems().add(item); + } else if (lexer.token() == Token.CONSTRAINT) { + lexer.nextToken(); + SQLAlterTableDropConstraint item = new SQLAlterTableDropConstraint(); + item.setConstraintName(this.exprParser.name()); + stmt.getItems().add(item); + } else if (identifierEquals("COLUMN")) { + lexer.nextToken(); + SQLAlterTableDropColumnItem item = new SQLAlterTableDropColumnItem(); + item.setColumnName(this.exprParser.name()); + stmt.getItems().add(item); + } else { + super.parseAlterDrop(stmt); + } + } + public SQLStatement parseRename() { MySqlRenameTableStatement stmt = new MySqlRenameTableStatement(); diff --git a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java index e80709e84b..8345a92223 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -203,8 +203,13 @@ public class WallVisitorUtils { if (Boolean.TRUE == getConditionValue(visitor, where, visitor.getConfig().isSelectWhereAlwayTrueCheck())) { boolean isSimpleConstExpr = false; - if (where instanceof SQLBinaryOpExpr) { - SQLBinaryOpExpr binaryOpExpr = (SQLBinaryOpExpr) where; + SQLExpr first = getFirst(where); + + if (first == where) { + isSimpleConstExpr = true; + } else if (first instanceof SQLBinaryOpExpr) { + SQLBinaryOpExpr binaryOpExpr = (SQLBinaryOpExpr) first; + if (binaryOpExpr.getOperator() == SQLBinaryOperator.Equality || binaryOpExpr.getOperator() == SQLBinaryOperator.NotEqual) { if (binaryOpExpr.getLeft() instanceof SQLIntegerExpr @@ -214,7 +219,7 @@ public class WallVisitorUtils { } } - if ((!isFirst(where)) && !isSimpleConstExpr) { + if (!isSimpleConstExpr) { addViolation(visitor, ErrorCode.ALWAY_TRUE, "select alway true condition not allow", x); } } @@ -229,7 +234,21 @@ public class WallVisitorUtils { } if (Boolean.TRUE == getConditionValue(visitor, x, visitor.getConfig().isSelectHavingAlwayTrueCheck())) { - addViolation(visitor, ErrorCode.ALWAY_TRUE, "having alway true condition not allow", x); + boolean isSimpleConstExpr = false; + if (x instanceof SQLBinaryOpExpr) { + SQLBinaryOpExpr binaryOpExpr = (SQLBinaryOpExpr) x; + if (binaryOpExpr.getOperator() == SQLBinaryOperator.Equality + || binaryOpExpr.getOperator() == SQLBinaryOperator.NotEqual) { + if (binaryOpExpr.getLeft() instanceof SQLIntegerExpr + && binaryOpExpr.getRight() instanceof SQLIntegerExpr) { + isSimpleConstExpr = true; + } + } + } + + if (!isSimpleConstExpr) { + addViolation(visitor, ErrorCode.ALWAY_TRUE, "having alway true condition not allow", x); + } } } @@ -602,6 +621,17 @@ public class WallVisitorUtils { return SQLEvalVisitorUtils.eval(dbType, x, Collections.emptyList(), false); } + + public static SQLExpr getFirst(SQLExpr x) { + if (x instanceof SQLBinaryOpExpr) { + SQLBinaryOpExpr binary = (SQLBinaryOpExpr) x; + if (binary.getOperator() == SQLBinaryOperator.BooleanAnd || binary.getOperator() == SQLBinaryOperator.BooleanOr) { + return getFirst(((SQLBinaryOpExpr) x).getLeft()); + } + } + + return x; + } public static boolean isFirst(SQLObject x) { if (x == null) { diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallHavingTest.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallHavingTest.java index 6b8b8383c7..4955a1ce0b 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallHavingTest.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallHavingTest.java @@ -27,7 +27,7 @@ import com.alibaba.druid.wall.WallUtils; * */ public class WallHavingTest extends TestCase { - private String sql = "SELECT F1, COUNT(*) FROM T GROUP BY F1 HAVING 1 = 1"; + private String sql = "SELECT F1, COUNT(*) FROM T GROUP BY F1 HAVING COUNT(*) OR 1 = 1"; public void testMySql() throws Exception { Assert.assertFalse(WallUtils.isValidateMySql(sql)); diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest0.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest0.java index d1fc557bc9..1b2ef8290a 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest0.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest0.java @@ -30,10 +30,10 @@ public class WallSelectWhereTest0 extends TestCase { private String sql = "SELECT F1, F2 WHERE 1 = 1 OR F1 = ?"; public void testMySql() throws Exception { - Assert.assertFalse(WallUtils.isValidateMySql(sql)); + Assert.assertTrue(WallUtils.isValidateMySql(sql)); } public void testORACLE() throws Exception { - Assert.assertFalse(WallUtils.isValidateOracle(sql)); + Assert.assertTrue(WallUtils.isValidateOracle(sql)); } } diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest1.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest1.java index 5eb60945d2..1cb1491442 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest1.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallSelectWhereTest1.java @@ -27,13 +27,20 @@ import com.alibaba.druid.wall.WallUtils; * */ public class WallSelectWhereTest1 extends TestCase { - private String sql = "SELECT F1, F2 WHERE 1 = 1 OR FID = ?"; - public void testMySql() throws Exception { - Assert.assertFalse(WallUtils.isValidateMySql(sql)); + public void testMySql_true() throws Exception { + Assert.assertTrue(WallUtils.isValidateMySql("SELECT F1, F2 WHERE 1 = 1 OR FID = ?")); } - public void testORACLE() throws Exception { - Assert.assertFalse(WallUtils.isValidateOracle(sql)); + public void testORACLE_true() throws Exception { + Assert.assertTrue(WallUtils.isValidateOracle("SELECT F1, F2 WHERE 1 = 1 OR FID = ?")); + } + + public void testMySql_false() throws Exception { + Assert.assertFalse(WallUtils.isValidateMySql("SELECT F1, F2 WHERE FID = ? OR 1 = 1")); + } + + public void testORACLE_false() throws Exception { + Assert.assertFalse(WallUtils.isValidateOracle("SELECT F1, F2 WHERE FID = ? OR 1 = 1")); } }