From 0c69b648ae2fe59f3e1398dcb6f8edce80d5d783 Mon Sep 17 00:00:00 2001
From: Tiger Wang <ziwei.tiger@outlook.com>
Date: Fri, 13 Nov 2020 21:09:42 +0000
Subject: [PATCH] HTTPServerConnection: more robust error handling

* Fix passing a nullptr to downstream code when the request was malformed
+ Reset the connection on errors
* Fixes #5029
---
 src/HTTP/HTTPServerConnection.cpp | 39 ++++++++++++++-----------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/src/HTTP/HTTPServerConnection.cpp b/src/HTTP/HTTPServerConnection.cpp
index 8c1afb1f0..df9c0970a 100644
--- a/src/HTTP/HTTPServerConnection.cpp
+++ b/src/HTTP/HTTPServerConnection.cpp
@@ -14,21 +14,15 @@
 
 cHTTPServerConnection::cHTTPServerConnection(cHTTPServer & a_HTTPServer) :
 	m_HTTPServer(a_HTTPServer),
-	m_Parser(*this),
-	m_CurrentRequest(nullptr)
+	m_Parser(*this)
 {
-	// LOGD("HTTP: New connection at %p", this);
 }
 
 
 
 
 
-cHTTPServerConnection::~cHTTPServerConnection()
-{
-	// LOGD("HTTP: Connection deleting: %p", this);
-	m_CurrentRequest.reset();
-}
+cHTTPServerConnection::~cHTTPServerConnection() = default;
 
 
 
@@ -101,6 +95,7 @@ void cHTTPServerConnection::Terminate(void)
 	{
 		m_HTTPServer.RequestFinished(*this, *m_CurrentRequest);
 	}
+	m_Link->Close();  // Terminate the connection
 	m_Link.reset();
 }
 
@@ -153,7 +148,7 @@ void cHTTPServerConnection::OnError(int a_ErrorCode, const AString & a_ErrorMsg)
 
 void cHTTPServerConnection::OnError(const AString & a_ErrorDescription)
 {
-	OnRemoteClosed();
+	Terminate();  // HTTP data malformed, disconnect
 }
 
 
@@ -162,15 +157,16 @@ void cHTTPServerConnection::OnError(const AString & a_ErrorDescription)
 
 void cHTTPServerConnection::OnFirstLine(const AString & a_FirstLine)
 {
-	// Create a new request object for this request:
-	auto split = StringSplit(a_FirstLine, " ");
-	if (split.size() < 2)
+	const auto Split = StringSplit(a_FirstLine, " ");
+	if (Split.size() < 2)
 	{
-		// Invalid request line. We need at least the Method and URL
-		OnRemoteClosed();
+		// Invalid request line. We need at least the Method and URL:
+		Terminate();
 		return;
 	}
-	m_CurrentRequest.reset(new cHTTPIncomingRequest(split[0], split[1]));
+
+	// Create a new request object for this request:
+	m_CurrentRequest = std::make_unique<cHTTPIncomingRequest>(Split[0], Split[1]);
 }
 
 
@@ -218,8 +214,13 @@ void cHTTPServerConnection::OnBodyData(const void * a_Data, size_t a_Size)
 
 void cHTTPServerConnection::OnBodyFinished(void)
 {
-	// Process the request and reset:
-	m_HTTPServer.RequestFinished(*this, *m_CurrentRequest);
+	// Process the request:
+	if (m_CurrentRequest != nullptr)
+	{
+		m_HTTPServer.RequestFinished(*this, *m_CurrentRequest);
+	}
+
+	// ...and reset:
 	m_CurrentRequest.reset();
 	m_Parser.Reset();
 }
@@ -232,7 +233,3 @@ void cHTTPServerConnection::SendData(const void * a_Data, size_t a_Size)
 {
 	m_Link->Send(a_Data, a_Size);
 }
-
-
-
-