WIP updates from review

This commit is contained in:
gregw 2024-09-25 09:19:43 +10:00
parent 033f0bbb12
commit 65252a3701
4 changed files with 36 additions and 30 deletions

View File

@ -11,19 +11,19 @@
<Class><Property name="jetty.dos.id.class" default="org.eclipse.jetty.server.handler.DoSHandler"/></Class> <Class><Property name="jetty.dos.id.class" default="org.eclipse.jetty.server.handler.DoSHandler"/></Class>
</Get> </Get>
</Arg> </Arg>
<Arg name="rateFactory"> <Arg name="rateControlFactory">
<New> <New>
<Class><Property name="jetty.dos.rateControlFactory" default="org.eclipse.jetty.server.handler.DoSHandler$ExponentialMovingAverageRateControlFactory"/></Class> <Class><Property name="jetty.dos.rateControlFactory" default="org.eclipse.jetty.server.handler.DoSHandler$ExponentialMovingAverageRateControlFactory"/></Class>
<Arg name="samplePeriodMs" type="long"><Property name="jetty.dos.samplePeriodMs" default="-1"/></Arg> <Arg name="samplePeriodMs" type="long"><Property name="jetty.dos.rateControlFactory.samplePeriodMs" default="-1"/></Arg>
<Arg name="alpha" type="double"><Property name="jetty.dos.alpha" default="-1.0"/></Arg> <Arg name="alpha" type="double"><Property name="jetty.dos.rateControlFactory.expMovingAvg.alpha" default="-1.0"/></Arg>
<Arg name="maxRequestsPerSecond" type="int"><Property name="jetty.dos.maxRequestsPerSecond" default="100"/></Arg> <Arg name="maxRequestsPerSecond" type="int"><Property name="jetty.dos.maxRequestsPerSecond" default="100"/></Arg>
</New> </New>
</Arg> </Arg>
<Arg name="rejectHandler"> <Arg name="rejectHandler">
<New> <New>
<Class><Property name="jetty.dos.rejectHandler" default="org.eclipse.jetty.server.handler.DoSHandler$DelayedRejectHandler"/></Class> <Class><Property name="jetty.dos.rejectHandler" default="org.eclipse.jetty.server.handler.DoSHandler$DelayedRejectHandler"/></Class>
<Arg name="delayMs" type="long"><Property name="jetty.dos.delayMs" default="1000"/></Arg> <Arg name="delayMs" type="long"><Property name="jetty.dos.rejectHandler.delayed.delayMs" default="1000"/></Arg>
<Arg name="maxDelayQueue" type="int"><Property name="jetty.dos.maxDelayQueue" default="1000"/></Arg> <Arg name="maxDelayQueue" type="int"><Property name="jetty.dos.rejectHandler.delayed.maxDelayQueue" default="1000"/></Arg>
<Arg name="reject"> <Arg name="reject">
<New class="org.eclipse.jetty.server.handler.DoSHandler$StatusRejectHandler"> <New class="org.eclipse.jetty.server.handler.DoSHandler$StatusRejectHandler">
<Arg name="status"><Property name="jetty.dos.rejectStatus" default="429"/></Arg> <Arg name="status"><Property name="jetty.dos.rejectStatus" default="429"/></Arg>

View File

@ -22,10 +22,10 @@ etc/jetty-dos.xml
#jetty.dos.rateControlFactory=org.eclipse.jetty.server.handler.DosHandler$ExponentialMovingAverageRateControlFactory #jetty.dos.rateControlFactory=org.eclipse.jetty.server.handler.DosHandler$ExponentialMovingAverageRateControlFactory
## The sample period(ms) to determine the request rate, or -1 for a default value ## The sample period(ms) to determine the request rate, or -1 for a default value
#jetty.dos.samplePeriodMs=100 #jetty.dos.rateControlFactory.samplePeriodMs=100
## The Exponential factor for the moving average rate ## The Exponential factor for the moving average rate
#jetty.dos.alpha=0.2 #jetty.dos.rateControlFactory.expMovingAvg.alpha=0.2
## The maximum requests per second per client ## The maximum requests per second per client
#jetty.dos.maxRequestsPerSecond=100 #jetty.dos.maxRequestsPerSecond=100
@ -34,10 +34,10 @@ etc/jetty-dos.xml
#jetty.dos.rejectHandler=org.eclipse.jetty.server.handler.DosHandler$TooManyRequestsRejectHandler #jetty.dos.rejectHandler=org.eclipse.jetty.server.handler.DosHandler$TooManyRequestsRejectHandler
## The period to delay dos requests before rejecting them. ## The period to delay dos requests before rejecting them.
#jetty.dos.delayMs=1000 #jetty.dos.rejectHandler.delayed.delayMs=1000
## The maximum number of requests to be held in the delay queue ## The maximum number of requests to be held in the delay queue
#jetty.dos.maxDelayQueueSize=1000 #jetty.dos.rejectHandler.delayed.maxDelayQueue=1000
## The maximum number of clients to track; or -1 for a default value ## The maximum number of clients to track; or -1 for a default value
#jetty.dos.maxTrackers=10000 #jetty.dos.maxTrackers=10000

