diff --git a/src/main/java/com/alibaba/druid/sql/visitor/ExportParameterVisitor.java b/src/main/java/com/alibaba/druid/sql/visitor/ExportParameterVisitor.java index 9a88085147..57fcda0fa0 100644 --- a/src/main/java/com/alibaba/druid/sql/visitor/ExportParameterVisitor.java +++ b/src/main/java/com/alibaba/druid/sql/visitor/ExportParameterVisitor.java @@ -1,6 +1,8 @@ package com.alibaba.druid.sql.visitor; +import java.util.List; + public interface ExportParameterVisitor extends SQLASTVisitor { - + List getParameters(); } diff --git a/src/main/java/com/alibaba/druid/wall/WallVisitor.java b/src/main/java/com/alibaba/druid/wall/WallVisitor.java index bc5f4234f1..98b6efab9f 100644 --- a/src/main/java/com/alibaba/druid/wall/WallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/WallVisitor.java @@ -8,6 +8,8 @@ import com.alibaba.druid.sql.visitor.SQLASTVisitor; public interface WallVisitor extends SQLASTVisitor { WallConfig getConfig(); + + WallProvider getProvider(); List getViolations(); diff --git a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallProvider.java b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallProvider.java index d5abdec753..cc3b6758b1 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallProvider.java @@ -27,7 +27,7 @@ public class MySqlWallProvider extends WallProvider { @Override public WallVisitor createWallVisitor() { - return new MySqlWallVisitor(config); + return new MySqlWallVisitor(this); } @Override diff --git a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java index 94f62cdd66..8f352e4ba2 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java @@ -32,16 +32,23 @@ import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlASTVisitor; import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlASTVisitorAdapter; import com.alibaba.druid.wall.Violation; import com.alibaba.druid.wall.WallConfig; +import com.alibaba.druid.wall.WallProvider; import com.alibaba.druid.wall.WallVisitor; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisitor, MySqlASTVisitor { private final WallConfig config; + private final WallProvider provider; private final List violations = new ArrayList(); - public MySqlWallVisitor(WallConfig config){ - this.config = config; + public MySqlWallVisitor(WallProvider provider){ + this.config = provider.getConfig(); + this.provider = provider; + } + + public WallProvider getProvider() { + return provider; } public WallConfig getConfig() { @@ -56,7 +63,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi WallVisitorUtils.check(this, x); return true; } - + public boolean visit(SQLInListExpr x) { WallVisitorUtils.check(this, x); return true; @@ -93,7 +100,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi @Override public boolean visit(MySqlDeleteStatement x) { WallVisitorUtils.checkReadOnly(this, x.getFrom()); - + return visit((SQLDeleteStatement) x); } diff --git a/src/main/java/com/alibaba/druid/wall/spi/OracleWallProvider.java b/src/main/java/com/alibaba/druid/wall/spi/OracleWallProvider.java index d41f972b9e..e223ce7c63 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/OracleWallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/spi/OracleWallProvider.java @@ -27,7 +27,7 @@ public class OracleWallProvider extends WallProvider { @Override public WallVisitor createWallVisitor() { - return new OracleWallVisitor(config); + return new OracleWallVisitor(this); } @Override diff --git a/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java b/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java index 661d71a1a2..f6b0a9be47 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java @@ -32,16 +32,23 @@ import com.alibaba.druid.sql.dialect.oracle.ast.stmt.OracleUpdateStatement; import com.alibaba.druid.sql.dialect.oracle.visitor.OracleASTVIsitorAdapter; import com.alibaba.druid.wall.Violation; import com.alibaba.druid.wall.WallConfig; +import com.alibaba.druid.wall.WallProvider; import com.alibaba.druid.wall.WallVisitor; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; public class OracleWallVisitor extends OracleASTVIsitorAdapter implements WallVisitor { private final WallConfig config; + private final WallProvider provider; private final List violations = new ArrayList(); - public OracleWallVisitor(WallConfig config){ - this.config = config; + public OracleWallVisitor(WallProvider provider){ + this.config = provider.getConfig(); + this.provider = provider; + } + + public WallProvider getProvider() { + return provider; } public WallConfig getConfig() { @@ -65,7 +72,7 @@ public class OracleWallVisitor extends OracleASTVIsitorAdapter implements WallVi WallVisitorUtils.check(this, x); return true; } - + public boolean visit(SQLInListExpr x) { WallVisitorUtils.check(this, x); return true; @@ -190,11 +197,11 @@ public class OracleWallVisitor extends OracleASTVIsitorAdapter implements WallVi return true; } - + @Override public boolean visit(InsertIntoClause x) { WallVisitorUtils.checkInsert(this, x); - + return true; } diff --git a/src/main/java/com/alibaba/druid/wall/spi/SQLServerProvider.java b/src/main/java/com/alibaba/druid/wall/spi/SQLServerProvider.java index 7188dc6cb8..dd419d00b4 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerProvider.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerProvider.java @@ -37,7 +37,7 @@ public class SQLServerProvider extends WallProvider { @Override public WallVisitor createWallVisitor() { - return new SQLServerWallVisitor(config); + return new SQLServerWallVisitor(this); } @Override diff --git a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java index bbf9c49e05..8795bd183a 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java @@ -27,20 +27,23 @@ import com.alibaba.druid.sql.dialect.sqlserver.visitor.SQLServerASTVisitor; import com.alibaba.druid.sql.dialect.sqlserver.visitor.SQLServerASTVisitorAdapter; import com.alibaba.druid.wall.Violation; import com.alibaba.druid.wall.WallConfig; +import com.alibaba.druid.wall.WallProvider; import com.alibaba.druid.wall.WallVisitor; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; - -public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements WallVisitor, SQLServerASTVisitor{ +public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements WallVisitor, SQLServerASTVisitor { private final WallConfig config; + private final WallProvider provider; private final List violations = new ArrayList(); - - /** - * @param config - */ - public SQLServerWallVisitor(WallConfig config) { - this.config = config; + + public SQLServerWallVisitor(WallProvider provider) { + this.config = provider.getConfig(); + this.provider = provider; + } + + public WallProvider getProvider() { + return provider; } @Override @@ -55,10 +58,10 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements @Override public boolean isPermitTable(String name) { - if(!config.isTableCheck()){ + if (!config.isTableCheck()) { return false; } - + name = WallVisitorUtils.form(name); return config.getPermitTables().contains(name); } @@ -67,7 +70,6 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements public String toSQL(SQLObject obj) { return SQLUtils.toSQLServerString(obj); } - public boolean visit(SQLIdentifierExpr x) { String name = x.getName(); @@ -82,7 +84,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements WallVisitorUtils.check(this, x); return true; } - + public boolean visit(SQLInListExpr x) { WallVisitorUtils.check(this, x); return true; @@ -147,7 +149,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements } else if (x instanceof SQLUpdateStatement) { } else if (x instanceof SQLTruncateStatement) { if (!config.isTruncateAllow()) { - violations.add(new IllegalSQLObjectViolation(toSQL(x))); + violations.add(new IllegalSQLObjectViolation(toSQL(x))); } } else { violations.add(new IllegalSQLObjectViolation(toSQL(x))); @@ -186,7 +188,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements return true; } - + public boolean visit(SQLVariantRefExpr x) { String varName = x.getName(); if (varName == null) { @@ -199,7 +201,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements return false; } - + @Override public boolean visit(SQLServerObjectReferenceExpr x) { if (x.getSchema() != null && config.isPermitSchema(x.getSchema())) { 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 9692540426..392bc00624 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -41,6 +41,7 @@ import com.alibaba.druid.sql.ast.statement.SQLTableSource; import com.alibaba.druid.sql.ast.statement.SQLUnionQuery; import com.alibaba.druid.sql.ast.statement.SQLUpdateStatement; import com.alibaba.druid.sql.dialect.mysql.ast.expr.MySqlBooleanExpr; +import com.alibaba.druid.sql.visitor.ExportParameterVisitor; import com.alibaba.druid.util.JdbcUtils; import com.alibaba.druid.wall.WallVisitor; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; @@ -50,24 +51,11 @@ public class WallVisitorUtils { private final static Log LOG = LogFactory.getLog(WallVisitorUtils.class); public static void check(WallVisitor visitor, SQLInListExpr x) { - if (visitor.getConfig().isMustParameterized()) { - for (SQLExpr param : x.getTargetList()) { - if (getValue(param) != null) { - visitor.getViolations().add(new IllegalSQLObjectViolation(visitor.toSQL(x))); - break; - } - } - } + } public static void check(WallVisitor visitor, SQLBinaryOpExpr x) { - if (x.getOperator().isRelational() && visitor.getConfig().isMustParameterized()) { - if (x.getLeft() instanceof SQLName && getValue(x.getRight()) != null) { - visitor.getViolations().add(new IllegalSQLObjectViolation(visitor.toSQL(x))); - } else if (x.getRight() instanceof SQLName && getValue(x.getLeft()) != null) { - visitor.getViolations().add(new IllegalSQLObjectViolation(visitor.toSQL(x))); - } - } + } public static void check(WallVisitor visitor, SQLPropertyExpr x) { @@ -97,19 +85,34 @@ public class WallVisitorUtils { } SQLExpr where = x.getWhere(); - if (where != null && Boolean.TRUE == getValue(where)) { - if (where instanceof SQLBinaryOpExpr) { - SQLBinaryOpExpr binaryOpExpr = (SQLBinaryOpExpr) where; - if (binaryOpExpr.getOperator() == SQLBinaryOperator.Equality - || binaryOpExpr.getOperator() == SQLBinaryOperator.NotEqual) { - if (binaryOpExpr.getLeft() instanceof SQLIntegerExpr - && binaryOpExpr.getRight() instanceof SQLIntegerExpr) { - return; - } + + // if (visitor.getConfig().isMustParameterized()) { + if (where != null) { + if (visitor.getConfig().isMustParameterized()) { + ExportParameterVisitor exportParameterVisitor = visitor.getProvider().createExportParameterVisitor(); + where.accept(exportParameterVisitor); + + if (exportParameterVisitor.getParameters().size() > 0) { + addViolation(visitor, x); + return; } } - addViolation(visitor, x); + if (Boolean.TRUE == getValue(where)) { + if (where instanceof SQLBinaryOpExpr) { + SQLBinaryOpExpr binaryOpExpr = (SQLBinaryOpExpr) where; + if (binaryOpExpr.getOperator() == SQLBinaryOperator.Equality + || binaryOpExpr.getOperator() == SQLBinaryOperator.NotEqual) { + if (binaryOpExpr.getLeft() instanceof SQLIntegerExpr + && binaryOpExpr.getRight() instanceof SQLIntegerExpr) { + return; + } + } + } + + addViolation(visitor, x); + } + } } diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/MustParameterizedTest3.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/MustParameterizedTest3.java index 1e65c74cca..f6a77f6fa2 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/MustParameterizedTest3.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/MustParameterizedTest3.java @@ -9,8 +9,6 @@ import com.alibaba.druid.wall.WallUtils; public class MustParameterizedTest3 extends TestCase { - private String sql = "select * from t where id not in (3, 5)"; - private WallConfig config = new WallConfig(); protected void setUp() throws Exception { @@ -18,11 +16,10 @@ public class MustParameterizedTest3 extends TestCase { } public void testMySql() throws Exception { - Assert.assertFalse(WallUtils.isValidateMySql(sql, config)); + Assert.assertFalse(WallUtils.isValidateMySql("select * from t where id = (3 + 5 - 2 - 1)", config)); + Assert.assertFalse(WallUtils.isValidateMySql("select * from t where id != id + 3", config)); + Assert.assertTrue(WallUtils.isValidateMySql("select * from t where id = ? ORDER BY 1", config)); + Assert.assertTrue(WallUtils.isValidateMySql("select 1, 2, 3 from t where id = ?", config)); } - public void testORACLE() throws Exception { - - Assert.assertFalse(WallUtils.isValidateOracle(sql, config)); - } } diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/MustParameterizedTest4.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/MustParameterizedTest4.java new file mode 100644 index 0000000000..82677243f2 --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/MustParameterizedTest4.java @@ -0,0 +1,28 @@ +package com.alibaba.druid.bvt.filter.wall; + +import junit.framework.TestCase; + +import org.junit.Assert; + +import com.alibaba.druid.wall.WallConfig; +import com.alibaba.druid.wall.WallUtils; + +public class MustParameterizedTest4 extends TestCase { + + private String sql = "select * from t where id not in (3, 5)"; + + private WallConfig config = new WallConfig(); + + protected void setUp() throws Exception { + config.setMustParameterized(true); + } + + public void testMySql() throws Exception { + Assert.assertFalse(WallUtils.isValidateMySql(sql, config)); + } + + public void testORACLE() throws Exception { + + Assert.assertFalse(WallUtils.isValidateOracle(sql, config)); + } +}