Legacy downloader requests have recieved a little TLC.

- Catch buffer overrun opportunities and fail early.
- Add #define MORELEGACYDOWNLOADER for the equivalent of MOREFILENEEDED, but disabled for now because honestly we really shouldn't be encouraging people to use this thing by making it support 255 WADs at once, but also because that'd be MISERABLE to test
- Add a menu report for when legacy downloader attempts fail
This commit is contained in:
toaster 2022-10-31 12:43:25 +00:00
parent fbf696a38a
commit 5ab988dc3e
2 changed files with 107 additions and 18 deletions

View file

@ -1130,6 +1130,7 @@ typedef enum
CL_PREPAREHTTPFILES, CL_PREPAREHTTPFILES,
CL_DOWNLOADHTTPFILES, CL_DOWNLOADHTTPFILES,
#endif #endif
CL_LEGACYREQUESTFAILED,
} cl_mode_t; } cl_mode_t;
static void GetPackets(void); static void GetPackets(void);
@ -1227,6 +1228,7 @@ static inline void CL_DrawConnectionStatus(void)
#endif #endif
case CL_ASKFULLFILELIST: case CL_ASKFULLFILELIST:
case CL_CONFIRMCONNECT: case CL_CONFIRMCONNECT:
case CL_LEGACYREQUESTFAILED:
cltext = ""; cltext = "";
break; break;
case CL_SETUPFILES: case CL_SETUPFILES:
@ -2124,6 +2126,10 @@ static void M_ConfirmConnect(event_t *ev)
{ {
cl_mode = CL_DOWNLOADFILES; cl_mode = CL_DOWNLOADFILES;
} }
else
{
cl_mode = CL_LEGACYREQUESTFAILED;
}
} }
#ifdef HAVE_CURL #ifdef HAVE_CURL
else else
@ -2279,6 +2285,10 @@ static boolean CL_FinishedFileList(void)
{ {
cl_mode = CL_DOWNLOADFILES; cl_mode = CL_DOWNLOADFILES;
} }
else
{
cl_mode = CL_LEGACYREQUESTFAILED;
}
} }
#endif #endif
} }
@ -2465,6 +2475,22 @@ static boolean CL_ServerConnectionTicker(const char *tmpsave, tic_t *oldtic, tic
cl_mode = CL_LOADFILES; cl_mode = CL_LOADFILES;
break; break;
case CL_LEGACYREQUESTFAILED:
{
CONS_Printf(M_GetText("Legacy downloader request packet failed.\n"));
CONS_Printf(M_GetText("Network game synchronization aborted.\n"));
D_QuitNetGame();
CL_Reset();
D_StartTitle();
M_StartMessage(M_GetText(
"The legacy file downloader could not handle that many files.\n"
"Ask the server host to set up a http source, or\n"
"locate and download the necessary files yourself.\n"
"\n"
"Press ESC\n"
), NULL, MM_NOTHING);
return false;
}
case CL_LOADFILES: case CL_LOADFILES:
if (CL_LoadServerFiles()) if (CL_LoadServerFiles())
cl_mode = CL_SETUPFILES; cl_mode = CL_SETUPFILES;

View file

@ -299,6 +299,9 @@ boolean CL_CheckDownloadable(void)
return false; return false;
} }
// The following was written and then quickly deemed too fragile on paper to be worth testing.
//#DEFINE MORELEGACYDOWNLOADER
/** Sends requests for files in the ::fileneeded table with a status of /** Sends requests for files in the ::fileneeded table with a status of
* ::FS_NOTFOUND. * ::FS_NOTFOUND.
* *
@ -311,42 +314,102 @@ boolean CL_SendRequestFile(void)
char *p; char *p;
INT32 i; INT32 i;
INT64 totalfreespaceneeded = 0, availablefreespace; INT64 totalfreespaceneeded = 0, availablefreespace;
INT32 skippedafile = -1;
#ifdef MORELEGACYDOWNLOADER
boolean firstloop = true;
#endif
#ifdef PARANOIA #ifdef PARANOIA
if (M_CheckParm("-nodownload")) if (M_CheckParm("-nodownload"))
I_Error("Attempted to download files in -nodownload mode"); I_Error("CL_SendRequestFile: Attempted to download files in -nodownload mode");
#endif
for (i = 0; i < fileneedednum; i++) for (i = 0; i < fileneedednum; i++)
{
#ifdef PARANOIA
if (fileneeded[i].status != FS_FOUND && fileneeded[i].status != FS_OPEN if (fileneeded[i].status != FS_FOUND && fileneeded[i].status != FS_OPEN
&& (fileneeded[i].willsend == 0 || fileneeded[i].willsend == 2)) && (fileneeded[i].willsend == 0 || fileneeded[i].willsend == 2))
{ {
I_Error("Attempted to download files that were not sendable"); I_Error("CL_SendRequestFile: Attempted to download files that were not sendable");
} }
#endif #endif
netbuffer->packettype = PT_REQUESTFILE;
p = (char *)netbuffer->u.textcmd;
for (i = 0; i < fileneedednum; i++)
if ((fileneeded[i].status == FS_NOTFOUND || fileneeded[i].status == FS_MD5SUMBAD || fileneeded[i].status == FS_FALLBACK)) if ((fileneeded[i].status == FS_NOTFOUND || fileneeded[i].status == FS_MD5SUMBAD || fileneeded[i].status == FS_FALLBACK))
{ {
// Error check for the first time around.
totalfreespaceneeded += fileneeded[i].totalsize; totalfreespaceneeded += fileneeded[i].totalsize;
nameonly(fileneeded[i].filename);
WRITEUINT8(p, i); // fileid
WRITESTRINGN(p, fileneeded[i].filename, MAX_WADPATH);
// put it in download dir
strcatbf(fileneeded[i].filename, downloaddir, "/");
fileneeded[i].status = FS_REQUESTED;
} }
WRITEUINT8(p, 0xFF); }
I_GetDiskFreeSpace(&availablefreespace); I_GetDiskFreeSpace(&availablefreespace);
if (totalfreespaceneeded > availablefreespace) if (totalfreespaceneeded > availablefreespace)
I_Error("To play on this server you must download %s KB,\n" I_Error("To play on this server you must download %s KB,\n"
"but you have only %s KB free space on this drive\n", "but you have only %s KB free space on this drive\n",
sizeu1((size_t)(totalfreespaceneeded>>10)), sizeu2((size_t)(availablefreespace>>10))); sizeu1((size_t)(totalfreespaceneeded>>10)), sizeu2((size_t)(availablefreespace>>10)));
// prepare to download #ifdef MORELEGACYDOWNLOADER
I_mkdir(downloaddir, 0755); tryagain:
return HSendPacket(servernode, true, 0, p - (char *)netbuffer->u.textcmd); skippedafile = -1;
#endif
netbuffer->packettype = PT_REQUESTFILE;
p = (char *)netbuffer->u.textcmd;
for (i = 0; i < fileneedednum; i++)
{
if ((fileneeded[i].status == FS_NOTFOUND || fileneeded[i].status == FS_MD5SUMBAD || fileneeded[i].status == FS_FALLBACK))
{
// Pre-prepare.
size_t checklen;
nameonly(fileneeded[i].filename);
// Figure out if we'd overrun our buffer.
checklen = strlen(fileneeded[i].filename)+2; // plus the fileid (and terminator, in case this is last)
if ((UINT8 *)(p + checklen) > netbuffer->u.textcmd + MAXTEXTCMD-1)
{
skippedafile = i;
// we might have a shorter file that can fit in the remaining space, and file ID permits out-of-order data
continue;
}
// Now write.
WRITEUINT8(p, i); // fileid
WRITESTRINGN(p, fileneeded[i].filename, MAX_WADPATH);
// put it in download dir
strcatbf(fileneeded[i].filename, downloaddir, "/");
fileneeded[i].status = FS_REQUESTED;
}
}
#ifdef MORELEGACYDOWNLOADER
if (firstloop)
#else
// If we're not trying extralong legacy download requests, gotta bail.
if (skippedafile != -1)
return false;
else
#endif
I_mkdir(downloaddir, 0755);
#ifdef PARANOIA
// Couldn't fit a single one in?
if (p == (char *)netbuffer->u.textcmd)
I_Error("CL_SendRequestFile: Fileneeded name for %s (fileneeded[%d]) too long??", (p > 0 ? fileneeded[p].filename : NULL), p);
#endif
WRITEUINT8(p, 0xFF); // terminator
if (!HSendPacket(servernode, true, 0, p - (char *)netbuffer->u.textcmd))
return false;
#ifdef MORELEGACYDOWNLOADER
if (skippedafile != -1)
{
firstloop = false;
goto tryagain;
}
#endif
return true;
} }
// get request filepak and put it on the send queue // get request filepak and put it on the send queue
@ -362,10 +425,10 @@ boolean Got_RequestFilePak(INT32 node)
if (id == 0xFF) if (id == 0xFF)
break; break;
READSTRINGN(p, wad, MAX_WADPATH); READSTRINGN(p, wad, MAX_WADPATH);
if (!SV_SendFile(node, wad, id)) if (p >= netbuffer->u.textcmd + MAXTEXTCMD-1 || !SV_SendFile(node, wad, id))
{ {
SV_AbortSendFiles(node); SV_AbortSendFiles(node);
return false; // don't read the rest of the files return false; // don't read any more
} }
} }
return true; // no problems with any files return true; // no problems with any files