10 files changed, 162 insertions, 127 deletions

diff --git a/src/ClientAction.h b/src/ClientAction.h
new file mode 100644
index 000000000..444cf7f3d
--- /dev/null
+++ b/src/ClientAction.h
@@ -0,0 +1,4 @@
+
+class cClientAction
+{
+};
diff --git a/src/ClientHandle.cpp b/src/ClientHandle.cpp
index 9062d8cab..c8cd4a98e 100644
--- a/src/ClientHandle.cpp
+++ b/src/ClientHandle.cpp
@@ -2968,6 +2968,7 @@ void cClientHandle::AddWantedChunk(int a_ChunkX, int a_ChunkZ)
 
 void cClientHandle::PacketBufferFull(void)
 {
+	#error
 	// Too much data in the incoming queue, the server is probably too busy, kick the client:
 	LOGERROR("Too much data in queue for client \"%s\" @ %s, kicking them.", m_Username.c_str(), m_IPString.c_str());
 	SendDisconnect("Server busy");
diff --git a/src/Globals.h b/src/Globals.h
index dc5d27636..f20764fb1 100644
--- a/src/Globals.h
+++ b/src/Globals.h
@@ -42,6 +42,8 @@
 	#define ALIGN_8
 	#define ALIGN_16
 	
+	#define WARN_UNUSED
+
 	#define FORMATSTRING(formatIndex, va_argsIndex)
 
 	// MSVC has its own custom version of zu format
@@ -69,6 +71,8 @@
 	#define ALIGN_8 __attribute__((aligned(8)))
 	#define ALIGN_16 __attribute__((aligned(16)))
 
+	#define WARN_UNUSED __attribute__((warn_unused_result))
+
 	// Some portability macros :)
 	#define stricmp strcasecmp
 	
diff --git a/src/Protocol/CMakeLists.txt b/src/Protocol/CMakeLists.txt
index 58a69efbf..e70dcbd57 100644
--- a/src/Protocol/CMakeLists.txt
+++ b/src/Protocol/CMakeLists.txt
@@ -9,6 +9,7 @@ SET (SRCS
 	ChunkDataSerializer.cpp
 	MojangAPI.cpp
 	Packetizer.cpp
+	Protocol.cpp
 	Protocol17x.cpp
 	Protocol18x.cpp
 	ProtocolRecognizer.cpp
diff --git a/src/Protocol/Protocol.cpp b/src/Protocol/Protocol.cpp
new file mode 100644
index 000000000..2c8491a0b
--- /dev/null
+++ b/src/Protocol/Protocol.cpp
@@ -0,0 +1,87 @@
+#include "Globals.h"
+#include "Protocol.h"
+
+cProtocol::cProtocolError cProtocol::DataReceived(const char * a_Data, size_t a_Size, std::vector<std::unique_ptr<cClientAction>> & a_Actions)
+{
+	a_Actions.clear();
+	if (m_IsEncrypted)
+	{
+		ASSERT((a_Size % 16) == 0); // AES requirement.
+		Byte Decrypted[512];
+		while (a_Size > 0)
+		{
+			size_t NumBytes = (a_Size > sizeof(Decrypted)) ? sizeof(Decrypted) : a_Size;
+			m_Decryptor.ProcessData(Decrypted, reinterpret_cast<const Byte *>(a_Data), NumBytes);
+			auto success = AddReceivedData(reinterpret_cast<const char *>(Decrypted), NumBytes, a_Actions);
+			if (success != cProtocolError::Success)
+			{
+				return success;
+			}
+			a_Size -= NumBytes;
+			a_Data += NumBytes;
+		}
+		return cProtocolError::Success;
+	}
+	else
+	{
+		return AddReceivedData(a_Data, a_Size, a_Actions);
+	}
+}
+
+cProtocol::cProtocolError cProtocol::AddReceivedData(const char * a_Data, size_t a_Size, std::vector<std::unique_ptr<cClientAction>> & a_Actions)
+{
+	// Write the incoming data into the comm log file:
+	if (g_ShouldLogCommIn)
+	{
+		if (m_ReceivedData.GetReadableSpace() > 0)
+		{
+			AString AllData;
+			size_t OldReadableSpace = m_ReceivedData.GetReadableSpace();
+			m_ReceivedData.ReadAll(AllData);
+			m_ReceivedData.ResetRead();
+			m_ReceivedData.SkipRead(m_ReceivedData.GetReadableSpace() - OldReadableSpace);
+			ASSERT(m_ReceivedData.GetReadableSpace() == OldReadableSpace);
+			AString Hex;
+			CreateHexDump(Hex, AllData.data(), AllData.size(), 16);
+			m_CommLogFile.Printf("Incoming data, " SIZE_T_FMT " (0x" SIZE_T_FMT_HEX ") unparsed bytes already present in buffer:\n%s\n",
+				AllData.size(), AllData.size(), Hex.c_str()
+			);
+		}
+		AString Hex;
+		CreateHexDump(Hex, a_Data, a_Size, 16);
+		m_CommLogFile.Printf("Incoming data: %u (0x%x) bytes: \n%s\n",
+			static_cast<unsigned>(a_Size), static_cast<unsigned>(a_Size), Hex.c_str()
+		);
+		m_CommLogFile.Flush();
+	}
+
+	if (!m_ReceivedData.Write(a_Data, a_Size))
+	{
+		return cProtocolError::BufferFull;
+	}
+
+	auto status = OnDataAddedToBuffer(m_ReceivedData, a_Actions);
+	if (status != cProtocolError::Success)
+	{
+		return status;
+	}
+
+	
+	// Log any leftover bytes into the logfile:
+	if (g_ShouldLogCommIn && (m_ReceivedData.GetReadableSpace() > 0))
+	{
+		AString AllData;
+		size_t OldReadableSpace = m_ReceivedData.GetReadableSpace();
+		m_ReceivedData.ReadAll(AllData);
+		m_ReceivedData.ResetRead();
+		m_ReceivedData.SkipRead(m_ReceivedData.GetReadableSpace() - OldReadableSpace);
+		ASSERT(m_ReceivedData.GetReadableSpace() == OldReadableSpace);
+		AString Hex;
+		CreateHexDump(Hex, AllData.data(), AllData.size(), 16);
+		m_CommLogFile.Printf("There are " SIZE_T_FMT " (0x" SIZE_T_FMT_HEX ") bytes of non-parse-able data left in the buffer:\n%s",
+			m_ReceivedData.GetReadableSpace(), m_ReceivedData.GetReadableSpace(), Hex.c_str()
+		);
+		m_CommLogFile.Flush();
+	}
+	return cProtocolError::Success;
+}
diff --git a/src/Protocol/Protocol.h b/src/Protocol/Protocol.h
index af0485a78..775c7354c 100644
--- a/src/Protocol/Protocol.h
+++ b/src/Protocol/Protocol.h
@@ -16,6 +16,11 @@
 #include "../Map.h"
 #include "../ByteBuffer.h"
 #include "../EffectID.h"
+#include "../ClientAction.h"
+
+
+#include "PolarSSL++/AesCfb128Decryptor.h"
+#include "PolarSSL++/AesCfb128Encryptor.h"
 
 #include <array>
 
@@ -45,23 +50,45 @@ class cPacketizer;
 typedef unsigned char Byte;
 
 
+// fwd: main.cpp:
+extern bool g_ShouldLogCommIn, g_ShouldLogCommOut;
+
+
 
 
 
 class cProtocol
 {
 public:
-	cProtocol(cClientHandle * a_Client) :
+
+	enum class cProtocolError
+	{
+		Success,
+		BufferFull,
+		PacketError
+	};
+
+	cProtocol(cClientHandle * a_Client, AString a_LogID) :
 		m_Client(a_Client),
 		m_OutPacketBuffer(64 KiB),
-		m_OutPacketLenBuffer(20)  // 20 bytes is more than enough for one VarInt
-	{
+		m_OutPacketLenBuffer(20),  // 20 bytes is more than enough for one VarInt
+		m_IsEncrypted(false),
+		m_ReceivedData(32 KiB)
+	{	
+		// Create the comm log file, if so requested:
+		if (g_ShouldLogCommIn || g_ShouldLogCommOut)
+		{
+			static int sCounter = 0;
+			cFile::CreateFolder("CommLogs");
+			AString FileName = Printf("CommLogs/%x_%d__%s.log", static_cast<unsigned>(time(nullptr)), sCounter++, a_LogID.c_str());
+			m_CommLogFile.Open(FileName, cFile::fmWrite);
+		}
 	}
 
 	virtual ~cProtocol() {}
 	
-	/** Called when client sends some data */
-	virtual void DataReceived(const char * a_Data, size_t a_Size) = 0;
+	/** Called when client sends some data, a_Actions is cleared before being filled */
+	cProtocolError DataReceived(const char * a_Data, size_t a_Size, std::vector<std::unique_ptr<cClientAction>> & a_Actions)  WARN_UNUSED;
 	
 	// Sending stuff to clients (alphabetically sorted):
 	virtual void SendAttachEntity               (const cEntity & a_Entity, const cEntity * a_Vehicle) = 0;
@@ -163,6 +190,12 @@ protected:
 	
 	/** Buffer for composing packet length (so that each cPacketizer instance doesn't allocate a new cPacketBuffer) */
 	cByteBuffer m_OutPacketLenBuffer;
+
+	bool m_IsEncrypted;
+	
+	cAesCfb128Decryptor m_Decryptor;
+	cAesCfb128Encryptor m_Encryptor;
+
 	
 	/** A generic data-sending routine, all outgoing packet data needs to be routed through this so that descendants may override it. */
 	virtual void SendData(const char * a_Data, size_t a_Size) = 0;
@@ -170,6 +203,20 @@ protected:
 	/** Sends a single packet contained within the cPacketizer class.
 	The cPacketizer's destructor calls this to send the contained packet; protocol may transform the data (compression in 1.8 etc). */
 	virtual void SendPacket(cPacketizer & a_Packet) = 0;
+
+	/** This method should append the actions from incoming packets to a_Action */
+	virtual cProtocolError OnDataAddedToBuffer(cByteBuffer & a_Buffer, std::vector<std::unique_ptr<cClientAction>> & a_Action) WARN_UNUSED = 0;
+
+	/** The logfile where the comm is logged, when g_ShouldLogComm is true */
+	cFile m_CommLogFile;
+
+private:
+
+	/** Buffer for the received data */
+	cByteBuffer m_ReceivedData;
+
+	/** Adds the received (unencrypted) data to m_ReceivedData, parses complete packets, appends to a_Actions */
+	cProtocolError AddReceivedData(const char * a_Data, size_t a_Size, std::vector<std::unique_ptr<cClientAction>> & a_Actions) WARN_UNUSED;
 } ;
 
 
diff --git a/src/Protocol/Protocol17x.cpp b/src/Protocol/Protocol17x.cpp
index 010e1a8ba..586efdd85 100644
--- a/src/Protocol/Protocol17x.cpp
+++ b/src/Protocol/Protocol17x.cpp
@@ -101,12 +101,10 @@ extern bool g_ShouldLogCommIn, g_ShouldLogCommOut;
 // cProtocol172:
 
 cProtocol172::cProtocol172(cClientHandle * a_Client, const AString & a_ServerAddress, UInt16 a_ServerPort, UInt32 a_State) :
-	super(a_Client),
+	super(a_Client, a_Client->GetIPString()),
 	m_ServerAddress(a_ServerAddress),
 	m_ServerPort(a_ServerPort),
 	m_State(a_State),
-	m_ReceivedData(32 KiB),
-	m_IsEncrypted(false),
 	m_LastSentDimension(dimNotSet)
 {
 	// BungeeCord handling:
@@ -121,44 +119,12 @@ cProtocol172::cProtocol172(cClientHandle * a_Client, const AString & a_ServerAdd
 		m_Client->SetUUID(cMojangAPI::MakeUUIDShort(Params[2]));
 		m_Client->SetProperties(Params[3]);
 	}
-	
-	// Create the comm log file, if so requested:
-	if (g_ShouldLogCommIn || g_ShouldLogCommOut)
-	{
-		static int sCounter = 0;
-		cFile::CreateFolder("CommLogs");
-		AString FileName = Printf("CommLogs/%x_%d__%s.log", static_cast<unsigned>(time(nullptr)), sCounter++, a_Client->GetIPString().c_str());
-		m_CommLogFile.Open(FileName, cFile::fmWrite);
-	}
 }
 
 
 
 
 
-void cProtocol172::DataReceived(const char * a_Data, size_t a_Size)
-{
-	if (m_IsEncrypted)
-	{
-		Byte Decrypted[512];
-		while (a_Size > 0)
-		{
-			size_t NumBytes = (a_Size > sizeof(Decrypted)) ? sizeof(Decrypted) : a_Size;
-			m_Decryptor.ProcessData(Decrypted, reinterpret_cast<const Byte *>(a_Data), NumBytes);
-			AddReceivedData(reinterpret_cast<const char *>(Decrypted), NumBytes);
-			a_Size -= NumBytes;
-			a_Data += NumBytes;
-		}
-	}
-	else
-	{
-		AddReceivedData(a_Data, a_Size);
-	}
-}
-
-
-
-
 
 void cProtocol172::SendAttachEntity(const cEntity & a_Entity, const cEntity * a_Vehicle)
 {
@@ -1544,59 +1510,27 @@ void cProtocol172::SendWindowProperty(const cWindow & a_Window, short a_Property
 
 
 
-void cProtocol172::AddReceivedData(const char * a_Data, size_t a_Size)
+cProtocol::cProtocolError cProtocol172::OnDataAddedToBuffer(cByteBuffer & a_Buffer, std::vector<std::unique_ptr<cClientAction>> & a_Action)
 {
-	// Write the incoming data into the comm log file:
-	if (g_ShouldLogCommIn)
-	{
-		if (m_ReceivedData.GetReadableSpace() > 0)
-		{
-			AString AllData;
-			size_t OldReadableSpace = m_ReceivedData.GetReadableSpace();
-			m_ReceivedData.ReadAll(AllData);
-			m_ReceivedData.ResetRead();
-			m_ReceivedData.SkipRead(m_ReceivedData.GetReadableSpace() - OldReadableSpace);
-			ASSERT(m_ReceivedData.GetReadableSpace() == OldReadableSpace);
-			AString Hex;
-			CreateHexDump(Hex, AllData.data(), AllData.size(), 16);
-			m_CommLogFile.Printf("Incoming data, " SIZE_T_FMT " (0x" SIZE_T_FMT_HEX ") unparsed bytes already present in buffer:\n%s\n",
-				AllData.size(), AllData.size(), Hex.c_str()
-			);
-		}
-		AString Hex;
-		CreateHexDump(Hex, a_Data, a_Size, 16);
-		m_CommLogFile.Printf("Incoming data: %u (0x%x) bytes: \n%s\n",
-			static_cast<unsigned>(a_Size), static_cast<unsigned>(a_Size), Hex.c_str()
-		);
-		m_CommLogFile.Flush();
-	}
-
-	if (!m_ReceivedData.Write(a_Data, a_Size))
-	{
-		// Too much data in the incoming queue, report to caller:
-		m_Client->PacketBufferFull();
-		return;
-	}
-
 	// Handle all complete packets:
 	for (;;)
 	{
 		UInt32 PacketLen;
-		if (!m_ReceivedData.ReadVarInt(PacketLen))
+		if (!a_Buffer.ReadVarInt(PacketLen))
 		{
 			// Not enough data
-			m_ReceivedData.ResetRead();
+			a_Buffer.ResetRead();
 			break;
 		}
-		if (!m_ReceivedData.CanReadBytes(PacketLen))
+		if (!a_Buffer.CanReadBytes(PacketLen))
 		{
 			// The full packet hasn't been received yet
-			m_ReceivedData.ResetRead();
+			a_Buffer.ResetRead();
 			break;
 		}
 		cByteBuffer bb(PacketLen + 1);
-		VERIFY(m_ReceivedData.ReadToByteBuffer(bb, static_cast<size_t>(PacketLen)));
-		m_ReceivedData.CommitRead();
+		VERIFY(a_Buffer.ReadToByteBuffer(bb, static_cast<size_t>(PacketLen)));
+		a_Buffer.CommitRead();
 
 		UInt32 PacketType;
 		if (!bb.ReadVarInt(PacketType))
@@ -1646,7 +1580,7 @@ void cProtocol172::AddReceivedData(const char * a_Data, size_t a_Size)
 				m_CommLogFile.Printf("^^^^^^ Unhandled packet ^^^^^^\n\n\n");
 			}
 			
-			return;
+			return cProtocolError::PacketError;
 		}
 
 		if (bb.GetReadableSpace() != 1)
@@ -1666,26 +1600,10 @@ void cProtocol172::AddReceivedData(const char * a_Data, size_t a_Size)
 			}
 
 			ASSERT(!"Read wrong number of bytes!");
-			m_Client->PacketError(PacketType);
+			return cProtocolError::PacketError;
 		}
 	}  // for (ever)
-
-	// Log any leftover bytes into the logfile:
-	if (g_ShouldLogCommIn && (m_ReceivedData.GetReadableSpace() > 0))
-	{
-		AString AllData;
-		size_t OldReadableSpace = m_ReceivedData.GetReadableSpace();
-		m_ReceivedData.ReadAll(AllData);
-		m_ReceivedData.ResetRead();
-		m_ReceivedData.SkipRead(m_ReceivedData.GetReadableSpace() - OldReadableSpace);
-		ASSERT(m_ReceivedData.GetReadableSpace() == OldReadableSpace);
-		AString Hex;
-		CreateHexDump(Hex, AllData.data(), AllData.size(), 16);
-		m_CommLogFile.Printf("There are " SIZE_T_FMT " (0x" SIZE_T_FMT_HEX ") bytes of non-parse-able data left in the buffer:\n%s",
-			m_ReceivedData.GetReadableSpace(), m_ReceivedData.GetReadableSpace(), Hex.c_str()
-		);
-		m_CommLogFile.Flush();
-	}
+	return cProtocolError::Success;
 }
 
 
diff --git a/src/Protocol/Protocol17x.h b/src/Protocol/Protocol17x.h
index 747ffe186..41af2e568 100644
--- a/src/Protocol/Protocol17x.h
+++ b/src/Protocol/Protocol17x.h
@@ -31,9 +31,6 @@ Declares the 1.7.x protocol classes:
 	#pragma warning(pop)
 #endif
 
-#include "PolarSSL++/AesCfb128Decryptor.h"
-#include "PolarSSL++/AesCfb128Encryptor.h"
-
 
 
 
@@ -56,9 +53,6 @@ class cProtocol172 :
 public:
 
 	cProtocol172(cClientHandle * a_Client, const AString & a_ServerAddress, UInt16 a_ServerPort, UInt32 a_State);
-	
-	/** Called when client sends some data: */
-	virtual void DataReceived(const char * a_Data, size_t a_Size) override;
 
 	/** Sending stuff to clients (alphabetically sorted): */
 	virtual void SendAttachEntity               (const cEntity & a_Entity, const cEntity * a_Vehicle) override;
@@ -154,25 +148,10 @@ protected:
 	
 	/** State of the protocol. 1 = status, 2 = login, 3 = game */
 	UInt32 m_State;
-
-	/** Buffer for the received data */
-	cByteBuffer m_ReceivedData;
-	
-	bool m_IsEncrypted;
-	
-	cAesCfb128Decryptor m_Decryptor;
-	cAesCfb128Encryptor m_Encryptor;
-
-	/** The logfile where the comm is logged, when g_ShouldLogComm is true */
-	cFile m_CommLogFile;
 	
 	/** The dimension that was last sent to a player in a Respawn or Login packet.
 	Used to avoid Respawning into the same dimension, which confuses the client. */
 	eDimension m_LastSentDimension;
-	
-	
-	/** Adds the received (unencrypted) data to m_ReceivedData, parses complete packets */
-	void AddReceivedData(const char * a_Data, size_t a_Size);
 
 	/** Reads and handles the packet. The packet length and type have already been read.
 	Returns true if the packet was understood, false if it was an unknown packet
@@ -250,6 +229,8 @@ protected:
 
 	/** Writes the block entity data for the specified block entity into the packet. */
 	void WriteBlockEntity(cPacketizer & a_Pkt, const cBlockEntity & a_BlockEntity);
+
+	virtual cProtocolError OnDataAddedToBuffer(cByteBuffer & a_Buffer, std::vector<std::unique_ptr<cClientAction>> & a_Action) override WARN_UNUSED;
 } ;
 
 
