From 0981b4f83b1cf58eefddd7e705b2b33c0cafdcfa Mon Sep 17 00:00:00 2001 From: yakolee Date: Fri, 14 Mar 2014 13:38:39 +0800 Subject: [PATCH] improve sql wall --- .../java/com/alibaba/druid/wall/WallContext.java | 12 ++++++------ .../java/com/alibaba/druid/wall/WallProvider.java | 6 +----- .../alibaba/druid/wall/spi/WallVisitorUtils.java | 7 ++----- .../wall/violation/SyntaxErrorViolation.java | 4 ++-- .../druid/bvt/filter/wall/MySqlWallTest134.java | 3 ++- .../druid/bvt/filter/wall/MySqlWallTest49.java | 2 +- .../druid/bvt/filter/wall/MySqlWallTest62.java | 15 ++++++++++++++- 7 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/alibaba/druid/wall/WallContext.java b/src/main/java/com/alibaba/druid/wall/WallContext.java index 037b19db4d..602373b4f0 100644 --- a/src/main/java/com/alibaba/druid/wall/WallContext.java +++ b/src/main/java/com/alibaba/druid/wall/WallContext.java @@ -169,9 +169,9 @@ public class WallContext { } public void incrementUpdateNoneConditionWarnnings() { - if (this.updateNoneConditionWarnnings == 0) { - this.incrementWarnnings(); - } + // if (this.updateNoneConditionWarnnings == 0) { + // this.incrementWarnnings(); + // } this.updateNoneConditionWarnnings++; } @@ -180,9 +180,9 @@ public class WallContext { } public void incrementDeleteNoneConditionWarnnings() { - if (this.deleteNoneConditionWarnnings == 0) { - this.incrementWarnnings(); - } + // if (this.deleteNoneConditionWarnnings == 0) { + // this.incrementWarnnings(); + // } this.deleteNoneConditionWarnnings++; } diff --git a/src/main/java/com/alibaba/druid/wall/WallProvider.java b/src/main/java/com/alibaba/druid/wall/WallProvider.java index 8447569c84..f8da88399f 100644 --- a/src/main/java/com/alibaba/druid/wall/WallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/WallProvider.java @@ -665,11 +665,7 @@ public abstract class WallProvider { } if (visitor.getViolations().size() == 0 && context != null && context.getWarnnings() >= 2) { - if (context.getDeleteNoneConditionWarnnings() > 0) { - violations.add(new IllegalSQLObjectViolation(ErrorCode.NONE_CONDITION, "delete none condition", sql)); - } else if (context.getUpdateNoneConditionWarnnings() > 0) { - violations.add(new IllegalSQLObjectViolation(ErrorCode.NONE_CONDITION, "update none condition", sql)); - } else if (context.getCommentCount() > 0) { + if (context.getCommentCount() > 0) { violations.add(new IllegalSQLObjectViolation(ErrorCode.COMMIT_NOT_ALLOW, "comment not allow", sql)); } else if (context.getLikeNumberWarnnings() > 0) { violations.add(new IllegalSQLObjectViolation(ErrorCode.COMMIT_NOT_ALLOW, "like number", sql)); 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 3f1c2c2f5d..6760ddf5f8 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -277,10 +277,6 @@ public class WallVisitorUtils { Object whereValue = getConditionValue(visitor, where, visitor.getConfig().isSelectWhereAlwayTrueCheck()); if (Boolean.TRUE == whereValue) { - if (queryBlockFromIsNull(visitor, x, false)) { - addViolation(visitor, ErrorCode.EMPTY_QUERY_HAS_CONDITION, "empty select has condition", x); - } - if (!isSimpleConstExpr(where)) {// 简单表达式 addViolation(visitor, ErrorCode.ALWAY_TRUE, "select alway true condition not allow", x); } @@ -2102,7 +2098,8 @@ public class WallVisitorUtils { return; } - if (WallVisitorUtils.queryBlockFromIsNull(visitor, x.getRight())) { + if (!WallVisitorUtils.queryBlockFromIsNull(visitor, x.getLeft()) + && WallVisitorUtils.queryBlockFromIsNull(visitor, x.getRight())) { boolean isTopUpdateStatement = false; boolean isTopInsertStatement = false; SQLObject selectParent = x.getParent(); diff --git a/src/main/java/com/alibaba/druid/wall/violation/SyntaxErrorViolation.java b/src/main/java/com/alibaba/druid/wall/violation/SyntaxErrorViolation.java index 2c33800d7d..313d97a9d0 100644 --- a/src/main/java/com/alibaba/druid/wall/violation/SyntaxErrorViolation.java +++ b/src/main/java/com/alibaba/druid/wall/violation/SyntaxErrorViolation.java @@ -43,10 +43,10 @@ public class SyntaxErrorViolation implements Violation { public String getMessage() { if (exception == null) { - return null; + return "syntax error"; } - return exception.getMessage(); + return "syntax error: " + exception.getMessage(); } @Override diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest134.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest134.java index 1ee91281f8..cf7267c437 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest134.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest134.java @@ -27,7 +27,8 @@ public class MySqlWallTest134 extends TestCase { public void test_false() throws Exception { WallProvider provider = new MySqlWallProvider(); - Assert.assertFalse(provider.checkValid("SELECT 1 FROM dual WHERE 1 = '1'''''''''''''UNION SELECT '2';")); + Assert.assertTrue(provider.checkValid("SELECT 1 FROM dual WHERE 1 = '1'''''''''''''UNION SELECT '2';")); + Assert.assertFalse(provider.checkValid("SELECT 1 FROM t WHERE 1 = '1'''''''''''''UNION SELECT '2';")); } } diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest49.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest49.java index 3c8be0c14f..af88285720 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest49.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest49.java @@ -34,7 +34,7 @@ public class MySqlWallTest49 extends TestCase { public void test_false() throws Exception { WallProvider provider = new MySqlWallProvider(); - Assert.assertFalse(provider.checkValid(// + Assert.assertTrue(provider.checkValid(// "select temp.*, u.CanComment, u.CanBeShared, " + // " u.CanForward, COALESCE(b.UserID,0) as isBlocked, " + // " COALESCE(f.UserID,0) as Followed, COALESCE(ff.UserID,0) as IsFollowed, " + // diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest62.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest62.java index bef7784adc..0c8035cce1 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest62.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/MySqlWallTest62.java @@ -35,7 +35,7 @@ public class MySqlWallTest62 extends TestCase { WallProvider provider = new MySqlWallProvider(); provider.getConfig().setSchemaCheck(true); - Assert.assertFalse(provider.checkValid(// + Assert.assertTrue(provider.checkValid(// "select temp.*, u.CanComment, u.CanBeShared, u.CanForward, COALESCE(b.UserID,0) as isBlocked" + // " , COALESCE(f.UserID,0) as Followed, COALESCE(ff.UserID,0) as IsFollowed" + // " , COALESCE(ul.UserID,0) as liked, COALESCE(fff.UserID,0) as RIsFollowed " + // @@ -50,6 +50,19 @@ public class MySqlWallTest62 extends TestCase { Assert.assertEquals(4, provider.getTableStats().size()); } + + public void test_false() throws Exception { + WallProvider provider = new MySqlWallProvider(); + provider.getConfig().setSchemaCheck(true); + String sql = "SELECT 1, 2, 3" + // + " UNION ALL SELECT a from tt " + // + " UNION ALL SELECT 2 FROM dual "; + Assert.assertFalse(provider.checkValid(sql)); + + sql = "SELECT a from t UNION ALL SELECT 2 FROM dual "; + Assert.assertFalse(provider.checkValid(sql)); + } + }