Skip to content

Commit

Permalink
Implement HTTP padding at session layer
Browse files Browse the repository at this point in the history
Summary: Integrate the recently introduced codec APIs for padding into the session abstraction.

Reviewed By: knekritz

Differential Revision: D62219973

fbshipit-source-id: c75dfc5d01f3284d2b28c11418edb5ba8df2703d
  • Loading branch information
dcsommer authored and facebook-github-bot committed Oct 9, 2024
1 parent f8fba79 commit 88cb8af
Show file tree
Hide file tree
Showing 15 changed files with 119 additions and 1 deletion.
15 changes: 15 additions & 0 deletions proxygen/lib/http/session/HQSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3298,6 +3298,21 @@ size_t HQSession::HQStreamTransportBase::sendChunkTerminator(
return encodedSize;
}

size_t HQSession::HQStreamTransportBase::sendPadding(
HTTPTransaction* txn, uint16_t padding) noexcept {
VLOG(4) << __func__ << " txn=" << txn_ << " padding=" << padding;
CHECK(hasEgressStreamId()) << __func__ << " invoked on stream without egress";
DCHECK(txn == &txn_);
auto g = folly::makeGuard(setActiveCodec(__func__));
CHECK(codecStreamId_);
size_t encodedSize =
codecFilterChain->generatePadding(writeBuf_, *codecStreamId_, padding);
if (encodedSize > 0) {
notifyPendingEgress();
}
return encodedSize;
}

