Merge branch 'netcode-hotfix' into 'master'

Netcode hotfix

Various netcode fixes are included in this branch. Most important of all of course is a fix for the Net_CloseConnection invalid node -32769 detected message thing, or rather what actually caused that to occur.

It's a merge to master since everything here should be compatible with 2.1.18: the only main changes are related to recieving packets (whether you're the host of a netgame or a client), which AFAIK shouldn't cause desyncs if you played with these fixes on a 2.1.18-hosted netgame (or hosted a netgame with the fixes and 2.1.18-using players join you).

See merge request !185

src/d_clisrv.c

@@ -3402,17 +3402,38 @@ static void HandlePacketFromAwayNode(SINT8 node)
 	if (node != servernode)
 		DEBFILE(va("Received packet from unknown host %d\n", node));
 
+// macro for packets that should only be sent by the server
+// if it is NOT from the server, bail out and close the connection!
+#define SERVERONLY \
+			if (node != servernode) \
+			{ \
+				Net_CloseConnection(node); \
+				break; \
+			}
 	switch (netbuffer->packettype)
 	{
 		case PT_ASKINFOVIAMS:
 			if (server && serverrunning)
 			{
-				INT32 clientnode = I_NetMakeNode(netbuffer->u.msaskinfo.clientaddr);
-				SV_SendServerInfo(clientnode, (tic_t)LONG(netbuffer->u.msaskinfo.time));
-				SV_SendPlayerInfo(clientnode); // Send extra info
-				Net_CloseConnection(clientnode);
-				// Don't close connection to MS.
+				INT32 clientnode;
+				if (ms_RoomId < 0) // ignore if we're not actually on the MS right now
+				{
+					Net_CloseConnection(node); // and yes, close connection
+					return;
+				}
+				clientnode = I_NetMakeNode(netbuffer->u.msaskinfo.clientaddr);
+				if (clientnode != -1)
+				{
+					SV_SendServerInfo(clientnode, (tic_t)LONG(netbuffer->u.msaskinfo.time));
+					SV_SendPlayerInfo(clientnode); // Send extra info
+					Net_CloseConnection(clientnode);
+					// Don't close connection to MS...
+				}
+				else
+					Net_CloseConnection(node); // ...unless the IP address is not valid
 			}
+			else
+				Net_CloseConnection(node); // you're not supposed to get it, so ignore it
 			break;
 
 		case PT_ASKINFO:
@@ -3420,8 +3441,8 @@ static void HandlePacketFromAwayNode(SINT8 node)
 			{
 				SV_SendServerInfo(node, (tic_t)LONG(netbuffer->u.askinfo.time));
 				SV_SendPlayerInfo(node); // Send extra info
-				Net_CloseConnection(node);
 			}
+			Net_CloseConnection(node);
 			break;
 
 		case PT_SERVERREFUSE: // Negative response of client join request
@@ -3430,6 +3451,7 @@ static void HandlePacketFromAwayNode(SINT8 node)
 				Net_CloseConnection(node);
 				break;
 			}
+			SERVERONLY
 			if (cl_mode == CL_WAITJOINRESPONSE)
 			{
 				// Save the reason so it can be displayed after quitting the netgame
@@ -3461,6 +3483,7 @@ static void HandlePacketFromAwayNode(SINT8 node)
 				Net_CloseConnection(node);
 				break;
 			}
+			SERVERONLY
 			/// \note how would this happen? and is it doing the right thing if it does?
 			if (cl_mode != CL_WAITJOINRESPONSE)
 				break;
@@ -3524,13 +3547,18 @@ static void HandlePacketFromAwayNode(SINT8 node)
 				Net_CloseConnection(node);
 				break;
 			}
-			else
-				Got_Filetxpak();
+			SERVERONLY
+			Got_Filetxpak();
 			break;
 
 		case PT_REQUESTFILE:
 			if (server)
-				Got_RequestFilePak(node);
+			{
+				if (!cv_downloading.value || !Got_RequestFilePak(node))
+					Net_CloseConnection(node); // close connection if one of the requested files could not be sent, or you disabled downloading anyway
+			}
+			else
+				Net_CloseConnection(node); // nope
 			break;
 
 		case PT_NODETIMEOUT:
@@ -3553,6 +3581,7 @@ static void HandlePacketFromAwayNode(SINT8 node)
 			break; // Ignore it
 
 	}
+#undef SERVERONLY
 }
 
 /** Handles a packet received from a node that is in game
@@ -3585,6 +3614,8 @@ FILESTAMP
 	{
 // -------------------------------------------- SERVER RECEIVE ----------
 		case PT_RESYNCHGET:
+			if (client)
+				break;
 			SV_AcknowledgeResynchAck(netconsole, netbuffer->u.resynchgot);
 			break;
 		case PT_CLIENTCMD:
@@ -3651,7 +3682,8 @@ FILESTAMP
 			}
 
 			// Splitscreen cmd
-			if (netbuffer->packettype == PT_CLIENT2CMD && nodetoplayer2[node] >= 0)
+			if ((netbuffer->packettype == PT_CLIENT2CMD || netbuffer->packettype == PT_CLIENT2MIS)
+				&& nodetoplayer2[node] >= 0)
 				G_MoveTiccmd(&netcmds[maketic%BACKUPTICS][(UINT8)nodetoplayer2[node]],
 					&netbuffer->u.client2pak.cmd2, 1);
 
@@ -3710,6 +3742,27 @@ FILESTAMP
 				tic_t tic = maketic;
 				UINT8 *textcmd;
 
+				// ignore if the textcmd has a reported size of zero
+				// this shouldn't be sent at all
+				if (!netbuffer->u.textcmd[0])
+				{
+					DEBFILE(va("GetPacket: Textcmd with size 0 detected! (node %u, player %d)\n",
+						node, netconsole));
+					Net_UnAcknowledgePacket(node);
+					break;
+				}
+
+				// ignore if the textcmd size var is actually larger than it should be
+				// BASEPACKETSIZE + 1 (for size) + textcmd[0] should == datalength
+				if (netbuffer->u.textcmd[0] > (size_t)doomcom->datalength-BASEPACKETSIZE-1)
+				{
+					DEBFILE(va("GetPacket: Bad Textcmd packet size! (expected %d, actual %s, node %u, player %d)\n",
+					netbuffer->u.textcmd[0], sizeu1((size_t)doomcom->datalength-BASEPACKETSIZE-1),
+						node, netconsole));
+					Net_UnAcknowledgePacket(node);
+					break;
+				}
+
 				// check if tic that we are making isn't too large else we cannot send it :(
 				// doomcom->numslots+1 "+1" since doomcom->numslots can change within this time and sent time
 				j = software_MAXPACKETLENGTH
@@ -3913,6 +3966,21 @@ FILESTAMP
 		case PT_SERVERCFG:
 			break;
 		case PT_FILEFRAGMENT:
+			// Only accept PT_FILEFRAGMENT from the server.
+			if (node != servernode)
+			{
+				CONS_Alert(CONS_WARNING, M_GetText("%s received from non-host %d\n"), "PT_FILEFRAGMENT", node);
+
+				if (server)
+				{
+					XBOXSTATIC UINT8 buf[2];
+					buf[0] = (UINT8)node;
+					buf[1] = KICK_MSG_CON_FAIL;
+					SendNetXCmd(XD_KICK, &buf, 2);
+				}
+
+				break;
+			}
 			if (client)
 				Got_Filetxpak();
 			break;

src/d_net.c

@@ -711,6 +711,13 @@ void Net_CloseConnection(INT32 node)
 #else
 	INT32 i;
 	boolean forceclose = (node & FORCECLOSE) != 0;
+
+	if (node == -1)
+	{
+		DEBFILE(M_GetText("Net_CloseConnection: node -1 detected!\n"));
+		return; // nope, just ignore it
+	}
+
 	node &= ~FORCECLOSE;
 
 	if (!node)
@@ -718,7 +725,7 @@ void Net_CloseConnection(INT32 node)
 
 	if (node < 0 || node >= MAXNETNODES) // prevent invalid nodes from crashing the game
 	{
-		CONS_Alert(CONS_WARNING, M_GetText("Net_CloseConnection: invalid node %d detected!\n"), node);
+		DEBFILE(va(M_GetText("Net_CloseConnection: invalid node %d detected!\n"), node));
 		return;
 	}
 

src/d_netfil.c

@@ -62,7 +62,8 @@
 
 #include <errno.h>
 
-static void SV_SendFile(INT32 node, const char *filename, UINT8 fileid);
+// Prototypes
+static boolean SV_SendFile(INT32 node, const char *filename, UINT8 fileid);
 
 // Sender structure
 typedef struct filetx_s
@@ -303,7 +304,8 @@ boolean CL_SendRequestFile(void)
 }
 
 // get request filepak and put it on the send queue
-void Got_RequestFilePak(INT32 node)
+// returns false if a requested file was not found or cannot be sent
+boolean Got_RequestFilePak(INT32 node)
 {
 	char wad[MAX_WADPATH+1];
 	UINT8 *p = netbuffer->u.textcmd;
@@ -314,8 +316,13 @@ void Got_RequestFilePak(INT32 node)
 		if (id == 0xFF)
 			break;
 		READSTRINGN(p, wad, MAX_WADPATH);
-		SV_SendFile(node, wad, id);
+		if (!SV_SendFile(node, wad, id))
+		{
+			SV_AbortSendFiles(node);
+			return false; // don't read the rest of the files
+		}
 	}
+	return true; // no problems with any files
 }
 
 /** Checks if the files needed aren't already loaded or on the disk
@@ -480,7 +487,7 @@ static INT32 filestosend = 0;
   * \sa SV_SendRam
   *
   */
-static void SV_SendFile(INT32 node, const char *filename, UINT8 fileid)
+static boolean SV_SendFile(INT32 node, const char *filename, UINT8 fileid)
 {
 	filetx_t **q; // A pointer to the "next" field of the last file in the list
 	filetx_t *p; // The new file request
@@ -488,7 +495,7 @@ static void SV_SendFile(INT32 node, const char *filename, UINT8 fileid)
 	char wadfilename[MAX_WADPATH];
 
 	if (cv_noticedownload.value)
-		CONS_Printf("Sending file \"%s\" to node %d\n", filename, node);
+		CONS_Printf("Sending file \"%s\" to node %d (%s)\n", filename, node, I_GetNodeAddress(node));
 
 	// Find the last file in the list and set a pointer to its "next" field
 	q = &transfer[node].txlist;
@@ -537,7 +544,7 @@ static void SV_SendFile(INT32 node, const char *filename, UINT8 fileid)
 		free(p->id.filename);
 		free(p);
 		*q = NULL;
-		return;
+		return false; // cancel the rest of the requests
 	}
 
 	// Handle huge file requests (i.e. bigger than cv_maxsend.value KB)
@@ -549,7 +556,7 @@ static void SV_SendFile(INT32 node, const char *filename, UINT8 fileid)
 		free(p->id.filename);
 		free(p);
 		*q = NULL;
-		return;
+		return false; // cancel the rest of the requests
 	}
 
 	DEBFILE(va("Sending file %s (id=%d) to %d\n", filename, fileid, node));
@@ -557,6 +564,7 @@ static void SV_SendFile(INT32 node, const char *filename, UINT8 fileid)
 	p->fileid = fileid;
 	p->next = NULL; // End of list
 	filestosend++;
+	return true;
 }
 
 /** Adds a memory block to the file list for a node

src/d_netfil.h

@@ -69,7 +69,7 @@ boolean SV_SendingFile(INT32 node);
 
 boolean CL_CheckDownloadable(void);
 boolean CL_SendRequestFile(void);
-void Got_RequestFilePak(INT32 node);
+boolean Got_RequestFilePak(INT32 node);
 
 void SV_AbortSendFiles(INT32 node);
 void CloseNetFile(void);