From 88bfa3489805cff60bb27cd3f564623838c96ac4 Mon Sep 17 00:00:00 2001 From: Andy Janata Date: Fri, 23 Mar 2018 14:36:26 -0700 Subject: [PATCH] Global and game chats have distinct flood controls. Split the configuration value into two. Add a tracking object to ChatFilter per user which currently contains the two different last message times. Removed the last message times from the User object. Show chat error messages only in the tab that caused them, not both tabs. --- WebContent/js/cah.ajax.handlers.js | 6 ++ build.properties.example | 11 ++- .../filtered-resources/WEB-INF/pyx.properties | 6 +- .../java/net/socialgamer/cah/data/User.java | 9 --- .../net/socialgamer/cah/util/ChatFilter.java | 77 ++++++++++++++----- 5 files changed, 76 insertions(+), 33 deletions(-) diff --git a/WebContent/js/cah.ajax.handlers.js b/WebContent/js/cah.ajax.handlers.js index 654ec70..0cb04e7 100644 --- a/WebContent/js/cah.ajax.handlers.js +++ b/WebContent/js/cah.ajax.handlers.js @@ -128,6 +128,12 @@ cah.ajax.SuccessHandlers[cah.$.AjaxOperation.CHAT] = function(data) { // pass }; +cah.ajax.ErrorHandlers[cah.$.AjaxOperation.CHAT] = function(data, req) { + cah.log.status_with_game(req[cah.$.AjaxRequest.GAME_ID], "Error: " + + cah.$.ErrorCode_msg[data[cah.$.AjaxResponse.ERROR_CODE]], "error") +}; +cah.ajax.ErrorHandlers[cah.$.AjaxOperation.GAME_CHAT] = cah.ajax.ErrorHandlers[cah.$.AjaxOperation.CHAT]; + cah.ajax.SuccessHandlers[cah.$.AjaxOperation.GAME_CHAT] = function(data) { // pass }; diff --git a/build.properties.example b/build.properties.example index 7169b4d..dc2427f 100644 --- a/build.properties.example +++ b/build.properties.example @@ -10,10 +10,15 @@ pyx.insecure_id_allowed=true pyx.id_code_salt= # comma-separated listed of IP addresses (v4 or v6) from which users are considered admins. pyx.admin_addrs=127.0.0.1,::1 -# this many messages in that many seconds is considered chatting too fast. -pyx.flood_count=4 +# this many messages to global chat in that many seconds is considered chatting too fast. +pyx.global_flood_count=3 # seconds -pyx.flood_time=30 +pyx.global_flood_time=25 +# same but for game chats +pyx.game_flood_count=5 +# seconds +pyx.game_flood_time=30 + # for production use, use postgres #hibernate.dialect=org.hibernate.dialect.PostgreSQLDialect diff --git a/src/main/filtered-resources/WEB-INF/pyx.properties b/src/main/filtered-resources/WEB-INF/pyx.properties index ad71923..d7af4b1 100644 --- a/src/main/filtered-resources/WEB-INF/pyx.properties +++ b/src/main/filtered-resources/WEB-INF/pyx.properties @@ -6,8 +6,10 @@ pyx.server.broadcast_connects_and_disconnects=${pyx.broadcast_connects_and_disco pyx.server.global_chat_enabled=${pyx.global_chat_enabled} pyx.server.id_code_salt=${pyx.id_code_salt} pyx.server.admin_addrs=${pyx.admin_addrs} -pyx.chat.flood_count=${pyx.flood_count} -pyx.chat.flood_time=${pyx.flood_time} +pyx.chat.global.flood_count=${pyx.global_flood_count} +pyx.chat.global.flood_time=${pyx.global_flood_time} +pyx.chat.game.flood_count=${pyx.game_flood_count} +pyx.chat.game.flood_time=${pyx.game_flood_time} pyx.build=${buildNumber} # this is NOT allowed to be changed during a reload, as metrics depend on previous events diff --git a/src/main/java/net/socialgamer/cah/data/User.java b/src/main/java/net/socialgamer/cah/data/User.java index 8450610..3ff3960 100644 --- a/src/main/java/net/socialgamer/cah/data/User.java +++ b/src/main/java/net/socialgamer/cah/data/User.java @@ -25,10 +25,7 @@ package net.socialgamer.cah.data; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Date; -import java.util.LinkedList; -import java.util.List; import java.util.concurrent.PriorityBlockingQueue; import javax.annotation.Nullable; @@ -77,8 +74,6 @@ public class User { private final ReadableUserAgent agent; - private final List lastMessageTimes = Collections.synchronizedList(new LinkedList()); - /** * Reset when this user object is no longer valid, most likely because it pinged out. */ @@ -342,8 +337,4 @@ public class User { currentGame = null; } } - - public List getLastMessageTimes() { - return lastMessageTimes; - } } diff --git a/src/main/java/net/socialgamer/cah/util/ChatFilter.java b/src/main/java/net/socialgamer/cah/util/ChatFilter.java index 436fd45..2ab98b9 100644 --- a/src/main/java/net/socialgamer/cah/util/ChatFilter.java +++ b/src/main/java/net/socialgamer/cah/util/ChatFilter.java @@ -23,7 +23,13 @@ package net.socialgamer.cah.util; +import java.util.Collections; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; import java.util.Properties; +import java.util.TreeMap; +import java.util.WeakHashMap; import java.util.concurrent.TimeUnit; import org.apache.log4j.Logger; @@ -47,49 +53,55 @@ public class ChatFilter { private static final long DEFAULT_CHAT_FLOOD_TIME = TimeUnit.SECONDS.toMillis(30); private final Provider propsProvider; + private final Map filterData = Collections.synchronizedMap(new WeakHashMap<>()); public enum Result { OK, TOO_FAST, TOO_LONG, NO_MESSAGE } + private enum Scope { + global, game + } + @Inject public ChatFilter(final Provider propsProvider) { this.propsProvider = propsProvider; } public Result filterGlobal(final User user, final String message) { - final Result result = filterCommon(user, message); + final Result result = filterCommon(Scope.global, user, message); if (Result.OK != result) { return result; } // TODO - user.getLastMessageTimes().add(System.currentTimeMillis()); + + getMessageTimes(user, Scope.global).add(System.currentTimeMillis()); return result; } public Result filterGame(final User user, final String message) { - final Result result = filterCommon(user, message); + final Result result = filterCommon(Scope.game, user, message); if (Result.OK != result) { return result; } // TODO - user.getLastMessageTimes().add(System.currentTimeMillis()); + + getMessageTimes(user, Scope.game).add(System.currentTimeMillis()); return result; } - private Result filterCommon(final User user, final String message) { + private Result filterCommon(final Scope scope, final User user, final String message) { // TODO - // Intentionally leaving flood protection as per-user, rather than - // changing it to per-user-per-game. - if (user.getLastMessageTimes().size() >= getFloodCount()) { - final Long head = user.getLastMessageTimes().get(0); - if (System.currentTimeMillis() - head < getFloodTime()) { + final List messageTimes = getMessageTimes(user, scope); + if (messageTimes.size() >= getFloodCount(scope)) { + final Long head = messageTimes.get(0); + if (System.currentTimeMillis() - head < getFloodTime(scope)) { return Result.TOO_FAST; } - user.getLastMessageTimes().remove(0); + messageTimes.remove(0); } if (message.length() > Constants.CHAT_MAX_LENGTH) { @@ -101,25 +113,52 @@ public class ChatFilter { return Result.OK; } - private int getFloodCount() { + private int getFloodCount(final Scope scope) { try { - return Integer.parseInt(propsProvider.get().getProperty("pyx.chat.flood_count", + return Integer.parseInt(propsProvider.get().getProperty( + String.format("pyx.chat.%s.flood_count", scope), String.valueOf(DEFAULT_CHAT_FLOOD_MESSAGE_COUNT))); } catch (final NumberFormatException e) { - LOG.warn(String.format("Unable to parse pyx.chat.flood_count as a number," - + " using default of %d", DEFAULT_CHAT_FLOOD_MESSAGE_COUNT), e); + LOG.warn(String.format("Unable to parse pyx.chat.%s.flood_count as a number," + + " using default of %d", scope, DEFAULT_CHAT_FLOOD_MESSAGE_COUNT), e); return DEFAULT_CHAT_FLOOD_MESSAGE_COUNT; } } - private long getFloodTime() { + private long getFloodTime(final Scope scope) { try { return TimeUnit.SECONDS.toMillis(Integer.parseInt(propsProvider.get().getProperty( - "pyx.chat.flood_time", String.valueOf(DEFAULT_CHAT_FLOOD_TIME)))); + String.format("pyx.chat.%s.flood_time", scope), + String.valueOf(DEFAULT_CHAT_FLOOD_TIME)))); } catch (final NumberFormatException e) { - LOG.warn(String.format("Unable to parse pyx.chat.flood_time as a number," - + " using default of %d", DEFAULT_CHAT_FLOOD_TIME), e); + LOG.warn(String.format("Unable to parse pyx.chat.%s.flood_time as a number," + + " using default of %d", scope, DEFAULT_CHAT_FLOOD_TIME), e); return DEFAULT_CHAT_FLOOD_TIME; } } + + private List getMessageTimes(final User user, final Scope scope) { + FilterData data; + synchronized (filterData) { + data = filterData.get(user); + // we should only have to do this once per user... + if (null == data) { + LOG.trace(String.format("Created new FilterData for user %s", user.getNickname())); + data = new FilterData(); + filterData.put(user, data); + } + } + return data.lastMessageTimes.get(scope); + } + + private static class FilterData { + final Map> lastMessageTimes; + + private FilterData() { + final Map> map = new TreeMap<>(); + map.put(Scope.global, Collections.synchronizedList(new LinkedList<>())); + map.put(Scope.game, Collections.synchronizedList(new LinkedList<>())); + lastMessageTimes = Collections.unmodifiableMap(map); + } + } }