void HQSession::HQStreamTransportBase::onMessageBegin(
HTTPCodec::StreamID streamID, HTTPMessage* /* msg */) {
VLOG(4) << __func__ << " txn=" << txn_ << " streamID=" << streamID
Expand Down
2 changes: 2 additions & 0 deletions proxygen/lib/http/session/HQSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -1339,6 +1339,8 @@ class HQSession

size_t sendChunkTerminator(HTTPTransaction* txn) noexcept override;

size_t sendPadding(HTTPTransaction* txn, uint16_t bytes) noexcept override;

size_t sendEOM(HTTPTransaction* txn,
const HTTPHeaders* trailers) noexcept override;

Expand Down
11 changes: 11 additions & 0 deletions proxygen/lib/http/session/HTTPSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1826,6 +1826,17 @@ void HTTPSession::ShutdownTransportCallback::runLoopCallback() noexcept {
delete dg;
}

size_t HTTPSession::sendPadding(HTTPTransaction* txn, uint16_t bytes) noexcept {
auto encodedSize = codec_->generatePadding(writeBuf_, txn->getID(), bytes);
LOG(ERROR) << *this << " sending " << bytes
<< " bytes of padding, encodedSize=" << encodedSize
<< " for streamID=" << txn->getID();
if (encodedSize > 0) {
scheduleWrite();
}
return encodedSize;
}

size_t HTTPSession::sendEOM(HTTPTransaction* txn,
const HTTPHeaders* trailers) noexcept {

Expand Down
1 change: 1 addition & 0 deletions proxygen/lib/http/session/HTTPSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ class HTTPSession
}
size_t sendChunkHeader(HTTPTransaction* txn, size_t length) noexcept override;
size_t sendChunkTerminator(HTTPTransaction* txn) noexcept override;
size_t sendPadding(HTTPTransaction* txn, uint16_t bytes) noexcept override;
size_t sendEOM(HTTPTransaction* txn,
const HTTPHeaders* trailers) noexcept override;
size_t sendAbort(HTTPTransaction* txn,
Expand Down
10 changes: 10 additions & 0 deletions proxygen/lib/http/session/HTTPTransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1477,6 +1477,16 @@ size_t HTTPTransaction::maybeSendDeferredNoError() {
return bytes;
}

void HTTPTransaction::sendPadding(uint16_t bytes) {
VLOG(4) << "egress padding=" << bytes << " on " << *this;
if (!validateEgressStateTransition(
// Sending padding is valid only when you can send body
HTTPTransactionEgressSM::Event::sendBody)) {
return;
}
transport_.sendPadding(this, bytes);
}

size_t HTTPTransaction::sendEOMNow() {
DestructorGuard g(this);
VLOG(4) << "egress EOM on " << *this;
Expand Down
17 changes: 17 additions & 0 deletions proxygen/lib/http/session/HTTPTransaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,9 @@ class HTTPTransaction

virtual size_t sendChunkTerminator(HTTPTransaction* txn) noexcept = 0;

virtual size_t sendPadding(HTTPTransaction* txn,
uint16_t bytes) noexcept = 0;

virtual size_t sendEOM(HTTPTransaction* txn,
const HTTPHeaders* trailers) noexcept = 0;

Expand Down Expand Up @@ -1211,6 +1214,20 @@ class HTTPTransaction
*/
virtual void sendBody(std::unique_ptr<folly::IOBuf> body);

/**
* Send padding bytes that the receiving application layer will ignore. This
* is currently implemented only for HTTP/2 and HTTP/3 and will do nothing on
* HTTP/1 connections.
*
* sendPadding() may be called only when sendBody() is also valid to call.
*
* @param bytes The number of bytes of padding to send on this transaction.
* The actual serialized size of the padding will be greater than this number
* by some O(1) amount, depending on the exact framing and later transport
* encryption.
*/
virtual void sendPadding(uint16_t bytes);

/**
* Returns the cumulative size of body passed to sendBody so far
*/
Expand Down
24 changes: 24 additions & 0 deletions proxygen/lib/http/session/test/HQDownstreamSessionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,30 @@ TEST_P(HQDownstreamSessionTest, SimplePost) {
hqSession_->closeWhenIdle();
}

TEST_P(HQDownstreamSessionTest, SimplePostWithPadding) {
auto id = sendRequest(getPostRequest(10), false);
auto& request = getStream(id);
// Include padding on the request
request.codec->generatePadding(request.buf, request.id, 10'000);
request.codec->generateBody(
request.buf, request.id, makeBuf(10), HTTPCodec::NoPadding, true);
request.readEOF = true;
auto handler = addSimpleStrictHandler();
handler->expectHeaders();
handler->expectBody([](uint64_t, std::shared_ptr<folly::IOBuf> body) {
EXPECT_EQ(body->computeChainDataLength(), 10);
});
handler->expectEOM([&handler] {
// Include padding on the reply too
handler->sendReplyWithBody(200, 100, true, true, false, 200);
});
handler->expectDetachTransaction();
flushRequestsAndLoop();
EXPECT_GT(socketDriver_->streams_[id].writeBuf.chainLength(), 315);
EXPECT_TRUE(socketDriver_->streams_[id].writeEOF);
hqSession_->closeWhenIdle();
}

TEST_P(HQDownstreamSessionTest, SimpleGetEofDelay) {
auto idh = checkRequest();
flushRequestsAndLoop(false, std::chrono::milliseconds(10));
Expand Down
1 change: 1 addition & 0 deletions proxygen/lib/http/session/test/HQUpstreamSessionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ using HQUpstreamSessionTestDatagram = HQUpstreamSessionTest;
TEST_P(HQUpstreamSessionTest, SimpleGet) {
auto handler = openTransaction();
handler->txn_->sendHeaders(getGetRequest());
handler->txn_->sendPadding(123); // ignored by peer
handler->txn_->sendEOM();
handler->expectHeaders();
handler->expectBody();
Expand Down
22 changes: 22 additions & 0 deletions proxygen/lib/http/session/test/HTTPDownstreamSessionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1111,6 +1111,28 @@ TEST_F(HTTPDownstreamSessionTest, BadContentLength) {
flushRequestsAndLoop();
}

TEST_F(HTTP2DownstreamSessionTest, FrameBasedPadding) {
// Send a request with padding. Padding should not affect anything.
auto handler = addSimpleStrictHandler();
auto id = sendHeader();
clientCodec_->generatePadding(requests_, id, 123);
clientCodec_->generateEOM(requests_, id);
handler->expectHeaders();
handler->expectEOM([&handler] { handler->sendReplyWithBody(200, 100); });
handler->expectGoaway();
flushRequestsAndLoopN(1);
handler->expectDetachTransaction();
HTTPSession::DestructorGuard g(httpSession_);
gracefulShutdown();

NiceMock<MockHTTPCodecCallback> callbacks;
clientCodec_->setCallback(&callbacks);

InSequence enforceOrder;
EXPECT_CALL(callbacks, onHeadersComplete(_, _));
parseOutput(*clientCodec_);
}

TEST_F(HTTPDownstreamSessionTest, Connect) {
InSequence enforceOrder;

Expand Down
6 changes: 5 additions & 1 deletion proxygen/lib/http/session/test/HTTPSessionMocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,18 @@ class HTTPHandlerBase {
uint32_t content_length,
bool keepalive = true,
bool sendEOM = true,
bool hasTrailers = false) {
bool hasTrailers = false,
uint16_t padding = 0) {
sendHeaders(code, content_length, keepalive);
sendBody(content_length);
if (hasTrailers) {
HTTPHeaders trailers;
trailers.add("X-Trailer1", "Foo");
txn_->sendTrailers(trailers);
}
if (padding) {
txn_->sendPadding(padding);
}
if (sendEOM) {
txn_->sendEOM();
}
Expand Down
2 changes: 2 additions & 0 deletions proxygen/lib/http/session/test/HTTPTransactionMocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ class MockHTTPTransactionTransport : public HTTPTransaction::Transport {
(HTTPTransaction*, size_t),
(noexcept));
MOCK_METHOD((size_t), sendChunkTerminator, (HTTPTransaction*), (noexcept));
MOCK_METHOD((size_t), sendPadding, (HTTPTransaction*, uint16_t), (noexcept));
MOCK_METHOD((size_t),
sendEOM,
(HTTPTransaction*, const HTTPHeaders*),
Expand Down Expand Up @@ -390,6 +391,7 @@ class MockHTTPTransaction : public HTTPTransaction {
return HTTPTransaction::sendAbort();
}

MOCK_METHOD(void, sendPadding, (uint16_t));
MOCK_METHOD(void, sendChunkHeader, (size_t));
MOCK_METHOD(void, sendChunkTerminator, ());
MOCK_METHOD(void, sendTrailers, (const HTTPHeaders& trailers));
Expand Down
2 changes: 2 additions & 0 deletions proxygen/lib/http/session/test/HTTPUpstreamSessionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,12 @@ std::unique_ptr<folly::IOBuf> getResponseBuf(CodecProtocol protocol,
}
HTTPMessage resp = getResponse(code, bodyLen);
egressCodec->generateHeader(respBufQ, id, resp);
egressCodec->generatePadding(respBufQ, id, 123);
if (bodyLen > 0) {
auto buf = makeBuf(bodyLen);
egressCodec->generateBody(
respBufQ, id, std::move(buf), HTTPCodec::NoPadding, true /* eom */);
egressCodec->generatePadding(respBufQ, id, 42);
}
return respBufQ.move();
}
Expand Down
1 change: 1 addition & 0 deletions proxygen/lib/http/sink/HTTPSink.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class HTTPSink {
virtual void sendChunkHeader(size_t length) = 0;
virtual void sendChunkTerminator() = 0;
virtual void sendTrailers(const HTTPHeaders& trailers) = 0;
virtual void sendPadding(uint16_t bytes) = 0;
virtual void sendEOM() = 0;
virtual bool isEgressEOMSeen() = 0;
virtual void sendAbort() = 0;
Expand Down
3 changes: 3 additions & 0 deletions proxygen/lib/http/sink/HTTPTransactionSink.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,9 @@ class HTTPTransactionSink : public HTTPSink {
void sendTrailers(const HTTPHeaders& trailers) override {
httpTransaction_->sendTrailers(trailers);
}
void sendPadding(uint16_t bytes) override {
httpTransaction_->sendPadding(bytes);
}
void sendEOM() override {
httpTransaction_->sendEOM();
}
Expand Down
3 changes: 3 additions & 0 deletions proxygen/lib/http/sink/HTTPTunnelSink.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ class HTTPTunnelSink
void sendTrailers(const HTTPHeaders&) override {
}

void sendPadding(uint16_t) override {
}

void sendEOM() override;

bool isEgressEOMSeen() override;
Expand Down

0 comments on commit 88cb8af

Please sign in to comment.