diff --git a/proxygen/lib/http/coro/HTTPCoroSession.cpp b/proxygen/lib/http/coro/HTTPCoroSession.cpp index a7d785e37..c97744d51 100644 --- a/proxygen/lib/http/coro/HTTPCoroSession.cpp +++ b/proxygen/lib/http/coro/HTTPCoroSession.cpp @@ -766,9 +766,6 @@ folly::coro::Task HTTPUniplexTransportSession::runImpl() { void HTTPCoroSession::sendPreface() { codec_->generateConnectionPreface(writeBuf_); - if (isUpstream()) { - setSetting(SettingsId::ENABLE_PUSH, 1); - } applyEgressSettings(); codec_->generateSettings(writeBuf_); writeEvent_.signal(); diff --git a/proxygen/lib/http/coro/test/HTTPUpstreamCoroSessionTests.cpp b/proxygen/lib/http/coro/test/HTTPUpstreamCoroSessionTests.cpp index 6b3917252..9fd58673a 100644 --- a/proxygen/lib/http/coro/test/HTTPUpstreamCoroSessionTests.cpp +++ b/proxygen/lib/http/coro/test/HTTPUpstreamCoroSessionTests.cpp @@ -216,6 +216,8 @@ class HTTPUpstreamSessionTest : public HTTPCoroSessionTest { /* Simple push test */ folly::coro::Task testPush( bool expectIncomingStream = true, bool pushEOM = true, bool eof = true) { + enablePushForTest(); + // send a request for GET / auto responseSource = co_await co_awaitTry( session_->sendRequest(HTTPFixedSource::makeFixedRequest("/"))); @@ -248,6 +250,12 @@ class HTTPUpstreamSessionTest : public HTTPCoroSessionTest { co_return pushID; } + void enablePushForTest() { + if (!isHQ()) { + codec_->getEgressSettings()->setSetting(SettingsId::ENABLE_PUSH, 1); + } + } + void expectH1ConnectionReset() { if (IS_H1()) { EXPECT_TRUE(transportState_.closedWithReset); @@ -255,6 +263,20 @@ class HTTPUpstreamSessionTest : public HTTPCoroSessionTest { } } + folly::coro::Task expectAdvertisedPushSetting( + SettingsValue expectedValue) { + co_await rescheduleN(2); + parseOutputUniplex(); + + const auto *enablePush = + peerCodec_->getIngressSettings()->getSetting(SettingsId::ENABLE_PUSH); + EXPECT_NE(enablePush, nullptr); + if (enablePush) { + EXPECT_EQ(enablePush->value, expectedValue); + } + transport_->addReadEvent(nullptr, true); + } + HTTPCodec *serverCodec_{nullptr}; std::list>> pushes_; std::function(HTTPPushEvent)> pushPromiseHandler_; @@ -271,6 +293,24 @@ using H2QUpstreamSessionTest = HTTPUpstreamSessionTest; // Use this test class for h1/h2 only tests using H12UpstreamSessionTest = HTTPUpstreamSessionTest; +class H2UpstreamPushEnabledSessionTest : public HTTPUpstreamSessionTest { + public: + H2UpstreamPushEnabledSessionTest() { + initSelfCodec_ = [](HTTPCodec &codec) { + codec.getEgressSettings()->setSetting(SettingsId::ENABLE_PUSH, 1); + }; + } +}; + +CO_TEST_P_X(H2UpstreamSessionTest, PreservesDefaultServerPushSetting) { + co_await expectAdvertisedPushSetting(0); +} + +CO_TEST_P_X(H2UpstreamPushEnabledSessionTest, + PreservesConfiguredServerPushSetting) { + co_await expectAdvertisedPushSetting(1); +} + CO_TEST_P_X(HTTPUpstreamSessionTest, Simple) { auto responseSource = co_await co_awaitTry( session_->sendRequest(HTTPFixedSource::makeFixedRequest("/"))); @@ -708,6 +748,7 @@ CO_TEST_P_X(H2UpstreamSessionTest, PushParentReset) { // // Note, we need the PUSH_PROMISE to have a continuation, so give the server // codec a ridiculously small MAX_FRAME_SIZE. + enablePushForTest(); auto *settings = (HTTPSettings *)serverCodec_->getIngressSettings(); settings->setSetting(SettingsId::MAX_FRAME_SIZE, 5); @@ -1907,6 +1948,12 @@ INSTANTIATE_TEST_SUITE_P( Values(TestParams({.codecProtocol = CodecProtocol::HTTP_2})), paramsToTestName); +INSTANTIATE_TEST_SUITE_P( + HTTPUpstreamSessionTest, + H2UpstreamPushEnabledSessionTest, + Values(TestParams({.codecProtocol = CodecProtocol::HTTP_2})), + paramsToTestName); + INSTANTIATE_TEST_SUITE_P( HTTPUpstreamSessionTest, HQUpstreamSessionTest,