Skip to content

Commit

Permalink
Use HeadersRateLimitFilter for headers rate limiting
Browse files Browse the repository at this point in the history 
Summary: My intention is to have separate filters to rate limit each of the events (control messages, headers, direct responses, etc.). This commit is one step in that process. Previously, the `ControlMessageRateLimitFilter` used to handle rate limiting for headers, and I'm switching to use the `HeadersRateLimitFilter` instead.

Reviewed By: afrind

Differential Revision: D50698729

fbshipit-source-id: 876c9526fa6b5077ef223fcedcef9e82b6a865e0
  • Loading branch information
@facebook-github-bot
Aman Sharma authored and facebook-github-bot committed Nov 4, 2023
1 parent 8ce3f02 commit d9a52a7
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 79 deletions.
57 changes: 5 additions & 52 deletions proxygen/lib/http/codec/ControlMessageRateLimitFilter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,9 @@ constexpr uint32_t kDefaultMaxDirectErrorHandlingPerInterval = 100;
constexpr uint32_t kMaxDirectErrorHandlingPerIntervalLowerBound = 50;
constexpr std::chrono::milliseconds kDefaultDirectErrorHandlingDuration{100};

constexpr uint32_t kDefaultMaxHeadersPerInterval = 50000;
constexpr uint32_t kMaxHeadersPerIntervalLowerBound = 100;
constexpr std::chrono::milliseconds kDefaultHeadersDuration{100};

enum RateLimitTarget {
CONTROL_MSGS,
DIRECT_ERROR_HANDLING,
HEADERS,
};

/**
Expand All @@ -54,31 +49,24 @@ class ControlMessageRateLimitFilter : public PassThroughHTTPCodecFilter {
resetDirectErrors_(numDirectErrorHandlingInCurrentInterval_,
RateLimitTarget::DIRECT_ERROR_HANDLING,
httpSessionStats),
resetHeaders_(numHeadersInCurrentInterval_,
RateLimitTarget::HEADERS,
httpSessionStats),
timer_(timer),
httpSessionStats_(httpSessionStats) {
}

void setSessionStats(HTTPSessionStats* httpSessionStats) {
httpSessionStats_ = httpSessionStats;
resetControlMessages_.httpSessionStats = httpSessionStats;
resetHeaders_.httpSessionStats = httpSessionStats;
}

void setParams(uint32_t maxControlMsgsPerInterval,
uint32_t maxDirectErrorHandlingPerInterval,
uint32_t maxHeadersPerInterval,
std::chrono::milliseconds controlMsgIntervalDuration,
std::chrono::milliseconds directErrorHandlingIntervalDuration,
std::chrono::milliseconds headersIntervalDuration) {
void setParams(
uint32_t maxControlMsgsPerInterval,
uint32_t maxDirectErrorHandlingPerInterval,
std::chrono::milliseconds controlMsgIntervalDuration,
std::chrono::milliseconds directErrorHandlingIntervalDuration) {
maxControlMsgsPerInterval_ = maxControlMsgsPerInterval;
maxDirectErrorHandlingPerInterval_ = maxDirectErrorHandlingPerInterval;
controlMsgIntervalDuration_ = controlMsgIntervalDuration;
directErrorHandlingIntervalDuration_ = directErrorHandlingIntervalDuration;
maxHeadersPerInterval_ = maxHeadersPerInterval;
headersIntervalDuration_ = headersIntervalDuration;
}

// Filter functions
Expand Down Expand Up @@ -110,13 +98,6 @@ class ControlMessageRateLimitFilter : public PassThroughHTTPCodecFilter {
}
}

void onHeadersComplete(StreamID stream,
std::unique_ptr<HTTPMessage> msg) override {
if (!incrementNumHeadersInCurInterval()) {
callback_->onHeadersComplete(stream, std::move(msg));
}
}

void onError(HTTPCodec::StreamID streamID,
const HTTPException& error,
bool newTxn) override {
Expand All @@ -135,7 +116,6 @@ class ControlMessageRateLimitFilter : public PassThroughHTTPCodecFilter {
void detachThreadLocals() {
resetControlMessages_.cancelTimeout();
resetDirectErrors_.cancelTimeout();
resetHeaders_.cancelTimeout();
timer_ = nullptr;
// Free pass when switching threads
numControlMsgsInCurrentInterval_ = 0;
Expand Down Expand Up @@ -172,25 +152,6 @@ class ControlMessageRateLimitFilter : public PassThroughHTTPCodecFilter {
return false;
}

bool incrementNumHeadersInCurInterval() {
if (numHeadersInCurrentInterval_ == 0) {
// The first header (or first after a reset) schedules the next
// reset timer
CHECK(timer_);
timer_->scheduleTimeout(&resetHeaders_, headersIntervalDuration_);
}

if (++numHeadersInCurrentInterval_ > maxHeadersPerInterval_) {
if (httpSessionStats_) {
httpSessionStats_->recordHeadersRateLimited();
}
callback_->onGoaway(http2::kMaxStreamID, ErrorCode::NO_ERROR);
return true;
}

return false;
}

bool incrementDirectErrorHandlingInCurInterval() {
if (numDirectErrorHandlingInCurrentInterval_ == 0) {
// The first control message (or first after a reset) schedules the next
Expand Down Expand Up @@ -235,9 +196,6 @@ class ControlMessageRateLimitFilter : public PassThroughHTTPCodecFilter {
case RateLimitTarget::DIRECT_ERROR_HANDLING:
// No stats for this one
break;
case RateLimitTarget::HEADERS:
httpSessionStats->recordHeadersInInterval(counter);
break;
}
}
counter = 0;
Expand All @@ -262,18 +220,13 @@ class ControlMessageRateLimitFilter : public PassThroughHTTPCodecFilter {
uint32_t maxDirectErrorHandlingPerInterval_{
kDefaultMaxDirectErrorHandlingPerInterval};

uint32_t numHeadersInCurrentInterval_{0};
uint32_t maxHeadersPerInterval_{kDefaultMaxHeadersPerInterval};

std::chrono::milliseconds controlMsgIntervalDuration_{
kDefaultControlMsgDuration};
std::chrono::milliseconds directErrorHandlingIntervalDuration_{
kDefaultDirectErrorHandlingDuration};
std::chrono::milliseconds headersIntervalDuration_{kDefaultHeadersDuration};

ResetCounterTimeout resetControlMessages_;
ResetCounterTimeout resetDirectErrors_;
ResetCounterTimeout resetHeaders_;
folly::HHWheelTimer* timer_{nullptr};
HTTPSessionStats* httpSessionStats_{nullptr};
};
Expand Down
16 changes: 10 additions & 6 deletions proxygen/lib/http/session/HTTPSession.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <proxygen/lib/http/HTTPHeaderSize.h>
#include <proxygen/lib/http/codec/HTTP2Codec.h>
#include <proxygen/lib/http/codec/HTTPChecks.h>
#include <proxygen/lib/http/codec/HeadersRateLimitFilter.h>
#include <proxygen/lib/http/session/HTTPSessionController.h>
#include <proxygen/lib/http/session/HTTPSessionStats.h>
#include <wangle/acceptor/ConnectionManager.h>
Expand Down Expand Up @@ -221,13 +222,16 @@ void HTTPSession::setupCodec() {
// if we really support switching from spdy <-> h2, we need to update
// existing flow control filter
}
if (codec_->supportsParallelRequests() && !controlMessageRateLimitFilter_ &&
sock_ &&
if (codec_->supportsParallelRequests() && sock_ &&
codec_->getTransportDirection() == TransportDirection::DOWNSTREAM) {
controlMessageRateLimitFilter_ = new ControlMessageRateLimitFilter(
&getEventBase()->timer(), sessionStats_);
codec_.addFilters(std::unique_ptr<ControlMessageRateLimitFilter>(
controlMessageRateLimitFilter_));
addRateLimitFilter(RateLimitFilter::Type::HEADERS);

if (!controlMessageRateLimitFilter_) {
controlMessageRateLimitFilter_ = new ControlMessageRateLimitFilter(
&getEventBase()->timer(), sessionStats_);
codec_.addFilters(std::unique_ptr<ControlMessageRateLimitFilter>(
controlMessageRateLimitFilter_));
}
}

codec_.setCallback(this);
Expand Down
22 changes: 10 additions & 12 deletions proxygen/lib/http/session/HTTPSessionBase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,8 @@ void HTTPSessionBase::setSessionStats(HTTPSessionStats* stats) {
void HTTPSessionBase::setControlMessageRateLimitParams(
uint32_t maxControlMsgsPerInterval,
uint32_t maxDirectErrorHandlingPerInterval,
uint32_t maxHeadersPerInterval,
std::chrono::milliseconds controlMsgIntervalDuration,
std::chrono::milliseconds directErrorHandlingIntervalDuration,
std::chrono::milliseconds headersIntervalDuration) {
std::chrono::milliseconds directErrorHandlingIntervalDuration) {

if (maxControlMsgsPerInterval < kMaxControlMsgsPerIntervalLowerBound) {
XLOG_EVERY_MS(WARNING, 60000)
Expand All @@ -95,20 +93,12 @@ void HTTPSessionBase::setControlMessageRateLimitParams(
kMaxDirectErrorHandlingPerIntervalLowerBound;
}

if (maxHeadersPerInterval < kMaxHeadersPerIntervalLowerBound) {
XLOG_EVERY_MS(WARNING, 60000)
<< "Invalid maxHeadersPerInterval: " << maxHeadersPerInterval;
maxHeadersPerInterval = kMaxHeadersPerIntervalLowerBound;
}

if (controlMessageRateLimitFilter_) {
controlMessageRateLimitFilter_->setParams(
maxControlMsgsPerInterval,
maxDirectErrorHandlingPerInterval,
maxHeadersPerInterval,
controlMsgIntervalDuration,
directErrorHandlingIntervalDuration,
headersIntervalDuration);
directErrorHandlingIntervalDuration);
}
}

Expand All @@ -121,6 +111,14 @@ void HTTPSessionBase::setRateLimitParams(
<< "Out of bounds access to rate limit filter array";
RateLimitFilter* rateLimitFilter = rateLimitFilters_.at(typeIndex);
if (rateLimitFilter) {
uint32_t maxEventsPerIntervalLowerBound =
rateLimitFilter->getMaxEventsPerInvervalLowerBound();
if (maxEventsPerInterval < maxEventsPerIntervalLowerBound) {
LOG(WARNING) << "Invalid maxEventsPerInterval for event "
<< RateLimitFilter::toStr(type) << ": "
<< maxEventsPerInterval;
maxEventsPerInterval = maxEventsPerIntervalLowerBound;
}
rateLimitFilter->setParams(maxEventsPerInterval, intervalDuration);
}
}
Expand Down
5 changes: 1 addition & 4 deletions proxygen/lib/http/session/HTTPSessionBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,10 @@ class HTTPSessionBase : public wangle::ManagedConnection {
uint32_t maxControlMsgsPerInterval = kDefaultMaxControlMsgsPerInterval,
uint32_t maxDirectErrorHandlingPerInterval =
kDefaultMaxDirectErrorHandlingPerInterval,
uint32_t maxHeadersPerInterval = kDefaultMaxHeadersPerInterval,
std::chrono::milliseconds controlMsgIntervalDuration =
kDefaultControlMsgDuration,
std::chrono::milliseconds directErrorHandlingIntervalDuration =
kDefaultDirectErrorHandlingDuration,
std::chrono::milliseconds headersIntervalDuration =
kDefaultHeadersDuration);
kDefaultDirectErrorHandlingDuration);

void setRateLimitParams(RateLimitFilter::Type type,
uint32_t maxEventsPerInterval,
Expand Down
10 changes: 5 additions & 5 deletions proxygen/lib/http/session/test/HTTPDownstreamSessionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3728,8 +3728,8 @@ TEST_F(HTTP2DownstreamSessionTest, TestPriorityFCBlocked) {
}

TEST_F(HTTP2DownstreamSessionTest, TestHeadersRateLimitExceeded) {
httpSession_->setControlMessageRateLimitParams(
1000, 1000, 100, seconds(0), seconds(0), seconds(0));
httpSession_->setRateLimitParams(
RateLimitFilter::Type::HEADERS, 100, std::chrono::seconds(0));

std::vector<std::unique_ptr<testing::StrictMock<MockHTTPHandler>>> handlers;
for (int i = 0; i < 100; i++) {
Expand Down Expand Up @@ -3780,7 +3780,7 @@ TEST_F(HTTP2DownstreamSessionTest, TestControlMsgResetRateLimitTouched) {

auto streamid = clientCodec_->createStream();

httpSession_->setControlMessageRateLimitParams(10, 100, 100, milliseconds(0));
httpSession_->setControlMessageRateLimitParams(10, 100, milliseconds(0));

// Send 97 PRIORITY, 1 SETTINGS, and 2 PING frames. This doesn't exceed the
// limit of 10.
Expand Down Expand Up @@ -3821,7 +3821,7 @@ TEST_F(HTTP2DownstreamSessionTest, TestControlMsgResetRateLimitTouched) {
}

TEST_F(HTTP2DownstreamSessionTest, DirectErrorHandlingLimitTouched) {
httpSession_->setControlMessageRateLimitParams(100, 10, 100, milliseconds(0));
httpSession_->setControlMessageRateLimitParams(100, 10, milliseconds(0));

// Send 50 messages, each of which cause direct error handling. Since
// this doesn't exceed the limit, this should not cause the connection
Expand Down Expand Up @@ -3853,7 +3853,7 @@ TEST_F(HTTP2DownstreamSessionTest, DirectErrorHandlingLimitTouched) {
}

TEST_F(HTTP2DownstreamSessionTest, DirectErrorHandlingLimitExceeded) {
httpSession_->setControlMessageRateLimitParams(100, 10, 100, milliseconds(0));
httpSession_->setControlMessageRateLimitParams(100, 10, milliseconds(0));

// Send eleven messages, each of which causes direct error handling. Since
// this exceeds the limit, the connection should be dropped.
Expand Down

0 comments on commit d9a52a7

Please sign in to comment.