View File

@ -94,11 +94,11 @@ public class DoSHandler extends ConditionalHandler.ElseNext
public interface RateControl public interface RateControl
{ {
/** /**
* Calculate if the rate is exceeded at the given time by adding a sample * Record a request and calculate if the rate is exceeded at the given time.
* @param now The {@link NanoTime#now()} at which to calculate the rate * @param now The {@link NanoTime#now()} at which to calculate the rate
* @return {@code true} if the rate is currently exceeded * @return {@code true} if the rate is currently exceeded
*/ */
boolean isRateExceededBySample(long now); boolean onRequest(long now);
/** /**
* Check if the tracker is now idle * Check if the tracker is now idle
@ -220,7 +220,7 @@ public class DoSHandler extends ConditionalHandler.ElseNext
Tracker tracker = _trackers.computeIfAbsent(id, this::newTracker); Tracker tracker = _trackers.computeIfAbsent(id, this::newTracker);
// If we are not over-limit then handle normally // If we are not over-limit then handle normally
if (!tracker.isRateExceededBySample(request.getBeginNanoTime())) if (!tracker.onRequest(request.getBeginNanoTime()))
return nextHandler(request, response, callback); return nextHandler(request, response, callback);
// Otherwise reject the request // Otherwise reject the request
@ -271,7 +271,7 @@ public class DoSHandler extends ConditionalHandler.ElseNext
private final String _id; private final String _id;
private final RateControl _rateControl; private final RateControl _rateControl;
private final Duration _idleCheck; private final Duration _idleCheck;
private long _expireAt; private long _nextIdleCheckAt;
Tracker(String id, RateControl rateControl) Tracker(String id, RateControl rateControl)
{ {
@ -283,7 +283,7 @@ public class DoSHandler extends ConditionalHandler.ElseNext
_id = id; _id = id;
_rateControl = rateControl; _rateControl = rateControl;
_idleCheck = idleCheck == null ? Duration.ofSeconds(2) : idleCheck; _idleCheck = idleCheck == null ? Duration.ofSeconds(2) : idleCheck;
_expireAt = NanoTime.now() + _idleCheck.toNanos(); _nextIdleCheckAt = NanoTime.now() + _idleCheck.toNanos();
} }
public String getId() public String getId()
@ -296,7 +296,7 @@ public class DoSHandler extends ConditionalHandler.ElseNext
return _rateControl; return _rateControl;
} }
public boolean isRateExceededBySample(long now) public boolean onRequest(long now)
{ {
try (AutoLock l = _lock.lock()) try (AutoLock l = _lock.lock())
{ {
@ -304,10 +304,10 @@ public class DoSHandler extends ConditionalHandler.ElseNext
if (cyclicTimeouts != null) if (cyclicTimeouts != null)
{ {
// schedule a check to remove this tracker if idle // schedule a check to remove this tracker if idle
_expireAt = now + _idleCheck.toNanos(); _nextIdleCheckAt = now + _idleCheck.toNanos();
cyclicTimeouts.schedule(this); cyclicTimeouts.schedule(this);
} }
return _rateControl.isRateExceededBySample(now); return _rateControl.onRequest(now);
} }
} }
@ -315,6 +315,12 @@ public class DoSHandler extends ConditionalHandler.ElseNext
{ {
try (AutoLock l = _lock.lock()) try (AutoLock l = _lock.lock())
{ {
CyclicTimeouts<Tracker> cyclicTimeouts = _cyclicTimeouts;
if (cyclicTimeouts != null)
{
_nextIdleCheckAt = now + _idleCheck.toNanos();
cyclicTimeouts.schedule(this);
}
return _rateControl.isIdle(now); return _rateControl.isIdle(now);
} }
} }
@ -322,7 +328,7 @@ public class DoSHandler extends ConditionalHandler.ElseNext
@Override @Override
public long getExpireNanoTime() public long getExpireNanoTime()
{ {
return _expireAt; return _nextIdleCheckAt;
} }
@Override @Override
@ -396,7 +402,7 @@ public class DoSHandler extends ConditionalHandler.ElseNext
} }
@Override @Override
public boolean isRateExceededBySample(long now) public boolean onRequest(long now)
{ {
// Count the request // Count the request
_sampleCount++; _sampleCount++;
@ -473,7 +479,7 @@ public class DoSHandler extends ConditionalHandler.ElseNext
public StatusRejectHandler(int status) public StatusRejectHandler(int status)
{ {
_status = status >= 0 ? status : HttpStatus.TOO_MANY_REQUESTS_429; _status = status >= 0 ? status : HttpStatus.TOO_MANY_REQUESTS_429;
if (!HttpStatus.isClientError(_status) && !HttpStatus.isServerError(_status)) if (_status != 0 && _status != HttpStatus.OK_200 && !HttpStatus.isClientError(_status) && !HttpStatus.isServerError(_status))
throw new IllegalArgumentException("status must be a client or server error"); throw new IllegalArgumentException("status must be a client or server error");
} }
@ -602,14 +608,14 @@ public class DoSHandler extends ConditionalHandler.ElseNext
_scheduler.schedule(this::onTick, _delayMs / 2, TimeUnit.MILLISECONDS); _scheduler.schedule(this::onTick, _delayMs / 2, TimeUnit.MILLISECONDS);
} }
if (rejects != null) if (rejects != null)
{ {
for (Exchange exchange : rejects) for (Exchange exchange : rejects)
{ {
try try
{ {
Response.writeError(exchange.request, exchange.response, exchange.callback, HttpStatus.TOO_MANY_REQUESTS_429); if (!_reject.handle(exchange.request, exchange.response, exchange.callback))
exchange.callback.failed(new RejectedExecutionException());
} }
catch (Throwable t) catch (Throwable t)
{ {

View File

@ -42,7 +42,7 @@ public class DoSHandlerTest
for (int sample = 0; sample < 400; sample++) for (int sample = 0; sample < 400; sample++)
{ {
boolean exceeded = tracker.isRateExceededBySample(now); boolean exceeded = tracker.onRequest(now);
assertFalse(exceeded); assertFalse(exceeded);
now += TimeUnit.MILLISECONDS.toNanos(11); now += TimeUnit.MILLISECONDS.toNanos(11);
} }
@ -60,7 +60,7 @@ public class DoSHandlerTest
boolean exceeded = false; boolean exceeded = false;
for (int sample = 0; sample < 200; sample++) for (int sample = 0; sample < 200; sample++)
{ {
if (tracker.isRateExceededBySample(now)) if (tracker.onRequest(now))
{ {
exceeded = true; exceeded = true;
break; break;
@ -82,7 +82,7 @@ public class DoSHandlerTest
{ {
for (int burst = 0; burst < 9; burst++) for (int burst = 0; burst < 9; burst++)
{ {
boolean exceeded = tracker.isRateExceededBySample(now); boolean exceeded = tracker.onRequest(now);
assertFalse(exceeded); assertFalse(exceeded);
} }
@ -102,7 +102,7 @@ public class DoSHandlerTest
{ {
for (int burst = 0; burst < 11; burst++) for (int burst = 0; burst < 11; burst++)
{ {
if (tracker.isRateExceededBySample(now)) if (tracker.onRequest(now))
{ {
exceeded = true; exceeded = true;
break; break;
@ -126,7 +126,7 @@ public class DoSHandlerTest
{ {
for (int burst = 0; burst < 99; burst++) for (int burst = 0; burst < 99; burst++)
{ {
boolean exceeded = tracker.isRateExceededBySample(now); boolean exceeded = tracker.onRequest(now);
assertFalse(exceeded); assertFalse(exceeded);
} }
@ -146,7 +146,7 @@ public class DoSHandlerTest
{ {
for (int burst = 0; burst < 101; burst++) for (int burst = 0; burst < 101; burst++)
{ {
if (tracker.isRateExceededBySample(now)) if (tracker.onRequest(now))
{ {
exceeded = true; exceeded = true;
break; break;
@ -169,7 +169,7 @@ public class DoSHandlerTest
for (int seconds = 0; seconds < 2; seconds++) for (int seconds = 0; seconds < 2; seconds++)
{ {
for (int burst = 0; burst < 99; burst++) for (int burst = 0; burst < 99; burst++)
assertFalse(tracker.isRateExceededBySample(now++)); assertFalse(tracker.onRequest(now++));
now += TimeUnit.MILLISECONDS.toNanos(1000) - 100; now += TimeUnit.MILLISECONDS.toNanos(1000) - 100;
} }
@ -180,7 +180,7 @@ public class DoSHandlerTest
for (int seconds = 0; seconds < 2; seconds++) for (int seconds = 0; seconds < 2; seconds++)
{ {
for (int burst = 0; burst < 49; burst++) for (int burst = 0; burst < 49; burst++)
assertFalse(tracker.isRateExceededBySample(now++)); assertFalse(tracker.onRequest(now++));
now += TimeUnit.MILLISECONDS.toNanos(1000) - 100; now += TimeUnit.MILLISECONDS.toNanos(1000) - 100;
} }