From 8fbf72dc9cfba44826a35bc2028ee4204c964ae7 Mon Sep 17 00:00:00 2001
From: Daniil Demin <deminds@ydb.tech>
Date: Fri, 2 Feb 2024 08:04:55 +0000
Subject: [PATCH] Handle bad http headers in requests to YDB

---
 ydb/library/actors/http/http.cpp    | 10 ++++---
 ydb/library/actors/http/http.h      | 10 ++++---
 ydb/library/actors/http/http_ut.cpp | 42 +++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/ydb/library/actors/http/http.cpp b/ydb/library/actors/http/http.cpp
index 9da46e412bbd..8a8e28fd394b 100644
--- a/ydb/library/actors/http/http.cpp
+++ b/ydb/library/actors/http/http.cpp
@@ -119,8 +119,9 @@ void THttpParser<THttpRequest, TSocketBuffer>::Advance(size_t len) {
                         } else {
                             Stage = EParseStage::Done;
                         }
-                    } else {
-                        ProcessHeader(Header);
+                    } else if (!ProcessHeader(Header)) {
+                        Stage = EParseStage::Error;
+                        break;
                     }
                     Headers = TStringBuf(Headers.data(), data.data() - Headers.data());
                 }
@@ -278,8 +279,9 @@ void THttpParser<THttpResponse, TSocketBuffer>::Advance(size_t len) {
                         } else {
                             Stage = EParseStage::Done;
                         }
-                    } else {
-                        ProcessHeader(Header);
+                    } else if (!ProcessHeader(Header)) {
+                        Stage = EParseStage::Error;
+                        break;
                     }
                     Headers = TStringBuf(Headers.data(), data.data() - Headers.data());
                 }
diff --git a/ydb/library/actors/http/http.h b/ydb/library/actors/http/http.h
index d96ab062e899..47ba11cdcc10 100644
--- a/ydb/library/actors/http/http.h
+++ b/ydb/library/actors/http/http.h
@@ -291,16 +291,20 @@ class THttpParser : public HeaderType, public BufferType {
         return target.size() == size;
     }
 
-    void ProcessHeader(TStringBuf& header) {
-        TStringBuf name = header.NextTok(':');
+    bool ProcessHeader(TStringBuf& header) {
+        TStringBuf name;
+        TStringBuf value;
+        if (!header.TrySplit(':', name, value)) {
+            return false;
+        }
         TrimBegin(name, ' ');
-        TStringBuf value = header;
         Trim(value, ' ');
         auto cit = HeaderType::HeadersLocation.find(name);
         if (cit != HeaderType::HeadersLocation.end()) {
             this->*cit->second = value;
         }
         header.Clear();
+        return true;
     }
 
     size_t ParseHex(TStringBuf value) {
diff --git a/ydb/library/actors/http/http_ut.cpp b/ydb/library/actors/http/http_ut.cpp
index 6c92f3b938f0..6a10b9d5192e 100644
--- a/ydb/library/actors/http/http_ut.cpp
+++ b/ydb/library/actors/http/http_ut.cpp
@@ -50,6 +50,20 @@ Y_UNIT_TEST_SUITE(HttpProxy) {
         UNIT_ASSERT_EQUAL(request->Headers, "Host: test\r\nSome-Header: 32344\r\n\r\n");
     }
 
+    Y_UNIT_TEST(HeaderParsingError_Request) {
+        NHttp::THttpIncomingRequestPtr request = new NHttp::THttpIncomingRequest();
+        EatWholeString(request, "GET /test HTTP/1.1\r\nHost: test\r\nHeader-Without-A-Colon\r\n\r\n");
+        UNIT_ASSERT_C(request->IsError(), static_cast<int>(request->Stage));
+        UNIT_ASSERT_EQUAL_C(request->GetErrorText(), "Invalid http header", static_cast<int>(request->LastSuccessStage));
+    }
+
+    Y_UNIT_TEST(HeaderParsingError_Response) {
+        NHttp::THttpIncomingResponsePtr response = new NHttp::THttpIncomingResponse(nullptr);
+        EatWholeString(response, "HTTP/1.1 200 OK\r\nConnection: close\r\nHeader-Without-A-Colon\r\n\r\n");
+        UNIT_ASSERT_C(response->IsError(), static_cast<int>(response->Stage));
+        UNIT_ASSERT_EQUAL_C(response->GetErrorText(), "Invalid http header", static_cast<int>(response->LastSuccessStage));
+    }
+
     Y_UNIT_TEST(GetWithSpecifiedContentType) {
         NHttp::THttpIncomingRequestPtr request = new NHttp::THttpIncomingRequest();
         EatWholeString(request, "GET /test HTTP/1.1\r\nHost: test\r\nContent-Type: application/json\r\nSome-Header: 32344\r\n\r\n");
@@ -506,4 +520,32 @@ CRA/5XcX13GJwHHj6LCoc3sL7mt8qV9HKY2AOZ88mpObzISZxgPpdKCfjsrdm63V
         UNIT_ASSERT_EQUAL(response->Response->Status, "400");
         UNIT_ASSERT_EQUAL(response->Response->Body, "Invalid http header");
     }
+
+    Y_UNIT_TEST(HeaderWithoutAColon) {
+        NActors::TTestActorRuntimeBase actorSystem;
+        actorSystem.SetUseRealInterconnect();
+        TPortManager portManager;
+        TIpPort port = portManager.GetTcpPort();
+        TAutoPtr<NActors::IEventHandle> handle;
+        actorSystem.Initialize();
+
+        NActors::IActor* proxy = NHttp::CreateHttpProxy();
+        NActors::TActorId proxyId = actorSystem.Register(proxy);
+        actorSystem.Send(new NActors::IEventHandle(proxyId, actorSystem.AllocateEdgeActor(), new NHttp::TEvHttpProxy::TEvAddListeningPort(port)), 0, true);
+        actorSystem.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvConfirmListen>(handle);
+
+        NActors::TActorId serverId = actorSystem.AllocateEdgeActor();
+        actorSystem.Send(new NActors::IEventHandle(proxyId, serverId, new NHttp::TEvHttpProxy::TEvRegisterHandler("/test", serverId)), 0, true);
+
+        NActors::TActorId clientId = actorSystem.AllocateEdgeActor();
+        NHttp::THttpOutgoingRequestPtr httpRequest = NHttp::THttpOutgoingRequest::CreateRequestGet("http://[::1]:" + ToString(port) + "/test");
+        httpRequest->Append("Header-Without-A-Colon\r\n");
+        httpRequest->FinishHeader();
+        actorSystem.Send(new NActors::IEventHandle(proxyId, clientId, new NHttp::TEvHttpProxy::TEvHttpOutgoingRequest(httpRequest)), 0, true);
+
+        NHttp::TEvHttpProxy::TEvHttpIncomingResponse* response = actorSystem.GrabEdgeEvent<NHttp::TEvHttpProxy::TEvHttpIncomingResponse>(handle);
+
+        UNIT_ASSERT_EQUAL(response->Response->Status, "400");
+        UNIT_ASSERT_EQUAL(response->Response->Body, "Invalid http header");
+    }
 }