From 6642a5aa652b0bcb553ce9237a8a28660a58d8fa Mon Sep 17 00:00:00 2001 From: steveselva <100062566+SteveSelva@users.noreply.github.com> Date: Thu, 7 Nov 2024 22:19:46 +0530 Subject: [PATCH 1/5] Update HTTP1xCodec.cpp Updated HTTP1xCodec to allow Response Body for HTTP CONNECT requests when the response status code is either 4xx or 5xx. --- proxygen/lib/http/codec/HTTP1xCodec.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/proxygen/lib/http/codec/HTTP1xCodec.cpp b/proxygen/lib/http/codec/HTTP1xCodec.cpp index 6d4d668118..1e6e9ab615 100644 --- a/proxygen/lib/http/codec/HTTP1xCodec.cpp +++ b/proxygen/lib/http/codec/HTTP1xCodec.cpp @@ -1198,7 +1198,10 @@ int HTTP1xCodec::onHeadersComplete(size_t len) { ignoreBody = false; } else { is1xxResponse_ = msg_->is1xxResponse(); - if (expectNoResponseBody_) { + if (connectRequest_ && (msg_->is4xxResponse() || msg_->is5xxResponse())) { + ignoreBody = false; + } + else if (expectNoResponseBody_) { ignoreBody = true; } else { ignoreBody = RFC2616::responseBodyMustBeEmpty(msg_->getStatusCode()); From a343a29816e2418c94304dc4adfcddf5e622e7cf Mon Sep 17 00:00:00 2001 From: steveselva <100062566+SteveSelva@users.noreply.github.com> Date: Fri, 8 Nov 2024 10:04:03 +0530 Subject: [PATCH 2/5] Update HTTP1xCodec.cpp --- proxygen/lib/http/codec/HTTP1xCodec.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/proxygen/lib/http/codec/HTTP1xCodec.cpp b/proxygen/lib/http/codec/HTTP1xCodec.cpp index 1e6e9ab615..66c34ea0a7 100644 --- a/proxygen/lib/http/codec/HTTP1xCodec.cpp +++ b/proxygen/lib/http/codec/HTTP1xCodec.cpp @@ -1200,8 +1200,7 @@ int HTTP1xCodec::onHeadersComplete(size_t len) { is1xxResponse_ = msg_->is1xxResponse(); if (connectRequest_ && (msg_->is4xxResponse() || msg_->is5xxResponse())) { ignoreBody = false; - } - else if (expectNoResponseBody_) { + } else if (expectNoResponseBody_) { ignoreBody = true; } else { ignoreBody = RFC2616::responseBodyMustBeEmpty(msg_->getStatusCode()); From 8d8c061982f01b796e6911c6882b1f90a8876b8d Mon Sep 17 00:00:00 2001 From: steveselva <100062566+SteveSelva@users.noreply.github.com> Date: Sun, 10 Nov 2024 23:39:23 +0530 Subject: [PATCH 3/5] Update HTTP1xCodecTest.cpp Added test for checking body with CONNECT request --- .../lib/http/codec/test/HTTP1xCodecTest.cpp | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp b/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp index ba3b13cd0e..cfd6b381cc 100644 --- a/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp +++ b/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp @@ -1530,6 +1530,55 @@ TEST(HTTP1xCodecTest, AbsoluteURLNoPath) { EXPECT_EQ(callbacks.msg_->getPathAsStringPiece(), string("/")); } +TEST(HTTP1xCodecTest, ConnectRequestSuccess) { + HTTP1xCodec codec(TransportDirection::UPSTREAM); + HTTP1xCodecCallback callbacks; + HTTPMessage req; + auto id = codec.createStream(); + req.setHTTPVersion(1, 1); + req.setMethod(HTTPMethod::CONNECT); + req.setURL("facebook.com:443"); + codec.setCallback(&callbacks); + folly::IOBufQueue buf; + codec.generateHeader(buf, id, req, true); + auto buffer = folly::IOBuf::copyBuffer( + string("HTTP/1.1 200 Connection established\r\n\r\n")); + codec.onIngress(*buffer); + EXPECT_EQ(callbacks.headersComplete, 1); + EXPECT_EQ(callbacks.bodyLen, 0); + EXPECT_EQ(callbacks.errors, 0); + EXPECT_EQ(callbacks.messageComplete, 1); +} + +TEST(HTTP1xCodecTest, ConnectRequestError) { + HTTP1xCodec codec(TransportDirection::UPSTREAM); + HTTP1xCodecCallback callbacks; + HTTPMessage req; + auto id = codec.createStream(); + req.setHTTPVersion(1, 1); + req.setMethod(HTTPMethod::CONNECT); + req.setURL("facebook.com:443"); + codec.setCallback(&callbacks); + folly::IOBufQueue buf; + codec.generateHeader(buf, id, req, true); + auto buffer = folly::IOBuf::copyBuffer(string("HTTP/1.1 407 Proxy Authentication Required\r\n" + "Content-Type: text/html;charset=utf-8\r\n" + "Content-Length: 165\r\n" + "Proxy-Authenticate: Basic\r\n" + "Proxy-Authenticate: NTLM\r\n" + "Connection: keep-alive\r\n" + "\r\n" + "ERROR: " + "Cache Access Denied

ERROR

" + "Cache Access Denied.

")); + codec.onIngress(*buffer); + EXPECT_EQ(callbacks.headersComplete, 1); + EXPECT_EQ(callbacks.bodyLen, 165); + EXPECT_EQ(callbacks.errors, 0); + EXPECT_EQ(callbacks.messageComplete, 1); +} + class ConnectionHeaderTest : public TestWithParam, string>> { public: From ca496593e905cadd5ed4dd80d9760d8856ff4b26 Mon Sep 17 00:00:00 2001 From: steveselva <100062566+SteveSelva@users.noreply.github.com> Date: Wed, 20 Nov 2024 21:58:04 +0530 Subject: [PATCH 4/5] Update HTTP1xCodec.cpp Added extra check for ignoreBody with ContentLength header. --- proxygen/lib/http/codec/HTTP1xCodec.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proxygen/lib/http/codec/HTTP1xCodec.cpp b/proxygen/lib/http/codec/HTTP1xCodec.cpp index 66c34ea0a7..31518f53b7 100644 --- a/proxygen/lib/http/codec/HTTP1xCodec.cpp +++ b/proxygen/lib/http/codec/HTTP1xCodec.cpp @@ -1198,7 +1198,8 @@ int HTTP1xCodec::onHeadersComplete(size_t len) { ignoreBody = false; } else { is1xxResponse_ = msg_->is1xxResponse(); - if (connectRequest_ && (msg_->is4xxResponse() || msg_->is5xxResponse())) { + std::string contentLengthHeader = msg_->getHeaders().getSingleOrEmpty(HTTP_HEADER_CONTENT_LENGTH); + if (connectRequest_ && (msg_->is4xxResponse() || msg_->is5xxResponse()) && !contentLengthHeader.empty() && std::stol(contentLengthHeader) != 0) { ignoreBody = false; } else if (expectNoResponseBody_) { ignoreBody = true; From 0d2d627828835a53d5b0d7fb11cbf628059b9cad Mon Sep 17 00:00:00 2001 From: steveselva <100062566+SteveSelva@users.noreply.github.com> Date: Wed, 20 Nov 2024 22:02:29 +0530 Subject: [PATCH 5/5] Update HTTP1xCodecTest.cpp Added test for HTTP 407 without body. --- .../lib/http/codec/test/HTTP1xCodecTest.cpp | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp b/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp index cfd6b381cc..b934a90a65 100644 --- a/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp +++ b/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp @@ -1579,6 +1579,30 @@ TEST(HTTP1xCodecTest, ConnectRequestError) { EXPECT_EQ(callbacks.messageComplete, 1); } +TEST(HTTP1xCodecTest, ConnectRequestErrorWithoutBody) { + HTTP1xCodec codec(TransportDirection::UPSTREAM); + HTTP1xCodecCallback callbacks; + HTTPMessage req; + auto id = codec.createStream(); + req.setHTTPVersion(1, 1); + req.setMethod(HTTPMethod::CONNECT); + req.setURL("facebook.com:443"); + codec.setCallback(&callbacks); + folly::IOBufQueue buf; + codec.generateHeader(buf, id, req, true); + auto buffer = folly::IOBuf::copyBuffer(string("HTTP/1.1 407 Proxy Authentication Required\r\n" + "Content-Type: text/html;charset=utf-8\r\n" + "Proxy-Authenticate: Basic\r\n" + "Proxy-Authenticate: NTLM\r\n" + "Connection: keep-alive\r\n" + "\r\n")); + codec.onIngress(*buffer); + EXPECT_EQ(callbacks.headersComplete, 1); + EXPECT_EQ(callbacks.bodyLen, 0); + EXPECT_EQ(callbacks.errors, 0); + EXPECT_EQ(callbacks.messageComplete, 1); +} + class ConnectionHeaderTest : public TestWithParam, string>> { public: