From c6182111d94f33b4251b8ef585622c86f6234576 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 08:41:41 +0800 Subject: [PATCH 1/8] improve WallFilter --- .../com/alibaba/druid/wall/WallContext.java | 34 ++++ .../com/alibaba/druid/wall/WallFilter.java | 188 ++++++++++++++---- .../alibaba/druid/wall/WallFilterMBean.java | 6 + .../com/alibaba/druid/wall/WallProvider.java | 51 +++-- .../com/alibaba/druid/wall/WallSqlStat.java | 20 ++ 5 files changed, 237 insertions(+), 62 deletions(-) create mode 100644 src/main/java/com/alibaba/druid/wall/WallContext.java create mode 100644 src/main/java/com/alibaba/druid/wall/WallSqlStat.java diff --git a/src/main/java/com/alibaba/druid/wall/WallContext.java b/src/main/java/com/alibaba/druid/wall/WallContext.java new file mode 100644 index 0000000000..96997cac14 --- /dev/null +++ b/src/main/java/com/alibaba/druid/wall/WallContext.java @@ -0,0 +1,34 @@ +package com.alibaba.druid.wall; + +public class WallContext { + + private final static ThreadLocal contextLocal = new ThreadLocal(); + + private WallSqlStat sqlState; + + public static WallContext createIfNotExists() { + WallContext context = contextLocal.get(); + if (context == null) { + context = new WallContext(); + contextLocal.set(context); + } + return context; + } + + public static WallContext current() { + return contextLocal.get(); + } + + public static void clearContext() { + contextLocal.remove(); + } + + public WallSqlStat getSqlState() { + return sqlState; + } + + public void setSqlState(WallSqlStat sqlState) { + this.sqlState = sqlState; + } + +} diff --git a/src/main/java/com/alibaba/druid/wall/WallFilter.java b/src/main/java/com/alibaba/druid/wall/WallFilter.java index 105fe339a0..81ccc0d57f 100644 --- a/src/main/java/com/alibaba/druid/wall/WallFilter.java +++ b/src/main/java/com/alibaba/druid/wall/WallFilter.java @@ -159,139 +159,235 @@ public class WallFilter extends FilterAdapter implements WallFilterMBean { @Override public void statement_addBatch(FilterChain chain, StatementProxy statement, String sql) throws SQLException { - sql = check(sql); - chain.statement_addBatch(statement, sql); + WallContext.createIfNotExists(); + try { + sql = check(sql); + chain.statement_addBatch(statement, sql); + } finally { + WallContext.clearContext(); + } } @Override public PreparedStatementProxy connection_prepareStatement(FilterChain chain, ConnectionProxy connection, String sql) throws SQLException { - sql = check(sql); - return chain.connection_prepareStatement(connection, sql); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareStatement(connection, sql); + } finally { + WallContext.clearContext(); + } } @Override public PreparedStatementProxy connection_prepareStatement(FilterChain chain, ConnectionProxy connection, String sql, int autoGeneratedKeys) throws SQLException { - sql = check(sql); - return chain.connection_prepareStatement(connection, sql, autoGeneratedKeys); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareStatement(connection, sql, autoGeneratedKeys); + } finally { + WallContext.clearContext(); + } } @Override public PreparedStatementProxy connection_prepareStatement(FilterChain chain, ConnectionProxy connection, String sql, int resultSetType, int resultSetConcurrency) throws SQLException { - sql = check(sql); - return chain.connection_prepareStatement(connection, sql, resultSetType, resultSetConcurrency); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareStatement(connection, sql, resultSetType, resultSetConcurrency); + } finally { + WallContext.clearContext(); + } } @Override public PreparedStatementProxy connection_prepareStatement(FilterChain chain, ConnectionProxy connection, String sql, int resultSetType, int resultSetConcurrency, int resultSetHoldability) throws SQLException { - sql = check(sql); - return chain.connection_prepareStatement(connection, sql, resultSetType, resultSetConcurrency, - resultSetHoldability); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareStatement(connection, sql, resultSetType, resultSetConcurrency, + resultSetHoldability); + } finally { + WallContext.clearContext(); + } } @Override public PreparedStatementProxy connection_prepareStatement(FilterChain chain, ConnectionProxy connection, String sql, int[] columnIndexes) throws SQLException { - sql = check(sql); - return chain.connection_prepareStatement(connection, sql, columnIndexes); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareStatement(connection, sql, columnIndexes); + } finally { + WallContext.clearContext(); + } } @Override public PreparedStatementProxy connection_prepareStatement(FilterChain chain, ConnectionProxy connection, String sql, String[] columnNames) throws SQLException { - sql = check(sql); - return chain.connection_prepareStatement(connection, sql, columnNames); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareStatement(connection, sql, columnNames); + } finally { + WallContext.clearContext(); + } } @Override public CallableStatementProxy connection_prepareCall(FilterChain chain, ConnectionProxy connection, String sql) throws SQLException { - sql = check(sql); - return chain.connection_prepareCall(connection, sql); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareCall(connection, sql); + } finally { + WallContext.clearContext(); + } } @Override public CallableStatementProxy connection_prepareCall(FilterChain chain, ConnectionProxy connection, String sql, int resultSetType, int resultSetConcurrency) throws SQLException { - sql = check(sql); - return chain.connection_prepareCall(connection, sql, resultSetType, resultSetConcurrency); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareCall(connection, sql, resultSetType, resultSetConcurrency); + } finally { + WallContext.clearContext(); + } } @Override public CallableStatementProxy connection_prepareCall(FilterChain chain, ConnectionProxy connection, String sql, int resultSetType, int resultSetConcurrency, int resultSetHoldability) throws SQLException { - sql = check(sql); - return chain.connection_prepareCall(connection, sql, resultSetType, resultSetConcurrency, resultSetHoldability); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.connection_prepareCall(connection, sql, resultSetType, resultSetConcurrency, + resultSetHoldability); + } finally { + WallContext.clearContext(); + } } // ////////////// @Override public boolean statement_execute(FilterChain chain, StatementProxy statement, String sql) throws SQLException { - sql = check(sql); - return chain.statement_execute(statement, sql); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_execute(statement, sql); + } finally { + WallContext.clearContext(); + } } @Override public boolean statement_execute(FilterChain chain, StatementProxy statement, String sql, int autoGeneratedKeys) throws SQLException { - sql = check(sql); - return chain.statement_execute(statement, sql, autoGeneratedKeys); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_execute(statement, sql, autoGeneratedKeys); + } finally { + WallContext.clearContext(); + } } @Override public boolean statement_execute(FilterChain chain, StatementProxy statement, String sql, int columnIndexes[]) throws SQLException { - sql = check(sql); - return chain.statement_execute(statement, sql, columnIndexes); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_execute(statement, sql, columnIndexes); + } finally { + WallContext.clearContext(); + } } @Override public boolean statement_execute(FilterChain chain, StatementProxy statement, String sql, String columnNames[]) throws SQLException { - sql = check(sql); - return chain.statement_execute(statement, sql, columnNames); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_execute(statement, sql, columnNames); + } finally { + WallContext.clearContext(); + } } @Override public ResultSetProxy statement_executeQuery(FilterChain chain, StatementProxy statement, String sql) throws SQLException { - sql = check(sql); - return chain.statement_executeQuery(statement, sql); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_executeQuery(statement, sql); + } finally { + WallContext.clearContext(); + } } @Override public int statement_executeUpdate(FilterChain chain, StatementProxy statement, String sql) throws SQLException { - sql = check(sql); - return chain.statement_executeUpdate(statement, sql); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_executeUpdate(statement, sql); + } finally { + WallContext.clearContext(); + } } @Override public int statement_executeUpdate(FilterChain chain, StatementProxy statement, String sql, int autoGeneratedKeys) throws SQLException { - sql = check(sql); - return chain.statement_executeUpdate(statement, sql, autoGeneratedKeys); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_executeUpdate(statement, sql, autoGeneratedKeys); + } finally { + WallContext.clearContext(); + } } @Override public int statement_executeUpdate(FilterChain chain, StatementProxy statement, String sql, int columnIndexes[]) throws SQLException { - sql = check(sql); - return chain.statement_executeUpdate(statement, sql, columnIndexes); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_executeUpdate(statement, sql, columnIndexes); + } finally { + WallContext.clearContext(); + } } @Override public int statement_executeUpdate(FilterChain chain, StatementProxy statement, String sql, String columnNames[]) throws SQLException { - sql = check(sql); - return chain.statement_executeUpdate(statement, sql, columnNames); + WallContext.createIfNotExists(); + try { + sql = check(sql); + return chain.statement_executeUpdate(statement, sql, columnNames); + } finally { + WallContext.clearContext(); + } } public String check(String sql) throws SQLException { @@ -355,7 +451,7 @@ public class WallFilter extends FilterAdapter implements WallFilterMBean { if (isLogViolation()) { LOG.error("not support method : Connection.getMetdataData"); } - + if (throwException) { throw new WallSQLException("not support method : Connection.getMetdataData"); } else { @@ -369,4 +465,16 @@ public class WallFilter extends FilterAdapter implements WallFilterMBean { public long getViolationCount() { return this.violationCount.get(); } + + public void resetViolationCount() { + this.violationCount.set(0); + } + + public void clearWhiteList() { + this.provider.clearCache(); + } + + public boolean checkValid(String sql) { + return provider.checkValid(sql); + } } diff --git a/src/main/java/com/alibaba/druid/wall/WallFilterMBean.java b/src/main/java/com/alibaba/druid/wall/WallFilterMBean.java index 297c12f336..bd2f51450c 100644 --- a/src/main/java/com/alibaba/druid/wall/WallFilterMBean.java +++ b/src/main/java/com/alibaba/druid/wall/WallFilterMBean.java @@ -39,4 +39,10 @@ public interface WallFilterMBean { String check(String sql) throws SQLException; long getViolationCount(); + + void resetViolationCount(); + + void clearWhiteList(); + + boolean checkValid(String sql); } diff --git a/src/main/java/com/alibaba/druid/wall/WallProvider.java b/src/main/java/com/alibaba/druid/wall/WallProvider.java index 92a15f36b6..4ec7e9d2f9 100644 --- a/src/main/java/com/alibaba/druid/wall/WallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/WallProvider.java @@ -32,20 +32,17 @@ import com.alibaba.druid.wall.violation.SyntaxErrorViolation; public abstract class WallProvider { - // Dummy value to associate with an Object in the backing Map - private static final Object PRESENT = new Object(); + private LinkedHashMap whiteList; - private LinkedHashMap whiteList; + private int whileListMaxSize = 1024; - private int whileListMaxSize = 1024; + private int whiteSqlMaxLength = 1024; // 1k - private int whiteSqlMaxLength = 1024; // 1k + protected final WallConfig config; - protected final WallConfig config; + private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - - private static final ThreadLocal privileged = new ThreadLocal(); + private static final ThreadLocal privileged = new ThreadLocal(); public WallProvider(WallConfig config){ this.config = config; @@ -59,10 +56,10 @@ public abstract class WallProvider { lock.writeLock().lock(); try { if (whiteList == null) { - whiteList = new LRUCache(whileListMaxSize); + whiteList = new LRUCache(whileListMaxSize); } - whiteList.put(sql, PRESENT); + whiteList.put(sql, new WallSqlStat()); } finally { lock.writeLock().unlock(); } @@ -81,8 +78,12 @@ public abstract class WallProvider { return hashSet; } - + public void clearCache() { + clearWhiteList(); + } + + public void clearWhiteList() { lock.writeLock().lock(); try { if (whiteList != null) { @@ -92,20 +93,24 @@ public abstract class WallProvider { lock.writeLock().unlock(); } } - - public boolean whiteContains(String sql) { + + public WallSqlStat getWhiteSql(String sql) { lock.readLock().lock(); try { if (whiteList == null) { - return false; + return null; } - - return whiteList.get(sql) != null; + + return whiteList.get(sql); } finally { lock.readLock().unlock(); } } + public boolean whiteContains(String sql) { + return getWhiteSql(sql) != null; + } + public abstract SQLStatementParser createParser(String sql); public abstract WallVisitor createWallVisitor(); @@ -113,7 +118,8 @@ public abstract class WallProvider { public abstract ExportParameterVisitor createExportParameterVisitor(); public boolean checkValid(String sql) { - return check(sql).getViolations().isEmpty(); + WallCheckResult result = check(sql); + return result.getViolations().isEmpty(); } public WallCheckResult check(String sql) { @@ -124,8 +130,9 @@ public abstract class WallProvider { } // first step, check whiteList - boolean isWhite = whiteContains(sql); - if (isWhite) { + WallSqlStat sqlStat = getWhiteSql(sql); + if (sqlStat != null) { + sqlStat.incrementAndGetExecuteCount(); return result; } @@ -176,13 +183,13 @@ public abstract class WallProvider { result.getViolations().addAll(visitor.getViolations()); return result; } - + public static boolean ispPivileged() { Boolean value = privileged.get(); if (value == null) { return false; } - + return value.booleanValue(); } diff --git a/src/main/java/com/alibaba/druid/wall/WallSqlStat.java b/src/main/java/com/alibaba/druid/wall/WallSqlStat.java new file mode 100644 index 0000000000..224a676843 --- /dev/null +++ b/src/main/java/com/alibaba/druid/wall/WallSqlStat.java @@ -0,0 +1,20 @@ +package com.alibaba.druid.wall; + +import java.util.concurrent.atomic.AtomicLongFieldUpdater; + +public class WallSqlStat { + + private volatile long executeCount; + + final static AtomicLongFieldUpdater executeCountUpdater = AtomicLongFieldUpdater.newUpdater(WallSqlStat.class, + "executeCount"); + + public long incrementAndGetExecuteCount() { + return executeCountUpdater.incrementAndGet(this); + } + + public long getExecuteCount() { + return executeCount; + } + +} From 6efea7780777d08713e9083ccde1bffd2ecd09aa Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 08:59:49 +0800 Subject: [PATCH 2/8] refactor WallFilter, change permit > deny --- .../com/alibaba/druid/wall/WallConfig.java | 52 +++++++++---------- .../alibaba/druid/wall/WallConfigMBean.java | 16 +++--- .../com/alibaba/druid/wall/WallProvider.java | 2 +- .../com/alibaba/druid/wall/WallVisitor.java | 2 +- .../druid/wall/spi/MySqlWallVisitor.java | 4 +- .../druid/wall/spi/OracleWallVisitor.java | 6 +-- .../druid/wall/spi/SQLServerWallVisitor.java | 10 ++-- .../druid/wall/spi/WallVisitorUtils.java | 10 ++-- ...{permit-function.txt => deny-function.txt} | 0 .../{permit-schema.txt => deny-schema.txt} | 0 ...{permit-function.txt => deny-function.txt} | 0 .../{permit-object.txt => deny-object.txt} | 0 .../{permit-schema.txt => deny-schema.txt} | 0 .../{permit-table.txt => deny-table.txt} | 0 .../{permit-variant.txt => deny-variant.txt} | 0 15 files changed, 51 insertions(+), 51 deletions(-) rename src/main/resources/META-INF/druid/wall/mysql/{permit-function.txt => deny-function.txt} (100%) rename src/main/resources/META-INF/druid/wall/mysql/{permit-schema.txt => deny-schema.txt} (100%) rename src/main/resources/META-INF/druid/wall/oracle/{permit-function.txt => deny-function.txt} (100%) rename src/main/resources/META-INF/druid/wall/oracle/{permit-object.txt => deny-object.txt} (100%) rename src/main/resources/META-INF/druid/wall/oracle/{permit-schema.txt => deny-schema.txt} (100%) rename src/main/resources/META-INF/druid/wall/oracle/{permit-table.txt => deny-table.txt} (100%) rename src/main/resources/META-INF/druid/wall/oracle/{permit-variant.txt => deny-variant.txt} (100%) diff --git a/src/main/java/com/alibaba/druid/wall/WallConfig.java b/src/main/java/com/alibaba/druid/wall/WallConfig.java index 3cfb2f23b1..70e59fdbf2 100644 --- a/src/main/java/com/alibaba/druid/wall/WallConfig.java +++ b/src/main/java/com/alibaba/druid/wall/WallConfig.java @@ -63,11 +63,11 @@ public class WallConfig implements WallConfigMBean { private boolean doPrivilegedAllow = false; - protected final Set permitFunctions = new HashSet(); - protected final Set permitTables = new HashSet(); - protected final Set permitSchemas = new HashSet(); - protected final Set permitVariants = new HashSet(); - protected final Set permitObjects = new HashSet(); + protected final Set denyFunctions = new HashSet(); + protected final Set denyTables = new HashSet(); + protected final Set denySchemas = new HashSet(); + protected final Set denyVariants = new HashSet(); + protected final Set denyObjects = new HashSet(); protected final Set readOnlyTables = new HashSet(); @@ -159,11 +159,11 @@ public class WallConfig implements WallConfigMBean { dir = dir.substring(0, dir.length() - 1); } - loadResource(this.permitVariants, dir + "/permit-variant.txt"); - loadResource(this.permitSchemas, dir + "/permit-schema.txt"); - loadResource(this.permitFunctions, dir + "/permit-function.txt"); - loadResource(this.permitTables, dir + "/permit-table.txt"); - loadResource(this.permitObjects, dir + "/permit-object.txt"); + loadResource(this.denyVariants, dir + "/deny-variant.txt"); + loadResource(this.denySchemas, dir + "/deny-schema.txt"); + loadResource(this.denyFunctions, dir + "/deny-function.txt"); + loadResource(this.denyTables, dir + "/deny-table.txt"); + loadResource(this.denyObjects, dir + "/deny-object.txt"); loadResource(this.readOnlyTables, dir + "/readonly-table.txt"); } @@ -357,24 +357,24 @@ public class WallConfig implements WallConfigMBean { this.commentAllow = commentAllow; } - public Set getPermitFunctions() { - return permitFunctions; + public Set getDenyFunctions() { + return denyFunctions; } - public Set getPermitTables() { - return permitTables; + public Set getDenyTables() { + return denyTables; } - public Set getPermitSchemas() { - return permitSchemas; + public Set getDenySchemas() { + return denySchemas; } - public Set getPermitVariants() { - return permitVariants; + public Set getDenyVariants() { + return denyVariants; } - public Set getPermitObjects() { - return permitObjects; + public Set getDenyObjects() { + return denyObjects; } public Set getReadOnlyTables() { @@ -389,31 +389,31 @@ public class WallConfig implements WallConfigMBean { this.mustParameterized = mustParameterized; } - public boolean isPermitObjects(String name) { + public boolean isDenyObjects(String name) { if (!objectCheck) { return false; } name = WallVisitorUtils.form(name); - return permitObjects.contains(name); + return denyObjects.contains(name); } - public boolean isPermitSchema(String name) { + public boolean isDenySchema(String name) { if (!schemaCheck) { return false; } name = WallVisitorUtils.form(name); - return this.permitSchemas.contains(name); + return this.denySchemas.contains(name); } - public boolean isPermitFunction(String name) { + public boolean isDenyFunction(String name) { if (!functionCheck) { return false; } name = WallVisitorUtils.form(name); - return this.permitFunctions.contains(name); + return this.denyFunctions.contains(name); } public boolean isCallAllow() { diff --git a/src/main/java/com/alibaba/druid/wall/WallConfigMBean.java b/src/main/java/com/alibaba/druid/wall/WallConfigMBean.java index 317f59ea18..108eee17ac 100644 --- a/src/main/java/com/alibaba/druid/wall/WallConfigMBean.java +++ b/src/main/java/com/alibaba/druid/wall/WallConfigMBean.java @@ -113,22 +113,22 @@ public interface WallConfigMBean { void setCommentAllow(boolean commentAllow); - Set getPermitFunctions(); + Set getDenyFunctions(); - Set getPermitTables(); + Set getDenyTables(); - Set getPermitSchemas(); + Set getDenySchemas(); - Set getPermitVariants(); + Set getDenyVariants(); - Set getPermitObjects(); + Set getDenyObjects(); Set getReadOnlyTables(); - boolean isPermitObjects(String name); + boolean isDenyObjects(String name); - boolean isPermitSchema(String name); + boolean isDenySchema(String name); - boolean isPermitFunction(String name); + boolean isDenyFunction(String name); } diff --git a/src/main/java/com/alibaba/druid/wall/WallProvider.java b/src/main/java/com/alibaba/druid/wall/WallProvider.java index 4ec7e9d2f9..dd140d9f2d 100644 --- a/src/main/java/com/alibaba/druid/wall/WallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/WallProvider.java @@ -139,7 +139,7 @@ public abstract class WallProvider { SQLStatementParser parser = createParser(sql); if (!config.isCommentAllow()) { - parser.getLexer().setAllowComment(false); // permit comment + parser.getLexer().setAllowComment(false); // deny comment } try { diff --git a/src/main/java/com/alibaba/druid/wall/WallVisitor.java b/src/main/java/com/alibaba/druid/wall/WallVisitor.java index f1fccfeb24..34c852ed62 100644 --- a/src/main/java/com/alibaba/druid/wall/WallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/WallVisitor.java @@ -30,7 +30,7 @@ public interface WallVisitor extends SQLASTVisitor { void addViolation(Violation violation); - boolean isPermitTable(String name); + boolean isDenyTable(String name); String toSQL(SQLObject obj); } 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 8a50af9d04..e569976fbd 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java @@ -237,13 +237,13 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi } @Override - public boolean isPermitTable(String name) { + public boolean isDenyTable(String name) { if (!config.isTableCheck()) { return false; } name = WallVisitorUtils.form(name); - return config.getPermitTables().contains(name); + return config.getDenyTables().contains(name); } public void preVisit(SQLObject x) { 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 86de9d461d..0952403628 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java @@ -83,7 +83,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi public boolean visit(SQLIdentifierExpr x) { String name = x.getName(); name = WallVisitorUtils.form(name); - if (config.isVariantCheck() && config.getPermitVariants().contains(name)) { + if (config.isVariantCheck() && config.getDenyVariants().contains(name)) { getViolations().add(new IllegalSQLObjectViolation(toSQL(x))); } return true; @@ -158,7 +158,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi } @Override - public boolean isPermitTable(String name) { + public boolean isDenyTable(String name) { if (!config.isTableCheck()) { return false; } @@ -167,7 +167,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi if (name.startsWith("v$") || name.startsWith("v_$")) { return true; } - return config.getPermitTables().contains(name); + return config.getDenyTables().contains(name); } public void preVisit(SQLObject x) { 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 09fa50ee08..5ebfd98270 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java @@ -80,13 +80,13 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements } @Override - public boolean isPermitTable(String name) { + public boolean isDenyTable(String name) { if (!config.isTableCheck()) { return false; } name = WallVisitorUtils.form(name); - return config.getPermitTables().contains(name); + return config.getDenyTables().contains(name); } @Override @@ -97,7 +97,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements public boolean visit(SQLIdentifierExpr x) { String name = x.getName(); name = WallVisitorUtils.form(name); - if (config.isVariantCheck() && config.getPermitVariants().contains(name)) { + if (config.isVariantCheck() && config.getDenyVariants().contains(name)) { getViolations().add(new IllegalSQLObjectViolation(toSQL(x))); } return true; @@ -238,10 +238,10 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements @Override public boolean visit(SQLServerObjectReferenceExpr x) { - if (x.getSchema() != null && config.isPermitSchema(x.getSchema())) { + if (x.getSchema() != null && config.isDenySchema(x.getSchema())) { this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x))); } - if (x.getDatabase() != null && config.isPermitSchema(x.getDatabase())) { + if (x.getDatabase() != null && config.isDenySchema(x.getDatabase())) { this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x))); } return true; 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 bb29297f65..4a5ab161c9 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -737,7 +737,7 @@ public class WallVisitorUtils { String methodName = x.getMethodName(); - if (visitor.getConfig().isPermitFunction(methodName.toLowerCase())) { + if (visitor.getConfig().isDenyFunction(methodName.toLowerCase())) { addViolation(visitor, x); } @@ -747,11 +747,11 @@ public class WallVisitorUtils { if (x instanceof SQLName) { String owner = ((SQLName) x).getSimleName(); owner = WallVisitorUtils.form(owner); - if (visitor.getConfig().isPermitSchema(owner)) { + if (visitor.getConfig().isDenySchema(owner)) { addViolation(visitor, x); } - if (visitor.getConfig().isPermitObjects(owner)) { + if (visitor.getConfig().isDenyObjects(owner)) { addViolation(visitor, x); } } @@ -771,7 +771,7 @@ public class WallVisitorUtils { if (expr instanceof SQLName) { String tableName = ((SQLName) expr).getSimleName(); - if (visitor.isPermitTable(tableName)) { + if (visitor.isDenyTable(tableName)) { addViolation(visitor, x); } } @@ -879,7 +879,7 @@ public class WallVisitorUtils { } } } catch (IOException e) { - LOG.error("load oracle permit tables errror", e); + LOG.error("load oracle deny tables errror", e); } } } diff --git a/src/main/resources/META-INF/druid/wall/mysql/permit-function.txt b/src/main/resources/META-INF/druid/wall/mysql/deny-function.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/mysql/permit-function.txt rename to src/main/resources/META-INF/druid/wall/mysql/deny-function.txt diff --git a/src/main/resources/META-INF/druid/wall/mysql/permit-schema.txt b/src/main/resources/META-INF/druid/wall/mysql/deny-schema.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/mysql/permit-schema.txt rename to src/main/resources/META-INF/druid/wall/mysql/deny-schema.txt diff --git a/src/main/resources/META-INF/druid/wall/oracle/permit-function.txt b/src/main/resources/META-INF/druid/wall/oracle/deny-function.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/oracle/permit-function.txt rename to src/main/resources/META-INF/druid/wall/oracle/deny-function.txt diff --git a/src/main/resources/META-INF/druid/wall/oracle/permit-object.txt b/src/main/resources/META-INF/druid/wall/oracle/deny-object.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/oracle/permit-object.txt rename to src/main/resources/META-INF/druid/wall/oracle/deny-object.txt diff --git a/src/main/resources/META-INF/druid/wall/oracle/permit-schema.txt b/src/main/resources/META-INF/druid/wall/oracle/deny-schema.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/oracle/permit-schema.txt rename to src/main/resources/META-INF/druid/wall/oracle/deny-schema.txt diff --git a/src/main/resources/META-INF/druid/wall/oracle/permit-table.txt b/src/main/resources/META-INF/druid/wall/oracle/deny-table.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/oracle/permit-table.txt rename to src/main/resources/META-INF/druid/wall/oracle/deny-table.txt diff --git a/src/main/resources/META-INF/druid/wall/oracle/permit-variant.txt b/src/main/resources/META-INF/druid/wall/oracle/deny-variant.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/oracle/permit-variant.txt rename to src/main/resources/META-INF/druid/wall/oracle/deny-variant.txt From 2308716354b7b115b931fac635ea6bd9a9a818ef Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 09:33:50 +0800 Subject: [PATCH 3/8] refactor WallFilter, change permit > deny --- .../com/alibaba/druid/wall/WallConfig.java | 113 ++++++++++-------- .../druid/wall/spi/WallVisitorUtils.java | 9 +- ...{permit-function.txt => deny-function.txt} | 0 .../{permit-object.txt => deny-object.txt} | 0 .../{permit-schema.txt => deny-schema.txt} | 0 .../{permit-table.txt => deny-table.txt} | 0 .../{permit-variant.txt => deny-variant.txt} | 0 .../bvt/filter/wall/WallReadOnlyTest.java | 2 +- 8 files changed, 71 insertions(+), 53 deletions(-) rename src/main/resources/META-INF/druid/wall/sqlserver/{permit-function.txt => deny-function.txt} (100%) rename src/main/resources/META-INF/druid/wall/sqlserver/{permit-object.txt => deny-object.txt} (100%) rename src/main/resources/META-INF/druid/wall/sqlserver/{permit-schema.txt => deny-schema.txt} (100%) rename src/main/resources/META-INF/druid/wall/sqlserver/{permit-table.txt => deny-table.txt} (100%) rename src/main/resources/META-INF/druid/wall/sqlserver/{permit-variant.txt => deny-variant.txt} (100%) diff --git a/src/main/java/com/alibaba/druid/wall/WallConfig.java b/src/main/java/com/alibaba/druid/wall/WallConfig.java index 70e59fdbf2..dd474e28fe 100644 --- a/src/main/java/com/alibaba/druid/wall/WallConfig.java +++ b/src/main/java/com/alibaba/druid/wall/WallConfig.java @@ -17,69 +17,70 @@ package com.alibaba.druid.wall; import static com.alibaba.druid.wall.spi.WallVisitorUtils.loadResource; -import java.util.HashSet; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import com.alibaba.druid.wall.spi.WallVisitorUtils; public class WallConfig implements WallConfigMBean { - private boolean noneBaseStatementAllow = false; + private boolean noneBaseStatementAllow = false; - private boolean callAllow = true; - private boolean selelctAllow = true; - private boolean selectIntoAllow = true; - private boolean selectIntoOutfileAllow = false; - private boolean selectWhereAlwayTrueCheck = true; - private boolean selectHavingAlwayTrueCheck = true; - private boolean selectUnionCheck = true; + private boolean callAllow = true; + private boolean selelctAllow = true; + private boolean selectIntoAllow = true; + private boolean selectIntoOutfileAllow = false; + private boolean selectWhereAlwayTrueCheck = true; + private boolean selectHavingAlwayTrueCheck = true; + private boolean selectUnionCheck = true; - private boolean selectAllColumnAllow = true; + private boolean selectAllColumnAllow = true; - private boolean deleteAllow = true; - private boolean deleteWhereAlwayTrueCheck = true; + private boolean deleteAllow = true; + private boolean deleteWhereAlwayTrueCheck = true; - private boolean updateAllow = true; - private boolean updateWhereAlayTrueCheck = true; + private boolean updateAllow = true; + private boolean updateWhereAlayTrueCheck = true; - private boolean insertAllow = true; - private boolean mergeAllow = true; + private boolean insertAllow = true; + private boolean mergeAllow = true; - private boolean multiStatementAllow = false; + private boolean multiStatementAllow = false; - private boolean truncateAllow = false; + private boolean truncateAllow = false; - private boolean commentAllow = false; + private boolean commentAllow = false; - private boolean describeAllow = false; + private boolean describeAllow = false; - private boolean schemaCheck = true; - private boolean tableCheck = true; - private boolean functionCheck = true; - private boolean objectCheck = true; - private boolean variantCheck = true; + private boolean schemaCheck = true; + private boolean tableCheck = true; + private boolean functionCheck = true; + private boolean objectCheck = true; + private boolean variantCheck = true; - private boolean mustParameterized = false; + private boolean mustParameterized = false; - private boolean doPrivilegedAllow = false; + private boolean doPrivilegedAllow = false; - protected final Set denyFunctions = new HashSet(); - protected final Set denyTables = new HashSet(); - protected final Set denySchemas = new HashSet(); - protected final Set denyVariants = new HashSet(); - protected final Set denyObjects = new HashSet(); + protected final ConcurrentMap denyFunctions = new ConcurrentHashMap(); + protected final ConcurrentMap denyTables = new ConcurrentHashMap(); + protected final ConcurrentMap denySchemas = new ConcurrentHashMap(); + protected final ConcurrentMap denyVariants = new ConcurrentHashMap(); + protected final ConcurrentMap denyObjects = new ConcurrentHashMap(); - protected final Set readOnlyTables = new HashSet(); + protected final ConcurrentMap readOnlyTables = new ConcurrentHashMap(); - private String dir; + private String dir; - private boolean inited; + private boolean inited; - private String tenantTablePattern; - private String tenantColumn; + private String tenantTablePattern; + private String tenantColumn; - private boolean wrapAllow = true; - private boolean metadataAllow = true; + private boolean wrapAllow = true; + private boolean metadataAllow = true; public WallConfig(){ @@ -358,27 +359,43 @@ public class WallConfig implements WallConfigMBean { } public Set getDenyFunctions() { - return denyFunctions; + return denyFunctions.keySet(); } public Set getDenyTables() { - return denyTables; + return denyTables.keySet(); } public Set getDenySchemas() { - return denySchemas; + return denySchemas.keySet(); } public Set getDenyVariants() { - return denyVariants; + return denyVariants.keySet(); } public Set getDenyObjects() { - return denyObjects; + return denyObjects.keySet(); } public Set getReadOnlyTables() { - return readOnlyTables; + return readOnlyTables.keySet(); + } + + public void addReadOnlyTable(String tableName) { + this.readOnlyTables.putIfAbsent(tableName, new WallDenyStat()); + } + + public boolean checkReadOnly(String tableName) { + WallDenyStat wallStat = this.readOnlyTables.get(tableName); + + if (wallStat == null) { + return false; + } + + wallStat.incrementAndGetDenyCount(); + + return true; } public boolean isMustParameterized() { @@ -395,7 +412,7 @@ public class WallConfig implements WallConfigMBean { } name = WallVisitorUtils.form(name); - return denyObjects.contains(name); + return denyObjects.containsKey(name); } public boolean isDenySchema(String name) { @@ -404,7 +421,7 @@ public class WallConfig implements WallConfigMBean { } name = WallVisitorUtils.form(name); - return this.denySchemas.contains(name); + return this.denySchemas.containsKey(name); } public boolean isDenyFunction(String name) { @@ -413,7 +430,7 @@ public class WallConfig implements WallConfigMBean { } name = WallVisitorUtils.form(name); - return this.denyFunctions.contains(name); + return this.denyFunctions.containsKey(name); } public boolean isCallAllow() { 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 4a5ab161c9..35c733fff1 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -24,7 +24,7 @@ import java.net.URL; import java.util.ArrayList; import java.util.Enumeration; import java.util.List; -import java.util.Set; +import java.util.concurrent.ConcurrentMap; import com.alibaba.druid.sql.ast.SQLExpr; import com.alibaba.druid.sql.ast.SQLName; @@ -63,6 +63,7 @@ import com.alibaba.druid.support.logging.Log; import com.alibaba.druid.support.logging.LogFactory; import com.alibaba.druid.util.JdbcUtils; import com.alibaba.druid.util.ServletPathMatcher; +import com.alibaba.druid.wall.WallDenyStat; import com.alibaba.druid.wall.WallProvider; import com.alibaba.druid.wall.WallVisitor; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; @@ -350,7 +351,7 @@ public class WallVisitorUtils { if (tableName != null) { tableName = form(tableName); - if (visitor.getConfig().getReadOnlyTables().contains(tableName)) { + if (visitor.getConfig().checkReadOnly(tableName)) { addViolation(visitor, tableSource); } } @@ -850,7 +851,7 @@ public class WallVisitorUtils { return name; } - public static void loadResource(Set names, String resource) { + public static void loadResource(ConcurrentMap names, String resource) { try { Enumeration e = Thread.currentThread().getContextClassLoader().getResources(resource); while (e.hasMoreElements()) { @@ -868,7 +869,7 @@ public class WallVisitorUtils { line = line.trim(); if (line.length() > 0) { line = line.toLowerCase(); - names.add(line); + names.put(line, new WallDenyStat()); } } diff --git a/src/main/resources/META-INF/druid/wall/sqlserver/permit-function.txt b/src/main/resources/META-INF/druid/wall/sqlserver/deny-function.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/sqlserver/permit-function.txt rename to src/main/resources/META-INF/druid/wall/sqlserver/deny-function.txt diff --git a/src/main/resources/META-INF/druid/wall/sqlserver/permit-object.txt b/src/main/resources/META-INF/druid/wall/sqlserver/deny-object.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/sqlserver/permit-object.txt rename to src/main/resources/META-INF/druid/wall/sqlserver/deny-object.txt diff --git a/src/main/resources/META-INF/druid/wall/sqlserver/permit-schema.txt b/src/main/resources/META-INF/druid/wall/sqlserver/deny-schema.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/sqlserver/permit-schema.txt rename to src/main/resources/META-INF/druid/wall/sqlserver/deny-schema.txt diff --git a/src/main/resources/META-INF/druid/wall/sqlserver/permit-table.txt b/src/main/resources/META-INF/druid/wall/sqlserver/deny-table.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/sqlserver/permit-table.txt rename to src/main/resources/META-INF/druid/wall/sqlserver/deny-table.txt diff --git a/src/main/resources/META-INF/druid/wall/sqlserver/permit-variant.txt b/src/main/resources/META-INF/druid/wall/sqlserver/deny-variant.txt similarity index 100% rename from src/main/resources/META-INF/druid/wall/sqlserver/permit-variant.txt rename to src/main/resources/META-INF/druid/wall/sqlserver/deny-variant.txt diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest.java index a883648ce0..e6224353f8 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest.java @@ -31,7 +31,7 @@ public class WallReadOnlyTest extends TestCase { private WallConfig config = new WallConfig(); protected void setUp() throws Exception { - config.getReadOnlyTables().add("members"); + config.addReadOnlyTable("members"); } private String sql = "SELECT F1, F2 members"; private String insert_sql = "INSERT INTO members (FID, FNAME) VALUES (?, ?)"; From e751c2fce192dd3c9a72c8e3e62b44303029d55d Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 09:50:41 +0800 Subject: [PATCH 4/8] WallFilter refactor --- .../com/alibaba/druid/wall/WallConfig.java | 115 ++++++++---------- .../com/alibaba/druid/wall/WallDenyStat.java | 19 +++ .../druid/wall/spi/WallVisitorUtils.java | 9 +- 3 files changed, 76 insertions(+), 67 deletions(-) create mode 100644 src/main/java/com/alibaba/druid/wall/WallDenyStat.java diff --git a/src/main/java/com/alibaba/druid/wall/WallConfig.java b/src/main/java/com/alibaba/druid/wall/WallConfig.java index dd474e28fe..630bed9b3a 100644 --- a/src/main/java/com/alibaba/druid/wall/WallConfig.java +++ b/src/main/java/com/alibaba/druid/wall/WallConfig.java @@ -18,69 +18,68 @@ package com.alibaba.druid.wall; import static com.alibaba.druid.wall.spi.WallVisitorUtils.loadResource; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.ConcurrentSkipListSet; import com.alibaba.druid.wall.spi.WallVisitorUtils; public class WallConfig implements WallConfigMBean { - private boolean noneBaseStatementAllow = false; + private boolean noneBaseStatementAllow = false; - private boolean callAllow = true; - private boolean selelctAllow = true; - private boolean selectIntoAllow = true; - private boolean selectIntoOutfileAllow = false; - private boolean selectWhereAlwayTrueCheck = true; - private boolean selectHavingAlwayTrueCheck = true; - private boolean selectUnionCheck = true; + private boolean callAllow = true; + private boolean selelctAllow = true; + private boolean selectIntoAllow = true; + private boolean selectIntoOutfileAllow = false; + private boolean selectWhereAlwayTrueCheck = true; + private boolean selectHavingAlwayTrueCheck = true; + private boolean selectUnionCheck = true; - private boolean selectAllColumnAllow = true; + private boolean selectAllColumnAllow = true; - private boolean deleteAllow = true; - private boolean deleteWhereAlwayTrueCheck = true; + private boolean deleteAllow = true; + private boolean deleteWhereAlwayTrueCheck = true; - private boolean updateAllow = true; - private boolean updateWhereAlayTrueCheck = true; + private boolean updateAllow = true; + private boolean updateWhereAlayTrueCheck = true; - private boolean insertAllow = true; - private boolean mergeAllow = true; + private boolean insertAllow = true; + private boolean mergeAllow = true; - private boolean multiStatementAllow = false; + private boolean multiStatementAllow = false; - private boolean truncateAllow = false; + private boolean truncateAllow = false; - private boolean commentAllow = false; + private boolean commentAllow = false; - private boolean describeAllow = false; + private boolean describeAllow = false; - private boolean schemaCheck = true; - private boolean tableCheck = true; - private boolean functionCheck = true; - private boolean objectCheck = true; - private boolean variantCheck = true; + private boolean schemaCheck = true; + private boolean tableCheck = true; + private boolean functionCheck = true; + private boolean objectCheck = true; + private boolean variantCheck = true; - private boolean mustParameterized = false; + private boolean mustParameterized = false; - private boolean doPrivilegedAllow = false; + private boolean doPrivilegedAllow = false; - protected final ConcurrentMap denyFunctions = new ConcurrentHashMap(); - protected final ConcurrentMap denyTables = new ConcurrentHashMap(); - protected final ConcurrentMap denySchemas = new ConcurrentHashMap(); - protected final ConcurrentMap denyVariants = new ConcurrentHashMap(); - protected final ConcurrentMap denyObjects = new ConcurrentHashMap(); + protected final Set denyFunctions = new ConcurrentSkipListSet(); + protected final Set denyTables = new ConcurrentSkipListSet(); + protected final Set denySchemas = new ConcurrentSkipListSet(); + protected final Set denyVariants = new ConcurrentSkipListSet(); + protected final Set denyObjects = new ConcurrentSkipListSet(); - protected final ConcurrentMap readOnlyTables = new ConcurrentHashMap(); + protected final Set readOnlyTables = new ConcurrentSkipListSet(); - private String dir; + private String dir; - private boolean inited; + private boolean inited; - private String tenantTablePattern; - private String tenantColumn; + private String tenantTablePattern; + private String tenantColumn; - private boolean wrapAllow = true; - private boolean metadataAllow = true; + private boolean wrapAllow = true; + private boolean metadataAllow = true; public WallConfig(){ @@ -359,43 +358,35 @@ public class WallConfig implements WallConfigMBean { } public Set getDenyFunctions() { - return denyFunctions.keySet(); + return denyFunctions; } public Set getDenyTables() { - return denyTables.keySet(); + return denyTables; } public Set getDenySchemas() { - return denySchemas.keySet(); + return denySchemas; } public Set getDenyVariants() { - return denyVariants.keySet(); + return denyVariants; } public Set getDenyObjects() { - return denyObjects.keySet(); + return denyObjects; } public Set getReadOnlyTables() { - return readOnlyTables.keySet(); + return readOnlyTables; } - + public void addReadOnlyTable(String tableName) { - this.readOnlyTables.putIfAbsent(tableName, new WallDenyStat()); + this.readOnlyTables.add(tableName); } - - public boolean checkReadOnly(String tableName) { - WallDenyStat wallStat = this.readOnlyTables.get(tableName); - - if (wallStat == null) { - return false; - } - - wallStat.incrementAndGetDenyCount(); - - return true; + + public boolean isReadOnly(String tableName) { + return this.readOnlyTables.contains(tableName); } public boolean isMustParameterized() { @@ -412,7 +403,7 @@ public class WallConfig implements WallConfigMBean { } name = WallVisitorUtils.form(name); - return denyObjects.containsKey(name); + return denyObjects.contains(name); } public boolean isDenySchema(String name) { @@ -421,7 +412,7 @@ public class WallConfig implements WallConfigMBean { } name = WallVisitorUtils.form(name); - return this.denySchemas.containsKey(name); + return this.denySchemas.contains(name); } public boolean isDenyFunction(String name) { @@ -430,7 +421,7 @@ public class WallConfig implements WallConfigMBean { } name = WallVisitorUtils.form(name); - return this.denyFunctions.containsKey(name); + return this.denyFunctions.contains(name); } public boolean isCallAllow() { diff --git a/src/main/java/com/alibaba/druid/wall/WallDenyStat.java b/src/main/java/com/alibaba/druid/wall/WallDenyStat.java new file mode 100644 index 0000000000..1238b93164 --- /dev/null +++ b/src/main/java/com/alibaba/druid/wall/WallDenyStat.java @@ -0,0 +1,19 @@ +package com.alibaba.druid.wall; + +import java.util.concurrent.atomic.AtomicLongFieldUpdater; + +public class WallDenyStat { + + private volatile long denyCount; + + final static AtomicLongFieldUpdater denyCountUpdater = AtomicLongFieldUpdater.newUpdater(WallDenyStat.class, + "denyCount"); + + public long incrementAndGetDenyCount() { + return denyCountUpdater.incrementAndGet(this); + } + + public long getDenyCount() { + return denyCount; + } +} 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 35c733fff1..ce28b5d4f6 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -24,7 +24,7 @@ import java.net.URL; import java.util.ArrayList; import java.util.Enumeration; import java.util.List; -import java.util.concurrent.ConcurrentMap; +import java.util.Set; import com.alibaba.druid.sql.ast.SQLExpr; import com.alibaba.druid.sql.ast.SQLName; @@ -63,7 +63,6 @@ import com.alibaba.druid.support.logging.Log; import com.alibaba.druid.support.logging.LogFactory; import com.alibaba.druid.util.JdbcUtils; import com.alibaba.druid.util.ServletPathMatcher; -import com.alibaba.druid.wall.WallDenyStat; import com.alibaba.druid.wall.WallProvider; import com.alibaba.druid.wall.WallVisitor; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; @@ -351,7 +350,7 @@ public class WallVisitorUtils { if (tableName != null) { tableName = form(tableName); - if (visitor.getConfig().checkReadOnly(tableName)) { + if (visitor.getConfig().isReadOnly(tableName)) { addViolation(visitor, tableSource); } } @@ -851,7 +850,7 @@ public class WallVisitorUtils { return name; } - public static void loadResource(ConcurrentMap names, String resource) { + public static void loadResource(Set names, String resource) { try { Enumeration e = Thread.currentThread().getContextClassLoader().getResources(resource); while (e.hasMoreElements()) { @@ -869,7 +868,7 @@ public class WallVisitorUtils { line = line.trim(); if (line.length() > 0) { line = line.toLowerCase(); - names.put(line, new WallDenyStat()); + names.add(line); } } From 2548d88fc6961572239c2370e1c29b556dc05a20 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 10:48:21 +0800 Subject: [PATCH 5/8] improve WallFilter --- .../com/alibaba/druid/wall/WallProvider.java | 92 +++++++++++++++++-- .../druid/wall/spi/MySqlWallVisitor.java | 3 +- .../druid/wall/spi/OracleWallVisitor.java | 2 +- .../druid/wall/spi/SQLServerWallVisitor.java | 3 +- .../druid/wall/spi/WallVisitorUtils.java | 14 ++- .../bvt/filter/wall/WallDenyTableTest.java | 38 ++++++++ .../bvt/filter/wall/WallReadOnlyTest2.java | 69 ++++++++++++++ 7 files changed, 198 insertions(+), 23 deletions(-) create mode 100644 src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java create mode 100644 src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest2.java diff --git a/src/main/java/com/alibaba/druid/wall/WallProvider.java b/src/main/java/com/alibaba/druid/wall/WallProvider.java index dd140d9f2d..5dfba33679 100644 --- a/src/main/java/com/alibaba/druid/wall/WallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/WallProvider.java @@ -19,6 +19,8 @@ import java.security.PrivilegedAction; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.locks.ReentrantReadWriteLock; import com.alibaba.druid.sql.ast.SQLStatement; @@ -27,22 +29,26 @@ import com.alibaba.druid.sql.parser.SQLStatementParser; import com.alibaba.druid.sql.parser.Token; import com.alibaba.druid.sql.visitor.ExportParameterVisitor; import com.alibaba.druid.util.LRUCache; +import com.alibaba.druid.wall.spi.WallVisitorUtils; import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; import com.alibaba.druid.wall.violation.SyntaxErrorViolation; public abstract class WallProvider { - private LinkedHashMap whiteList; + private LinkedHashMap whiteList; - private int whileListMaxSize = 1024; + private int whileListMaxSize = 1024; - private int whiteSqlMaxLength = 1024; // 1k + private int whiteSqlMaxLength = 1024; // 1k - protected final WallConfig config; + protected final WallConfig config; - private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); + private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - private static final ThreadLocal privileged = new ThreadLocal(); + private static final ThreadLocal privileged = new ThreadLocal(); + + private final ConcurrentMap deniedTables = new ConcurrentHashMap(); + private final ConcurrentMap deniedFunctions = new ConcurrentHashMap(); public WallProvider(WallConfig config){ this.config = config; @@ -78,7 +84,7 @@ public abstract class WallProvider { return hashSet; } - + public void clearCache() { clearWhiteList(); } @@ -93,15 +99,15 @@ public abstract class WallProvider { lock.writeLock().unlock(); } } - + public WallSqlStat getWhiteSql(String sql) { lock.readLock().lock(); try { if (whiteList == null) { return null; } - - return whiteList.get(sql); + + return whiteList.get(sql); } finally { lock.readLock().unlock(); } @@ -122,6 +128,72 @@ public abstract class WallProvider { return result.getViolations().isEmpty(); } + public boolean checkDenyFunction(String functionName) { + if (functionName == null) { + return true; + } + + functionName = functionName.toLowerCase(); + if (getConfig().getDenyFunctions().contains(functionName)) { + WallDenyStat denyStat = this.deniedFunctions.get(functionName); + if (denyStat == null) { + this.deniedFunctions.putIfAbsent(functionName, new WallDenyStat()); + denyStat = this.deniedFunctions.get(functionName); + } + denyStat.incrementAndGetDenyCount(); + return false; + } + + return true; + } + + public boolean checkDenyTable(String tableName) { + if (tableName == null) { + return true; + } + + tableName = WallVisitorUtils.form(tableName); + if (getConfig().getDenyTables().contains(tableName)) { + WallDenyStat denyStat = this.deniedTables.get(tableName); + if (denyStat == null) { + this.deniedTables.putIfAbsent(tableName, new WallDenyStat()); + denyStat = this.deniedTables.get(tableName); + } + denyStat.incrementAndGetDenyCount(); + return false; + } + + return true; + } + + public boolean checkReadOnlyTable(String tableName) { + if (tableName == null) { + return true; + } + + tableName = WallVisitorUtils.form(tableName); + if (getConfig().isReadOnly(tableName)) { + WallDenyStat denyStat = this.deniedTables.get(tableName); + if (denyStat == null) { + this.deniedTables.putIfAbsent(tableName, new WallDenyStat()); + denyStat = this.deniedTables.get(tableName); + } + denyStat.incrementAndGetDenyCount(); + return false; + } + + return true; + } + + public WallDenyStat getDenniedTableStat(String tableName) { + if (tableName == null) { + return null; + } + + String formName = WallVisitorUtils.form(tableName); + return deniedTables.get(formName); + } + public WallCheckResult check(String sql) { WallCheckResult result = new WallCheckResult(); 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 e569976fbd..201fe6957c 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/MySqlWallVisitor.java @@ -242,8 +242,7 @@ public class MySqlWallVisitor extends MySqlASTVisitorAdapter implements WallVisi return false; } - name = WallVisitorUtils.form(name); - return config.getDenyTables().contains(name); + return !this.provider.checkDenyTable(name); } public void preVisit(SQLObject x) { 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 0952403628..e1137b6079 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java @@ -167,7 +167,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi if (name.startsWith("v$") || name.startsWith("v_$")) { return true; } - return config.getDenyTables().contains(name); + return !this.provider.checkDenyTable(name); } public void preVisit(SQLObject x) { 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 5ebfd98270..87d32ee7b3 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java @@ -85,8 +85,7 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements return false; } - name = WallVisitorUtils.form(name); - return config.getDenyTables().contains(name); + return !this.provider.checkDenyTable(name); } @Override 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 ce28b5d4f6..9ce4d37728 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -69,7 +69,7 @@ import com.alibaba.druid.wall.violation.IllegalSQLObjectViolation; public class WallVisitorUtils { - private final static Log LOG = LogFactory.getLog(WallVisitorUtils.class); + private final static Log LOG = LogFactory.getLog(WallVisitorUtils.class); public static void check(WallVisitor visitor, SQLInListExpr x) { @@ -88,7 +88,7 @@ public class WallVisitorUtils { && x.getParent() instanceof SQLSelectQueryBlock) { SQLSelectQueryBlock queryBlock = (SQLSelectQueryBlock) x.getParent(); SQLTableSource from = queryBlock.getFrom(); - + if (from instanceof SQLExprTableSource) { addViolation(visitor, x); } @@ -348,11 +348,9 @@ public class WallVisitorUtils { tableName = ((SQLName) tableNameExpr).getSimleName(); } - if (tableName != null) { - tableName = form(tableName); - if (visitor.getConfig().isReadOnly(tableName)) { - addViolation(visitor, tableSource); - } + boolean readOnlyValid = visitor.getProvider().checkReadOnlyTable(tableName); + if (!readOnlyValid) { + addViolation(visitor, tableSource); } } else if (tableSource instanceof SQLJoinTableSource) { SQLJoinTableSource join = (SQLJoinTableSource) tableSource; @@ -737,7 +735,7 @@ public class WallVisitorUtils { String methodName = x.getMethodName(); - if (visitor.getConfig().isDenyFunction(methodName.toLowerCase())) { + if (!visitor.getProvider().checkDenyFunction(methodName)) { addViolation(visitor, x); } diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java new file mode 100644 index 0000000000..a3f253989d --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java @@ -0,0 +1,38 @@ +/* + * 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.filter.wall; + +import junit.framework.TestCase; + +import org.junit.Assert; + +import com.alibaba.druid.wall.spi.OracleWallProvider; + +/** + * @author wenshao + * + */ +public class WallDenyTableTest extends TestCase { + + public void testORACLE() throws Exception { + String sql = "SELECT F1, F2 FROM ALL_ALL_TABLES"; + + OracleWallProvider provider = new OracleWallProvider(); + Assert.assertFalse(provider.checkValid(sql)); + + Assert.assertEquals(1, provider.getDenniedTableStat("ALL_ALL_TABLES").getDenyCount()); + } +} diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest2.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest2.java new file mode 100644 index 0000000000..7c189e049f --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallReadOnlyTest2.java @@ -0,0 +1,69 @@ +/* + * 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.filter.wall; + +import junit.framework.TestCase; + +import org.junit.Assert; + +import com.alibaba.druid.wall.WallConfig; +import com.alibaba.druid.wall.spi.MySqlWallProvider; +import com.alibaba.druid.wall.spi.OracleWallProvider; + +/** + * 这个场景，检测可疑的Having条件 + * @author wenshao + * + */ +public class WallReadOnlyTest2 extends TestCase { + private WallConfig config = new WallConfig(); + + protected void setUp() throws Exception { + config.addReadOnlyTable("members"); + } + private String sql = "SELECT F1, F2 members"; + private String insert_sql = "INSERT INTO members (FID, FNAME) VALUES (?, ?)"; + private String update_sql = "UPDATE members SET FNAME = ? WHERe FID = ?"; + private String delete_sql = "DELETE members WHERE FID = ?"; + + public void testMySql() throws Exception { + MySqlWallProvider provider = new MySqlWallProvider(config); + Assert.assertTrue(provider.checkValid(sql)); + + Assert.assertNull(provider.getDenniedTableStat("members")); + + Assert.assertFalse(provider.checkValid(insert_sql)); + Assert.assertFalse(provider.checkValid(update_sql)); + Assert.assertFalse(provider.checkValid(delete_sql)); + + Assert.assertEquals(3, provider.getDenniedTableStat("members").getDenyCount()); + Assert.assertEquals(3, provider.getDenniedTableStat("Members").getDenyCount()); + } + + public void testORACLE() throws Exception { + OracleWallProvider provider = new OracleWallProvider(config); + Assert.assertTrue(provider.checkValid(sql)); + + Assert.assertNull(provider.getDenniedTableStat("members")); + + Assert.assertFalse(provider.checkValid(insert_sql)); + Assert.assertFalse(provider.checkValid(update_sql)); + Assert.assertFalse(provider.checkValid(delete_sql)); + + Assert.assertEquals(3, provider.getDenniedTableStat("members").getDenyCount()); + Assert.assertEquals(3, provider.getDenniedTableStat("Members").getDenyCount()); + } +} From 80bbefa0881f709bc2f595927d0210d0f9c1ba52 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 11:04:28 +0800 Subject: [PATCH 6/8] improve WallFilter --- .../com/alibaba/druid/wall/WallProvider.java | 29 ++++++++++++++ .../druid/wall/spi/OracleWallVisitor.java | 3 +- .../druid/wall/spi/SQLServerWallVisitor.java | 4 +- .../druid/wall/spi/WallVisitorUtils.java | 20 +++++++--- .../bvt/filter/wall/WallDenySchemaTest.java | 38 +++++++++++++++++++ .../bvt/filter/wall/WallDenyTableTest.java | 4 +- 6 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenySchemaTest.java diff --git a/src/main/java/com/alibaba/druid/wall/WallProvider.java b/src/main/java/com/alibaba/druid/wall/WallProvider.java index 5dfba33679..9b5524aca9 100644 --- a/src/main/java/com/alibaba/druid/wall/WallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/WallProvider.java @@ -49,6 +49,7 @@ public abstract class WallProvider { private final ConcurrentMap deniedTables = new ConcurrentHashMap(); private final ConcurrentMap deniedFunctions = new ConcurrentHashMap(); + private final ConcurrentMap deniedSchemas = new ConcurrentHashMap(); public WallProvider(WallConfig config){ this.config = config; @@ -146,6 +147,25 @@ public abstract class WallProvider { return true; } + + public boolean checkDenySchema(String schemaName) { + if (schemaName == null) { + return true; + } + + schemaName = schemaName.toLowerCase(); + if (getConfig().getDenySchemas().contains(schemaName)) { + WallDenyStat denyStat = this.deniedSchemas.get(schemaName); + if (denyStat == null) { + this.deniedSchemas.putIfAbsent(schemaName, new WallDenyStat()); + denyStat = this.deniedSchemas.get(schemaName); + } + denyStat.incrementAndGetDenyCount(); + return false; + } + + return true; + } public boolean checkDenyTable(String tableName) { if (tableName == null) { @@ -193,6 +213,15 @@ public abstract class WallProvider { String formName = WallVisitorUtils.form(tableName); return deniedTables.get(formName); } + + public WallDenyStat getDenniedSchemaStat(String schemaName) { + if (schemaName == null) { + return null; + } + + String formName = WallVisitorUtils.form(schemaName); + return deniedSchemas.get(formName); + } public WallCheckResult check(String sql) { WallCheckResult result = new WallCheckResult(); 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 e1137b6079..879fd738c1 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/OracleWallVisitor.java @@ -112,8 +112,7 @@ public class OracleWallVisitor extends OracleASTVisitorAdapter implements WallVi } public boolean visit(OracleSelectTableReference x) { - WallVisitorUtils.check(this, x); - return true; + return WallVisitorUtils.check(this, x); } public boolean visit(SQLExprTableSource x) { 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 87d32ee7b3..f38e6f4ffe 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallVisitor.java @@ -237,10 +237,10 @@ public class SQLServerWallVisitor extends SQLServerASTVisitorAdapter implements @Override public boolean visit(SQLServerObjectReferenceExpr x) { - if (x.getSchema() != null && config.isDenySchema(x.getSchema())) { + if (x.getSchema() != null && !provider.checkDenySchema(x.getSchema())) { this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x))); } - if (x.getDatabase() != null && config.isDenySchema(x.getDatabase())) { + if (x.getDatabase() != null && !provider.checkDenySchema(x.getDatabase())) { this.getViolations().add(new IllegalSQLObjectViolation(this.toSQL(x))); } return true; 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 9ce4d37728..f1d73b0637 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java +++ b/src/main/java/com/alibaba/druid/wall/spi/WallVisitorUtils.java @@ -741,38 +741,48 @@ public class WallVisitorUtils { } - private static void checkSchema(WallVisitor visitor, SQLExpr x) { + private static boolean checkSchema(WallVisitor visitor, SQLExpr x) { if (x instanceof SQLName) { String owner = ((SQLName) x).getSimleName(); owner = WallVisitorUtils.form(owner); - if (visitor.getConfig().isDenySchema(owner)) { + if (!visitor.getProvider().checkDenySchema(owner)) { addViolation(visitor, x); + return false; } if (visitor.getConfig().isDenyObjects(owner)) { addViolation(visitor, x); + return false; } } // if (ownerExpr instanceof SQLPropertyExpr) { if (x instanceof SQLPropertyExpr) { - checkSchema(visitor, ((SQLPropertyExpr) x).getOwner()); + return checkSchema(visitor, ((SQLPropertyExpr) x).getOwner()); } + + return true; } - public static void check(WallVisitor visitor, SQLExprTableSource x) { + public static boolean check(WallVisitor visitor, SQLExprTableSource x) { SQLExpr expr = x.getExpr(); if (expr instanceof SQLPropertyExpr) { - checkSchema(visitor, ((SQLPropertyExpr) expr).getOwner()); + boolean checkResult = checkSchema(visitor, ((SQLPropertyExpr) expr).getOwner()); + if (!checkResult) { + return false; + } } if (expr instanceof SQLName) { String tableName = ((SQLName) expr).getSimleName(); if (visitor.isDenyTable(tableName)) { addViolation(visitor, x); + return false; } } + + return true; } private static void addViolation(WallVisitor visitor, SQLObject x) { diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenySchemaTest.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenySchemaTest.java new file mode 100644 index 0000000000..89dc026d83 --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenySchemaTest.java @@ -0,0 +1,38 @@ +/* + * 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.filter.wall; + +import junit.framework.TestCase; + +import org.junit.Assert; + +import com.alibaba.druid.wall.spi.OracleWallProvider; + +/** + * @author wenshao + * + */ +public class WallDenySchemaTest extends TestCase { + + public void testORACLE() throws Exception { + String sql = "SELECT F1, F2 FROM ALL_ALL_TABLES"; + + OracleWallProvider provider = new OracleWallProvider(); + Assert.assertFalse(provider.checkValid(sql)); + + Assert.assertEquals(1, provider.getDenniedTableStat("ALL_ALL_TABLES").getDenyCount()); + } +} diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java index a3f253989d..985fd51cb8 100644 --- a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallDenyTableTest.java @@ -28,11 +28,11 @@ import com.alibaba.druid.wall.spi.OracleWallProvider; public class WallDenyTableTest extends TestCase { public void testORACLE() throws Exception { - String sql = "SELECT F1, F2 FROM ALL_ALL_TABLES"; + String sql = "SELECT F1, F2 FROM SYS.ABC"; OracleWallProvider provider = new OracleWallProvider(); Assert.assertFalse(provider.checkValid(sql)); - Assert.assertEquals(1, provider.getDenniedTableStat("ALL_ALL_TABLES").getDenyCount()); + Assert.assertEquals(1, provider.getDenniedSchemaStat("SYS").getDenyCount()); } } From 88a6222d15850f772f8d08c28139a8780e2b51cb Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 11:41:35 +0800 Subject: [PATCH 7/8] improve WallFilter --- .../com/alibaba/druid/wall/WallDenyStat.java | 31 +++++++++++++++++-- .../com/alibaba/druid/wall/WallFilter.java | 6 ++-- .../com/alibaba/druid/wall/WallProvider.java | 21 +++++++++++-- .../com/alibaba/druid/wall/WallUtils.java | 6 ++-- ...ovider.java => SQLServerWallProvider.java} | 6 ++-- 5 files changed, 56 insertions(+), 14 deletions(-) rename src/main/java/com/alibaba/druid/wall/spi/{SQLServerProvider.java => SQLServerWallProvider.java} (89%) diff --git a/src/main/java/com/alibaba/druid/wall/WallDenyStat.java b/src/main/java/com/alibaba/druid/wall/WallDenyStat.java index 1238b93164..9e794bde49 100644 --- a/src/main/java/com/alibaba/druid/wall/WallDenyStat.java +++ b/src/main/java/com/alibaba/druid/wall/WallDenyStat.java @@ -1,19 +1,46 @@ package com.alibaba.druid.wall; +import java.util.Date; import java.util.concurrent.atomic.AtomicLongFieldUpdater; public class WallDenyStat { private volatile long denyCount; - final static AtomicLongFieldUpdater denyCountUpdater = AtomicLongFieldUpdater.newUpdater(WallDenyStat.class, - "denyCount"); + private volatile long lastDenyTimeMillis; + + private volatile long resetCount; + + final static AtomicLongFieldUpdater denyCountUpdater = AtomicLongFieldUpdater.newUpdater(WallDenyStat.class, + "denyCount"); + + final static AtomicLongFieldUpdater resetCountUpdater = AtomicLongFieldUpdater.newUpdater(WallDenyStat.class, + "resetCount"); public long incrementAndGetDenyCount() { + lastDenyTimeMillis = System.currentTimeMillis(); return denyCountUpdater.incrementAndGet(this); } public long getDenyCount() { return denyCount; } + + public Date getLastDenyTime() { + if (lastDenyTimeMillis <= 0) { + return null; + } + return new Date(lastDenyTimeMillis); + } + + public void reset() { + lastDenyTimeMillis = 0; + denyCount = 0; + resetCountUpdater.incrementAndGet(this); + } + + public long getResetCount() { + return this.resetCount; + } + } diff --git a/src/main/java/com/alibaba/druid/wall/WallFilter.java b/src/main/java/com/alibaba/druid/wall/WallFilter.java index 81ccc0d57f..826b565b06 100644 --- a/src/main/java/com/alibaba/druid/wall/WallFilter.java +++ b/src/main/java/com/alibaba/druid/wall/WallFilter.java @@ -37,7 +37,7 @@ import com.alibaba.druid.support.logging.LogFactory; import com.alibaba.druid.util.JdbcUtils; import com.alibaba.druid.wall.spi.MySqlWallProvider; import com.alibaba.druid.wall.spi.OracleWallProvider; -import com.alibaba.druid.wall.spi.SQLServerProvider; +import com.alibaba.druid.wall.spi.SQLServerWallProvider; import com.alibaba.druid.wall.violation.SyntaxErrorViolation; public class WallFilter extends FilterAdapter implements WallFilterMBean { @@ -86,10 +86,10 @@ public class WallFilter extends FilterAdapter implements WallFilterMBean { provider = new OracleWallProvider(config); } else if (JdbcUtils.SQL_SERVER.equals(dbType) || JdbcUtils.JTDS.equals(dbType)) { if (config == null) { - config = new WallConfig(SQLServerProvider.DEFAULT_CONFIG_DIR); + config = new WallConfig(SQLServerWallProvider.DEFAULT_CONFIG_DIR); } - provider = new SQLServerProvider(config); + provider = new SQLServerWallProvider(config); } else { throw new IllegalStateException("dbType not support : " + dbType + ", url " + dataSource.getUrl()); } diff --git a/src/main/java/com/alibaba/druid/wall/WallProvider.java b/src/main/java/com/alibaba/druid/wall/WallProvider.java index 9b5524aca9..fdd83c6544 100644 --- a/src/main/java/com/alibaba/druid/wall/WallProvider.java +++ b/src/main/java/com/alibaba/druid/wall/WallProvider.java @@ -24,6 +24,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.locks.ReentrantReadWriteLock; import com.alibaba.druid.sql.ast.SQLStatement; +import com.alibaba.druid.sql.parser.NotAllowCommentException; import com.alibaba.druid.sql.parser.ParserException; import com.alibaba.druid.sql.parser.SQLStatementParser; import com.alibaba.druid.sql.parser.Token; @@ -49,7 +50,9 @@ public abstract class WallProvider { private final ConcurrentMap deniedTables = new ConcurrentHashMap(); private final ConcurrentMap deniedFunctions = new ConcurrentHashMap(); - private final ConcurrentMap deniedSchemas = new ConcurrentHashMap(); + private final ConcurrentMap deniedSchemas = new ConcurrentHashMap(); + + public final WallDenyStat commentDeniedStat = new WallDenyStat(); public WallProvider(WallConfig config){ this.config = config; @@ -129,6 +132,10 @@ public abstract class WallProvider { return result.getViolations().isEmpty(); } + public void incrementCommentDeniedCount() { + this.commentDeniedStat.incrementAndGetDenyCount(); + } + public boolean checkDenyFunction(String functionName) { if (functionName == null) { return true; @@ -147,7 +154,7 @@ public abstract class WallProvider { return true; } - + public boolean checkDenySchema(String schemaName) { if (schemaName == null) { return true; @@ -213,7 +220,7 @@ public abstract class WallProvider { String formName = WallVisitorUtils.form(tableName); return deniedTables.get(formName); } - + public WallDenyStat getDenniedSchemaStat(String schemaName) { if (schemaName == null) { return null; @@ -223,6 +230,10 @@ public abstract class WallProvider { return deniedSchemas.get(formName); } + public WallDenyStat getCommentDenyStat() { + return this.commentDeniedStat; + } + public WallCheckResult check(String sql) { WallCheckResult result = new WallCheckResult(); @@ -245,6 +256,10 @@ public abstract class WallProvider { try { parser.parseStatementList(result.getStatementList()); + } catch (NotAllowCommentException e) { + result.getViolations().add(new SyntaxErrorViolation(e, sql)); + incrementCommentDeniedCount(); + return result; } catch (Exception e) { result.getViolations().add(new SyntaxErrorViolation(e, sql)); return result; diff --git a/src/main/java/com/alibaba/druid/wall/WallUtils.java b/src/main/java/com/alibaba/druid/wall/WallUtils.java index 8cb6fe560c..bce900a45e 100644 --- a/src/main/java/com/alibaba/druid/wall/WallUtils.java +++ b/src/main/java/com/alibaba/druid/wall/WallUtils.java @@ -17,7 +17,7 @@ package com.alibaba.druid.wall; import com.alibaba.druid.wall.spi.MySqlWallProvider; import com.alibaba.druid.wall.spi.OracleWallProvider; -import com.alibaba.druid.wall.spi.SQLServerProvider; +import com.alibaba.druid.wall.spi.SQLServerWallProvider; public class WallUtils { @@ -42,12 +42,12 @@ public class WallUtils { } public static boolean isValidateSqlServer(String sql) { - SQLServerProvider provider = new SQLServerProvider(); + SQLServerWallProvider provider = new SQLServerWallProvider(); return provider.checkValid(sql); } public static boolean isValidateSqlServer(String sql, WallConfig config) { - SQLServerProvider provider = new SQLServerProvider(config); + SQLServerWallProvider provider = new SQLServerWallProvider(config); return provider.checkValid(sql); } } diff --git a/src/main/java/com/alibaba/druid/wall/spi/SQLServerProvider.java b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallProvider.java similarity index 89% rename from src/main/java/com/alibaba/druid/wall/spi/SQLServerProvider.java rename to src/main/java/com/alibaba/druid/wall/spi/SQLServerWallProvider.java index f9e2f57013..8a10a6ade8 100644 --- a/src/main/java/com/alibaba/druid/wall/spi/SQLServerProvider.java +++ b/src/main/java/com/alibaba/druid/wall/spi/SQLServerWallProvider.java @@ -30,18 +30,18 @@ import com.alibaba.druid.wall.WallVisitor; * @version 1.0, 2012-3-17 * @see */ -public class SQLServerProvider extends WallProvider { +public class SQLServerWallProvider extends WallProvider { public final static String DEFAULT_CONFIG_DIR = "META-INF/druid/wall/sqlserver"; /** * @param config */ - public SQLServerProvider(){ + public SQLServerWallProvider(){ this(new WallConfig(DEFAULT_CONFIG_DIR)); } - public SQLServerProvider(WallConfig config){ + public SQLServerWallProvider(WallConfig config){ super(config); } From 8dc6ed4485282aaf35e6680a3d0720cc62807a75 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 17 Mar 2013 11:41:40 +0800 Subject: [PATCH 8/8] improve WallFilter --- .../bvt/filter/wall/WallCommentTest.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 src/test/java/com/alibaba/druid/bvt/filter/wall/WallCommentTest.java diff --git a/src/test/java/com/alibaba/druid/bvt/filter/wall/WallCommentTest.java b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallCommentTest.java new file mode 100644 index 0000000000..6c5aeafa12 --- /dev/null +++ b/src/test/java/com/alibaba/druid/bvt/filter/wall/WallCommentTest.java @@ -0,0 +1,57 @@ +/* + * 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.filter.wall; + +import junit.framework.TestCase; + +import org.junit.Assert; + +import com.alibaba.druid.wall.spi.MySqlWallProvider; +import com.alibaba.druid.wall.spi.OracleWallProvider; +import com.alibaba.druid.wall.spi.SQLServerWallProvider; + +/** + * @author wenshao + */ +public class WallCommentTest extends TestCase { + + public void testORACLE() throws Exception { + String sql = "SELECT F1, F2 FROM ABC --test"; + + OracleWallProvider provider = new OracleWallProvider(); + Assert.assertFalse(provider.checkValid(sql)); + + Assert.assertEquals(1, provider.getCommentDenyStat().getDenyCount()); + } + + public void testmysql() throws Exception { + String sql = "SELECT F1, F2 FROM ABC --test"; + + MySqlWallProvider provider = new MySqlWallProvider(); + Assert.assertFalse(provider.checkValid(sql)); + + Assert.assertEquals(1, provider.getCommentDenyStat().getDenyCount()); + } + + public void testsqlserver() throws Exception { + String sql = "SELECT F1, F2 FROM ABC --test"; + + SQLServerWallProvider provider = new SQLServerWallProvider(); + Assert.assertFalse(provider.checkValid(sql)); + + Assert.assertEquals(1, provider.getCommentDenyStat().getDenyCount()); + } +}