From a378ccdee05b9cee668e42e609f388b9ba2e2675 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 3 Nov 2018 17:36:00 -0400 Subject: [PATCH] Refactored rate-limiting. If rate-limiting is disabled, we now use a simple stub implementation that is simpler to reason with. --- .../velocitypowered/proxy/VelocityServer.java | 5 +-- .../GuavaCacheRatelimiter.java} | 35 +++++++++---------- .../util/ratelimit/NoopCacheRatelimiter.java | 15 ++++++++ .../proxy/util/ratelimit/Ratelimiter.java | 16 +++++++++ .../proxy/util/ratelimit/Ratelimiters.java | 14 ++++++++ .../GuavaCacheRatelimiterTest.java} | 8 ++--- 6 files changed, 68 insertions(+), 25 deletions(-) rename proxy/src/main/java/com/velocitypowered/proxy/util/{Ratelimiter.java => ratelimit/GuavaCacheRatelimiter.java} (58%) create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/NoopCacheRatelimiter.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiter.java create mode 100644 proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiters.java rename proxy/src/test/java/com/velocitypowered/proxy/util/{RatelimiterTest.java => ratelimit/GuavaCacheRatelimiterTest.java} (78%) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java index 29d5dec2e..0bc9e16ec 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java @@ -34,8 +34,9 @@ import com.velocitypowered.proxy.scheduler.VelocityScheduler; import com.velocitypowered.proxy.server.ServerMap; import com.velocitypowered.proxy.util.AddressUtil; import com.velocitypowered.proxy.util.EncryptionUtils; -import com.velocitypowered.proxy.util.Ratelimiter; import com.velocitypowered.proxy.util.VelocityChannelRegistrar; +import com.velocitypowered.proxy.util.ratelimit.Ratelimiter; +import com.velocitypowered.proxy.util.ratelimit.Ratelimiters; import io.netty.bootstrap.Bootstrap; import java.net.InetSocketAddress; import java.nio.file.Files; @@ -166,7 +167,7 @@ public class VelocityServer implements ProxyServer { servers.register(new ServerInfo(entry.getKey(), AddressUtil.parseAddress(entry.getValue()))); } - ipAttemptLimiter = new Ratelimiter(configuration.getLoginRatelimit()); + ipAttemptLimiter = Ratelimiters.createWithMilliseconds(configuration.getLoginRatelimit()); httpClient = new NettyHttpClient(this); loadPlugins(); diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/Ratelimiter.java b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiter.java similarity index 58% rename from proxy/src/main/java/com/velocitypowered/proxy/util/Ratelimiter.java rename to proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiter.java index 8e39133a1..71c25013d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/util/Ratelimiter.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiter.java @@ -1,6 +1,7 @@ -package com.velocitypowered.proxy.util; +package com.velocitypowered.proxy.util.ratelimit; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import com.google.common.base.Ticker; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; @@ -11,28 +12,25 @@ import java.util.concurrent.TimeUnit; /** * A simple rate-limiter based on a Guava {@link Cache}. */ -public class Ratelimiter { +public class GuavaCacheRatelimiter implements Ratelimiter { private final Cache expiringCache; private final long timeoutNanos; - public Ratelimiter(long timeoutMs) { - this(timeoutMs, Ticker.systemTicker()); + GuavaCacheRatelimiter(long time, TimeUnit unit) { + this(time, unit, Ticker.systemTicker()); } @VisibleForTesting - Ratelimiter(long timeoutMs, Ticker ticker) { - if (timeoutMs == 0) { - this.timeoutNanos = timeoutMs; - this.expiringCache = CacheBuilder.newBuilder().maximumSize(0).build(); - } else { - this.timeoutNanos = TimeUnit.MILLISECONDS.toNanos(timeoutMs); - this.expiringCache = CacheBuilder.newBuilder() - .ticker(ticker) - .concurrencyLevel(Runtime.getRuntime().availableProcessors()) - .expireAfterWrite(timeoutMs, TimeUnit.MILLISECONDS) - .build(); - } + GuavaCacheRatelimiter(long time, TimeUnit unit, Ticker ticker) { + Preconditions.checkNotNull(unit, "unit"); + Preconditions.checkNotNull(ticker, "ticker"); + this.timeoutNanos = unit.toNanos(time); + this.expiringCache = CacheBuilder.newBuilder() + .ticker(ticker) + .concurrencyLevel(Runtime.getRuntime().availableProcessors()) + .expireAfterWrite(time, unit) + .build(); } /** @@ -41,10 +39,9 @@ public class Ratelimiter { * @param address the address to rate limit * @return true if we should allow the client, false if we should rate-limit */ + @Override public boolean attempt(InetAddress address) { - if (timeoutNanos == 0) { - return true; - } + Preconditions.checkNotNull(address, "address"); long expectedNewValue = System.nanoTime() + timeoutNanos; long last; try { diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/NoopCacheRatelimiter.java b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/NoopCacheRatelimiter.java new file mode 100644 index 000000000..b1faa5345 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/NoopCacheRatelimiter.java @@ -0,0 +1,15 @@ +package com.velocitypowered.proxy.util.ratelimit; + +import java.net.InetAddress; + +/** + * A {@link Ratelimiter} that does no rate-limiting. + */ +enum NoopCacheRatelimiter implements Ratelimiter { + INSTANCE; + + @Override + public boolean attempt(InetAddress address) { + return true; + } +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiter.java b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiter.java new file mode 100644 index 000000000..c6cfa07d1 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiter.java @@ -0,0 +1,16 @@ +package com.velocitypowered.proxy.util.ratelimit; + +import java.net.InetAddress; + +/** + * Allows rate limiting of clients. + */ +public interface Ratelimiter { + + /** + * Determines whether or not to allow the connection. + * @param address the address to rate limit + * @return true if allowed, false if not + */ + boolean attempt(InetAddress address); +} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiters.java b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiters.java new file mode 100644 index 000000000..bd15abb07 --- /dev/null +++ b/proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiters.java @@ -0,0 +1,14 @@ +package com.velocitypowered.proxy.util.ratelimit; + +import java.util.concurrent.TimeUnit; + +public final class Ratelimiters { + private Ratelimiters() { + throw new AssertionError(); + } + + public static Ratelimiter createWithMilliseconds(long ms) { + return ms <= 0 ? NoopCacheRatelimiter.INSTANCE : new GuavaCacheRatelimiter(ms, + TimeUnit.MILLISECONDS); + } +} diff --git a/proxy/src/test/java/com/velocitypowered/proxy/util/RatelimiterTest.java b/proxy/src/test/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiterTest.java similarity index 78% rename from proxy/src/test/java/com/velocitypowered/proxy/util/RatelimiterTest.java rename to proxy/src/test/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiterTest.java index a4c0858e3..3ebb8db0f 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/util/RatelimiterTest.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiterTest.java @@ -1,4 +1,4 @@ -package com.velocitypowered.proxy.util; +package com.velocitypowered.proxy.util.ratelimit; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -9,11 +9,11 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import org.junit.jupiter.api.Test; -class RatelimiterTest { +class GuavaCacheRatelimiterTest { @Test void attemptZero() { - Ratelimiter noRatelimiter = new Ratelimiter(0); + Ratelimiter noRatelimiter = new GuavaCacheRatelimiter(0, TimeUnit.MILLISECONDS); assertTrue(noRatelimiter.attempt(InetAddress.getLoopbackAddress())); assertTrue(noRatelimiter.attempt(InetAddress.getLoopbackAddress())); } @@ -28,7 +28,7 @@ class RatelimiterTest { return base + extra.get(); } }; - Ratelimiter ratelimiter = new Ratelimiter(1000, testTicker); + Ratelimiter ratelimiter = new GuavaCacheRatelimiter(1000, TimeUnit.MILLISECONDS, testTicker); assertTrue(ratelimiter.attempt(InetAddress.getLoopbackAddress())); assertFalse(ratelimiter.attempt(InetAddress.getLoopbackAddress())); extra.addAndGet(TimeUnit.SECONDS.toNanos(2));