diff --git a/src/Protocol/Protocol18x.cpp b/src/Protocol/Protocol18x.cpp
index ad2964bed..c1d01e1f7 100644
--- a/src/Protocol/Protocol18x.cpp
+++ b/src/Protocol/Protocol18x.cpp
@@ -107,7 +107,6 @@ cProtocol180::cProtocol180(cClientHandle * a_Client, const AString & a_ServerAdd
 	m_ServerPort(a_ServerPort),
 	m_State(a_State),
 	m_ReceivedData(32 KiB),
-	m_IsEncrypted(false),
 	m_LastSentDimension(dimNotSet)
 {
 
diff --git a/src/Protocol/Protocol18x.h b/src/Protocol/Protocol18x.h
index 8b5b7ffa2..73f942895 100644
--- a/src/Protocol/Protocol18x.h
+++ b/src/Protocol/Protocol18x.h
@@ -55,9 +55,6 @@ class cProtocol180 :
 public:
 
 	cProtocol180(cClientHandle * a_Client, const AString & a_ServerAddress, UInt16 a_ServerPort, UInt32 a_State);
-	
-	/** Called when client sends some data: */
-	virtual void DataReceived(const char * a_Data, size_t a_Size) override;
 
 	/** Sending stuff to clients (alphabetically sorted): */
 	virtual void SendAttachEntity               (const cEntity & a_Entity, const cEntity * a_Vehicle) override;
@@ -179,10 +176,6 @@ protected:
 	/** The dimension that was last sent to a player in a Respawn or Login packet.
 	Used to avoid Respawning into the same dimension, which confuses the client. */
 	eDimension m_LastSentDimension;
-	
-	
-	/** Adds the received (unencrypted) data to m_ReceivedData, parses complete packets */
-	void AddReceivedData(const char * a_Data, size_t a_Size);
 
 	/** Reads and handles the packet. The packet length and type have already been read.
 	Returns true if the packet was understood, false if it was an unknown packet