From 7ee9b9336573f16d7c273a4da45c9393ed82acb3 Mon Sep 17 00:00:00 2001 From: Luck Date: Sat, 5 May 2018 18:17:48 +0100 Subject: [PATCH] Convert bulkupdate & search functionality to use PreparedStatements (#972) --- .../common/bulkupdate/BulkUpdate.java | 39 +++-------- .../bulkupdate/PreparedStatementBuilder.java | 69 +++++++++++++++++++ .../common/bulkupdate/action/Action.java | 3 +- .../bulkupdate/action/DeleteAction.java | 5 +- .../bulkupdate/action/UpdateAction.java | 7 +- .../bulkupdate/comparisons/Comparison.java | 6 +- .../bulkupdate/comparisons/Constraint.java | 10 ++- .../comparisons/StandardComparison.java | 6 +- .../common/bulkupdate/query/Query.java | 5 +- .../commands/misc/BulkUpdateCommand.java | 2 +- .../common/storage/dao/sql/SqlDao.java | 21 +++--- 11 files changed, 118 insertions(+), 55 deletions(-) create mode 100644 common/src/main/java/me/lucko/luckperms/common/bulkupdate/PreparedStatementBuilder.java diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/BulkUpdate.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/BulkUpdate.java index cf73d391..1e49b9fe 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/BulkUpdate.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/BulkUpdate.java @@ -87,56 +87,35 @@ public final class BulkUpdate { * * @return this query in SQL form */ - public String buildAsSql() { + public PreparedStatementBuilder buildAsSql() { // DELETE FROM {table} WHERE ... // UPDATE {table} SET ... WHERE ... - StringBuilder sb = new StringBuilder(); + PreparedStatementBuilder builder = new PreparedStatementBuilder(); // add the action // (DELETE FROM or UPDATE) - sb.append(this.action.getAsSql()); + this.action.appendSql(builder); // if there are no constraints, just return without a WHERE clause if (this.queries.isEmpty()) { - return sb.append(";").toString(); + return builder; } // append constraints - sb.append(" WHERE"); + builder.append(" WHERE"); for (int i = 0; i < this.queries.size(); i++) { Query query = this.queries.get(i); - sb.append(" "); + builder.append(" "); if (i != 0) { - sb.append("AND "); + builder.append("AND "); } - sb.append(query.getAsSql()); + query.appendSql(builder); } - return sb.append(";").toString(); - } - - /** - * Utility to appropriately escape a string for use in a query. - * - * @param s the string to escape - * @return an escaped string - */ - public static String escapeStringForSql(String s) { - if (s.equalsIgnoreCase("true") || s.equalsIgnoreCase("false")) { - return s.toLowerCase(); - } - - try { - Integer.parseInt(s); - return s; - } catch (NumberFormatException e) { - // ignored - } - - return "'" + s + "'"; + return builder; } public DataType getDataType() { diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/PreparedStatementBuilder.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/PreparedStatementBuilder.java new file mode 100644 index 00000000..f8dfc56c --- /dev/null +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/PreparedStatementBuilder.java @@ -0,0 +1,69 @@ +/* + * This file is part of LuckPerms, licensed under the MIT License. + * + * Copyright (c) lucko (Luck) + * Copyright (c) contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package me.lucko.luckperms.common.bulkupdate; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Function; + +public class PreparedStatementBuilder { + private final StringBuilder sb = new StringBuilder(); + private final List variables = new ArrayList<>(); + + public PreparedStatementBuilder() { + + } + + public PreparedStatementBuilder append(String s) { + this.sb.append(s); + return this; + } + + public PreparedStatementBuilder variable(String variable) { + this.variables.add(variable); + return this; + } + + public PreparedStatement build(Connection connection, Function mapping) throws SQLException { + PreparedStatement statement = connection.prepareStatement(mapping.apply(this.sb.toString())); + for (int i = 0; i < this.variables.size(); i++) { + String var = this.variables.get(i); + statement.setString(i + 1, var); + } + return statement; + } + + public String toReadableString() { + String s = this.sb.toString(); + for (String var : this.variables) { + s = s.replaceFirst("\\?", var); + } + return s; + } +} diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/Action.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/Action.java index 038f5bb2..9f80fa7d 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/Action.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/Action.java @@ -25,6 +25,7 @@ package me.lucko.luckperms.common.bulkupdate.action; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; import me.lucko.luckperms.common.node.model.NodeDataContainer; /** @@ -54,6 +55,6 @@ public interface Action { * * @return the action in sql form */ - String getAsSql(); + void appendSql(PreparedStatementBuilder builder); } diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/DeleteAction.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/DeleteAction.java index 97c1a415..1401f9a7 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/DeleteAction.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/DeleteAction.java @@ -25,6 +25,7 @@ package me.lucko.luckperms.common.bulkupdate.action; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; import me.lucko.luckperms.common.node.model.NodeDataContainer; public class DeleteAction implements Action { @@ -47,7 +48,7 @@ public class DeleteAction implements Action { } @Override - public String getAsSql() { - return "DELETE FROM {table}"; + public void appendSql(PreparedStatementBuilder builder) { + builder.append("DELETE FROM {table}"); } } diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/UpdateAction.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/UpdateAction.java index bee5a2bc..010f0f1c 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/UpdateAction.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/action/UpdateAction.java @@ -25,7 +25,7 @@ package me.lucko.luckperms.common.bulkupdate.action; -import me.lucko.luckperms.common.bulkupdate.BulkUpdate; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; import me.lucko.luckperms.common.bulkupdate.query.QueryField; import me.lucko.luckperms.common.node.model.NodeDataContainer; @@ -66,7 +66,8 @@ public class UpdateAction implements Action { } @Override - public String getAsSql() { - return "UPDATE {table} SET " + this.field.getSqlName() + "=" + BulkUpdate.escapeStringForSql(this.value); + public void appendSql(PreparedStatementBuilder builder) { + builder.append("UPDATE {table} SET " + this.field.getSqlName() + "=?"); + builder.variable(this.value); } } diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Comparison.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Comparison.java index 25097deb..de005b2b 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Comparison.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Comparison.java @@ -25,6 +25,8 @@ package me.lucko.luckperms.common.bulkupdate.comparisons; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; + /** * A method of comparing two strings */ @@ -49,9 +51,7 @@ public interface Comparison { /** * Returns the comparison operator in SQL form - * - * @return a sql form of this comparison */ - String getAsSql(); + void appendSql(PreparedStatementBuilder builder); } diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Constraint.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Constraint.java index 0e12e1f2..ba5f5a9d 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Constraint.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/Constraint.java @@ -25,7 +25,7 @@ package me.lucko.luckperms.common.bulkupdate.comparisons; -import me.lucko.luckperms.common.bulkupdate.BulkUpdate; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; public class Constraint { @@ -54,8 +54,12 @@ public class Constraint { return this.comparison.matches(value, this.expression); } - public String getAsSql(String field) { - return field + " " + this.comparison.getAsSql() + " " + BulkUpdate.escapeStringForSql(this.expression); + public void appendSql(PreparedStatementBuilder builder, String field) { + // e.g. field LIKE ? + builder.append(field + " "); + this.comparison.appendSql(builder); + builder.append(" ?"); + builder.variable(this.expression); } public Comparison getComparison() { diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/StandardComparison.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/StandardComparison.java index 072eeecb..ed73bee6 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/StandardComparison.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/comparisons/StandardComparison.java @@ -25,6 +25,8 @@ package me.lucko.luckperms.common.bulkupdate.comparisons; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; + /** * An enumeration of standard {@link Comparison}s. */ @@ -88,8 +90,8 @@ public enum StandardComparison implements Comparison { } @Override - public String getAsSql() { - return this.asSql; + public void appendSql(PreparedStatementBuilder builder) { + builder.append(this.asSql); } public static StandardComparison parseComparison(String s) { diff --git a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/query/Query.java b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/query/Query.java index 103bac2e..bdbf32d6 100644 --- a/common/src/main/java/me/lucko/luckperms/common/bulkupdate/query/Query.java +++ b/common/src/main/java/me/lucko/luckperms/common/bulkupdate/query/Query.java @@ -25,6 +25,7 @@ package me.lucko.luckperms.common.bulkupdate.query; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; import me.lucko.luckperms.common.bulkupdate.comparisons.Constraint; import me.lucko.luckperms.common.node.model.NodeDataContainer; @@ -67,8 +68,8 @@ public class Query { } } - public String getAsSql() { - return this.constraint.getAsSql(this.field.getSqlName()); + public void appendSql(PreparedStatementBuilder builder) { + this.constraint.appendSql(builder, this.field.getSqlName()); } public QueryField getField() { diff --git a/common/src/main/java/me/lucko/luckperms/common/commands/misc/BulkUpdateCommand.java b/common/src/main/java/me/lucko/luckperms/common/commands/misc/BulkUpdateCommand.java index bd038cf0..453854b9 100644 --- a/common/src/main/java/me/lucko/luckperms/common/commands/misc/BulkUpdateCommand.java +++ b/common/src/main/java/me/lucko/luckperms/common/commands/misc/BulkUpdateCommand.java @@ -151,7 +151,7 @@ public class BulkUpdateCommand extends SingleCommand { this.pendingOperations.put(id, bulkUpdate); - Message.BULK_UPDATE_QUEUED.send(sender, bulkUpdate.buildAsSql().replace("{table}", bulkUpdate.getDataType().getName())); + Message.BULK_UPDATE_QUEUED.send(sender, bulkUpdate.buildAsSql().toReadableString().replace("{table}", bulkUpdate.getDataType().getName())); Message.BULK_UPDATE_CONFIRM.send(sender, label, id); return CommandResult.SUCCESS; diff --git a/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java b/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java index a18ac0b9..9baaa59c 100644 --- a/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java +++ b/common/src/main/java/me/lucko/luckperms/common/storage/dao/sql/SqlDao.java @@ -35,6 +35,7 @@ import me.lucko.luckperms.api.Node; import me.lucko.luckperms.common.actionlog.ExtendedLogEntry; import me.lucko.luckperms.common.actionlog.Log; import me.lucko.luckperms.common.bulkupdate.BulkUpdate; +import me.lucko.luckperms.common.bulkupdate.PreparedStatementBuilder; import me.lucko.luckperms.common.bulkupdate.comparisons.Constraint; import me.lucko.luckperms.common.contexts.ContextSetJsonSerializer; import me.lucko.luckperms.common.managers.group.GroupManager; @@ -265,20 +266,18 @@ public class SqlDao extends AbstractDao { @Override public void applyBulkUpdate(BulkUpdate bulkUpdate) throws SQLException { - String queryString = bulkUpdate.buildAsSql(); - try (Connection c = this.provider.getConnection()) { if (bulkUpdate.getDataType().isIncludingUsers()) { String table = this.prefix.apply("{prefix}user_permissions"); - try (Statement s = c.createStatement()) { - s.execute(queryString.replace("{table}", table)); + try (PreparedStatement ps = bulkUpdate.buildAsSql().build(c, q -> q.replace("{table}", table))) { + ps.execute(); } } if (bulkUpdate.getDataType().isIncludingGroups()) { String table = this.prefix.apply("{prefix}group_permissions"); - try (Statement s = c.createStatement()) { - s.execute(queryString.replace("{table}", table)); + try (PreparedStatement ps = bulkUpdate.buildAsSql().build(c, q -> q.replace("{table}", table))) { + ps.execute(); } } } @@ -494,9 +493,12 @@ public class SqlDao extends AbstractDao { @Override public List> getUsersWithPermission(Constraint constraint) throws SQLException { + PreparedStatementBuilder builder = new PreparedStatementBuilder().append(USER_PERMISSIONS_SELECT_PERMISSION); + constraint.appendSql(builder, "permission"); + List> held = new ArrayList<>(); try (Connection c = this.provider.getConnection()) { - try (PreparedStatement ps = c.prepareStatement(this.prefix.apply(USER_PERMISSIONS_SELECT_PERMISSION + constraint.getAsSql("permission")))) { + try (PreparedStatement ps = builder.build(c, this.prefix)) { try (ResultSet rs = ps.executeQuery()) { while (rs.next()) { UUID holder = UUID.fromString(rs.getString("uuid")); @@ -737,9 +739,12 @@ public class SqlDao extends AbstractDao { @Override public List> getGroupsWithPermission(Constraint constraint) throws SQLException { + PreparedStatementBuilder builder = new PreparedStatementBuilder().append(GROUP_PERMISSIONS_SELECT_PERMISSION); + constraint.appendSql(builder, "permission"); + List> held = new ArrayList<>(); try (Connection c = this.provider.getConnection()) { - try (PreparedStatement ps = c.prepareStatement(this.prefix.apply(GROUP_PERMISSIONS_SELECT_PERMISSION + constraint.getAsSql("permission")))) { + try (PreparedStatement ps = builder.build(c, this.prefix)) { try (ResultSet rs = ps.executeQuery()) { while (rs.next()) { String holder = rs.getString("name");