Fix mishandling of PACKET_SERVER_MAP_BEGIN queuing at server

This could result in broken packet framing in the TCP stream

src/network/core/tcp.cpp

@@ -67,12 +67,28 @@ void NetworkTCPSocketHandler::SendPacket(std::unique_ptr<Packet> packet)
  * if the OS-network-buffer is full)
  * @param packet the packet to send
  */
-void NetworkTCPSocketHandler::SendPrependPacket(std::unique_ptr<Packet> packet)
+void NetworkTCPSocketHandler::SendPrependPacket(std::unique_ptr<Packet> packet, int queue_after_packet_type)
 {
 	assert(packet != nullptr);
 
 	packet->PrepareToSend();
 
+	if (queue_after_packet_type >= 0) {
+		for (auto iter = this->packet_queue.begin(); iter != this->packet_queue.end(); ++iter) {
+			if ((*iter)->GetPacketType() == queue_after_packet_type) {
+				++iter;
+				this->packet_queue.insert(iter, std::move(packet));
+				return;
+			}
+		}
+	}
+
+	/* The very first packet in the queue may be partially written out, so cannot be replaced.
+	 * If the queue is non-empty, swap packet with the first packet in the queue.
+	 * The insert the packet (either the incoming packet or the previous first packet) at the front. */
+	if (!this->packet_queue.empty()) {
+		packet.swap(this->packet_queue.front());
+	}
 	this->packet_queue.push_front(std::move(packet));
 }
 

src/network/core/tcp.h

@@ -44,7 +44,7 @@ public:
 
 	NetworkRecvStatus CloseConnection(bool error = true) override;
 	void SendPacket(std::unique_ptr<Packet> packet);
-	void SendPrependPacket(std::unique_ptr<Packet> packet);
+	void SendPrependPacket(std::unique_ptr<Packet> packet, int queue_after_packet_type);
 
 	void SendPacket(Packet *packet)
 	{

src/network/network_server.cpp

@@ -63,7 +63,7 @@ struct PacketWriter : SaveFilter {
 	std::unique_ptr<Packet> current;    ///< The packet we're currently writing to.
 	size_t total_size;                  ///< Total size of the compressed savegame.
 	std::vector<std::unique_ptr<Packet>> packets; ///< Packet queue of the savegame; send these "slowly" to the client.
-	std::vector<std::unique_ptr<Packet>> prepend_packets; ///< Packet queue of the savegame; send these "slowly" to the client.
+	std::unique_ptr<Packet> map_size_packet; ///< Map size packet, fast tracked to the client
 	std::mutex mutex;                   ///< Mutex for making threaded saving safe.
 	std::condition_variable exit_sig;   ///< Signal for threaded destruction of this packet writer.
 
@@ -85,7 +85,7 @@ struct PacketWriter : SaveFilter {
 		/* This must all wait until the Destroy function is called. */
 
 		this->packets.clear();
-		this->prepend_packets.clear();
+		this->map_size_packet.reset();
 		this->current.reset();
 	}
 
@@ -125,8 +125,9 @@ struct PacketWriter : SaveFilter {
 	{
 		std::lock_guard<std::mutex> lock(this->mutex);
 
-		for (auto &p : this->prepend_packets) {
-			socket->SendPrependPacket(std::move(p));
+		if (this->map_size_packet) {
+			/* Don't queue the PACKET_SERVER_MAP_SIZE before the corresponding PACKET_SERVER_MAP_BEGIN */
+			socket->SendPrependPacket(std::move(this->map_size_packet), PACKET_SERVER_MAP_BEGIN);
 		}
 		bool last_packet = false;
 		for (auto &p : this->packets) {
@@ -134,7 +135,6 @@ struct PacketWriter : SaveFilter {
 			socket->SendPacket(std::move(p));
 
 		}
-		this->prepend_packets.clear();
 		this->packets.clear();
 
 		return last_packet;
@@ -148,14 +148,6 @@ struct PacketWriter : SaveFilter {
 		this->packets.push_back(std::move(this->current));
 	}
 
-	/** Prepend the current packet to the queue. */
-	void PrependQueue()
-	{
-		if (this->current == nullptr) return;
-
-		this->prepend_packets.push_back(std::move(this->current));
-	}
-
 	void Write(byte *buf, size_t size) override
 	{
 		/* We want to abort the saving when the socket is closed. */
@@ -194,9 +186,8 @@ struct PacketWriter : SaveFilter {
 		this->AppendQueue();
 
 		/* Fast-track the size to the client. */
-		this->current.reset(new Packet(PACKET_SERVER_MAP_SIZE, SHRT_MAX));
-		this->current->Send_uint32((uint32)this->total_size);
-		this->PrependQueue();
+		this->map_size_packet.reset(new Packet(PACKET_SERVER_MAP_SIZE, SHRT_MAX));
+		this->map_size_packet->Send_uint32((uint32)this->total_size);
 	}
 };