improve campaign file extension handling (scp-fs2open#6443)

As discovered by WouterSmits, campaigns with periods in their filenames caused issues with FSO's campaign handling.  Specifically, in The Babylon Project, the EA-Minbari war v1.0.fc2 campaign would not save progress because FSO expected EA-Minbari war v1.fc2 in certain places.  This is because the `_splitpath()` function attempted to remove an extension from a filename that already had it removed.  For most campaigns this did not matter, but WouterSmits found a campaign where it did.

To solve this, there is now a new `util::get_file_part()` function to remove the path without removing the extension.  This replaces `_splitpath()` for campaign files, as well as the similar `clean_filename()` and `GetFilePart()` functions in other places.  All code paths were inspected for any chance of inadvertent extension adding, and Assertions were added to verify future code.  The only potential problem area was `player:loadCampaignSavefile()` which now pre-removes the campaign extension if needed.

code/exceptionhandler/exceptionhandler.cpp

@@ -291,17 +291,6 @@ static const char *GetExceptionDescription(DWORD ExceptionCode)
 	return "Unknown exception type";
 }
 
-static char* GetFilePart(char *source)
-{
-	char *result = strrchr(source, '\\');
-	if (result) {
-		result++;
-	} else {
-		result = source;
-	}
-	return result;
-}
-
 // --------------------
 //
 // External Functions
@@ -338,7 +327,7 @@ int __cdecl RecordExceptionInfo(PEXCEPTION_POINTERS data, const char *Message)
 		ModuleName[0] = 0;
 	}
 
-	char *FilePart = GetFilePart(ModuleName);
+	char *FilePart = util::get_file_part(ModuleName);
 
 	// Extract the file name portion and remove it's file extension. We'll
 	// use that name shortly.
@@ -371,7 +360,7 @@ int __cdecl RecordExceptionInfo(PEXCEPTION_POINTERS data, const char *Message)
 	// code address, which is the same as the ModuleHandle. This can be used
 	// to get the filename of the module that the crash happened in.
 	if (VirtualQuery((void*)Context->Eip, &MemInfo, sizeof(MemInfo)) && GetModuleFileName((HINSTANCE)MemInfo.AllocationBase, CrashModulePathName, sizeof(CrashModulePathName)) > 0) {
-		CrashModuleFileName = GetFilePart(CrashModulePathName);
+		CrashModuleFileName = util::get_file_part(CrashModulePathName);
 	}
 
 	// Print out the beginning of the error log in a Win95 error window

code/globalincs/windebug.cpp

@@ -34,6 +34,7 @@
 #include "cmdline/cmdline.h"
 #include "parse/parselo.h"
 #include "debugconsole/console.h"
+#include "utils/string_utils.h"
 
 #if defined( SHOW_CALL_STACK ) && defined( PDB_DEBUGGING )
 #	include "globalincs/mspdb_callstack.h"
@@ -50,17 +51,6 @@ extern void gr_activate(int active);
   #endif
 #endif
 
-const char *clean_filename( const char *name)
-{
-	const char *p = name+strlen(name)-1;
-	// Move p to point to first letter of EXE filename
-	while( (*p!='\\') && (*p!='/') && (*p!=':') && (p>= name) )
-		p--;
-	p++;
-
-	return p;
-}
-
 
 /* MSVC2005+ callstack support
  */
@@ -87,7 +77,7 @@ class SCP_DebugCallStack : public SCP_IDumpHandler
 		UNREFERENCED_PARAMETER( address );
 
 		StackEntry entry;
-		entry.module = clean_filename( module );
+		entry.module = util::get_file_part( module );
 		entry.symbol = symbol;
 		m_stackFrames.push_back( entry );
 	}

code/menuui/readyroom.cpp

@@ -1575,12 +1575,10 @@ void campaign_room_scroll_info_down()
 
 void campaign_reset(const SCP_string& campaign_file)
 {
-	auto filename = campaign_file + FS_CAMPAIGN_FILE_EXT;
+	// note: we do not toss all-time stats from player's performance in campaign up till now
+	mission_campaign_savefile_delete(campaign_file.c_str());
 
-		// note: we do not toss all-time stats from player's performance in campaign up till now
-	mission_campaign_savefile_delete(filename.c_str());
-
-	const int load_status = mission_campaign_load(filename.c_str(), nullptr, nullptr, 1);
+	const int load_status = mission_campaign_load(campaign_file.c_str(), nullptr, nullptr, 1);
 
 	// see if we successfully loaded this campaign
 	if (load_status == 0) {

code/mission/missioncampaign.cpp

@@ -49,6 +49,7 @@
 #include "ship/ship.h"
 #include "starfield/supernova.h"
 #include "ui/ui.h"
+#include "utils/string_utils.h"
 #include "weapon/weapon.h"
 
 // campaign wasn't ended
@@ -740,14 +741,16 @@ void mission_campaign_init()
  */
 void mission_campaign_savefile_delete(const char* cfilename)
 {
-	char filename[_MAX_FNAME], base[_MAX_FNAME];
-
-	_splitpath( cfilename, NULL, NULL, base, NULL );
+	char filename[_MAX_FNAME];
 
 	if ( Player->flags & PLAYER_FLAGS_IS_MULTI ) {
 		return;	// no such thing as a multiplayer campaign savefile
 	}
 
+	auto base = util::get_file_part(cfilename);
+	// do a sanity check, but don't arbitrarily drop any extension in case the filename contains a period
+	Assertion(!stristr(base, FS_CAMPAIGN_FILE_EXT), "The campaign should not have an extension at this point!");
+
 	// only support the new filename here - taylor
 	sprintf_safe( filename, NOX("%s.%s.csg"), Player->callsign, base );
 

code/model/modelread.cpp

@@ -1113,10 +1113,9 @@ void do_new_subsystem( int n_subsystems, model_subsystem *slist, int subobj_num,
 		}
 	}
 #ifndef NDEBUG
-	char bname[_MAX_FNAME];
+	char bname[FILESPEC_LENGTH];
 
 	if ( !ss_warning_shown_mismatch) {
-		_splitpath(model_filename, NULL, NULL, bname, NULL);
 		// Lets still give a comment about it and not just erase it
 		Warning(LOCATION,"Not all subsystems in model \"%s\" have a record in ships.tbl.\nThis can cause game to crash.\n\nList of subsystems not found from table is in log file.\n", model_get(model_num)->filename );
 		mprintf(("Subsystem %s in model %s was not found in ships.tbl!\n", subobj_name, model_get(model_num)->filename));
@@ -1602,9 +1601,9 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename,
 	// code to get a filename to write out subsystem information for each model that
 	// is read.  This info is essentially debug stuff that is used to help get models
 	// into the game quicker
-#if 0
+#ifndef NDEBUG
 	{
-		char bname[_MAX_FNAME];
+		char bname[FILESPEC_LENGTH];
 
 		_splitpath(filename, NULL, NULL, bname, NULL);
 		sprintf(debug_name, "%s.subsystems", bname);

code/osapi/dialogs.cpp

@@ -5,6 +5,7 @@
 #include "cmdline/cmdline.h"
 #include "graphics/2d.h"
 #include "scripting/ade.h"
+#include "utils/string_utils.h"
 
 #include <SDL_messagebox.h>
 #include <SDL_clipboard.h>
@@ -65,17 +66,6 @@ namespace
 		return outStream.str();
 	}
 
-	const char* clean_filename(const char* filename)
-	{
-		auto separator = strrchr(filename, DIR_SEPARATOR_CHAR);
-		if (separator != nullptr)
-		{
-			filename = separator + 1;
-		}
-
-		return filename;
-	}
-
 	void set_clipboard_text(const char* text)
 	{
 		// Make sure video is enabled
@@ -106,7 +96,7 @@ namespace os
 		void AssertMessage(const char * text, const char * filename, int linenum, const char * format, ...)
 		{
 			// We only want to display the file name
-			filename = clean_filename(filename);
+			filename = util::get_file_part(filename);
 
 			SCP_stringstream msgStream;
 			msgStream << "Assert: \"" << text << "\"\n";
@@ -274,7 +264,7 @@ namespace os
 		void Error(const char * filename, int line, const char * format, ...)
 		{
 			SCP_string formatText;
-			filename = clean_filename(filename);
+			filename = util::get_file_part(filename);
 
 			va_list args;
 			va_start(args, format);
@@ -354,7 +344,7 @@ namespace os
 		// Actual implementation of the warning function. Used by the various warning functions
 		void WarningImpl(const char* filename, int line, const SCP_string& text)
 		{
-			filename = clean_filename(filename);
+			filename = util::get_file_part(filename);
 
 			// output to the debug log before anything else (so that we have a complete record)
 			mprintf(("WARNING: \"%s\" at %s:%d\n", text.c_str(), filename, line));
@@ -478,7 +468,7 @@ namespace os
 			va_end(args);
 
 			// Below is essentially a stripped down copy pasta of WarningImpl
-			filename = clean_filename(filename);
+			filename = util::get_file_part(filename);
 
 			// output to the debug log before anything else (so that we have a complete record)
 			mprintf(("INFO: \"%s\" at %s:%d\n", msg.c_str(), filename, line));

code/pilotfile/csg.cpp

@@ -19,6 +19,7 @@
 #include "ship/ship.h"
 #include "sound/audiostr.h"
 #include "stats/medals.h"
+#include "utils/string_utils.h"
 #include "weapon/weapon.h"
 
 #define REDALERT_INTERNAL
@@ -1595,8 +1596,7 @@ void pilotfile::csg_close()
 
 bool pilotfile::load_savefile(player *_p, const char *campaign)
 {
-	char base[_MAX_FNAME] = { '\0' };
-	std::ostringstream buf;
+	SCP_string campaign_filename;
 
 	if (Game_mode & GM_MULTIPLAYER) {
 		return false;
@@ -1610,18 +1610,18 @@ bool pilotfile::load_savefile(player *_p, const char *campaign)
 	Assert( (Player_num >= 0) && (Player_num < MAX_PLAYERS) );
 	p = _p;
 
-	// build up filename for the savefile...
-	_splitpath((char*)campaign, NULL, NULL, base, NULL);
-
-	buf << p->callsign << "." << base << ".csg";
+	auto base = util::get_file_part(campaign);
+	// do a sanity check, but don't arbitrarily drop any extension in case the filename contains a period
+	Assertion(!stristr(base, FS_CAMPAIGN_FILE_EXT), "The campaign should not have an extension at this point!");
 
-	filename = buf.str().c_str();
+	// build up filename for the savefile...
+	sprintf(filename, NOX("%s.%s.csg"), p->callsign, base);
 
 	// if campaign file doesn't exist, abort so we don't load irrelevant data
-	buf.str(std::string());
-	buf << base << FS_CAMPAIGN_FILE_EXT;
-	if ( !cf_exists_full((char*)buf.str().c_str(), CF_TYPE_MISSIONS) ) {
-		mprintf(("CSG => Unable to find campaign file '%s'!\n", buf.str().c_str()));
+	campaign_filename = base;
+	campaign_filename += FS_CAMPAIGN_FILE_EXT;
+	if ( !cf_exists_full(campaign_filename.c_str(), CF_TYPE_MISSIONS) ) {
+		mprintf(("CSG => Unable to find campaign file '%s'!\n", campaign_filename.c_str()));
 		return false;
 	}
 
@@ -1782,9 +1782,6 @@ bool pilotfile::load_savefile(player *_p, const char *campaign)
 
 bool pilotfile::save_savefile()
 {
-	char base[_MAX_FNAME] = { '\0' };
-	std::ostringstream buf;
-
 	if (Game_mode & GM_MULTIPLAYER) {
 		return false;
 	}
@@ -1797,12 +1794,12 @@ bool pilotfile::save_savefile()
 		return false;
 	}
 
-	// build up filename for the savefile...
-	_splitpath(Campaign.filename, NULL, NULL, base, NULL);
-
-	buf << p->callsign << "." << base << ".csg";
+	auto base = util::get_file_part(Campaign.filename);
+	// do a sanity check, but don't arbitrarily drop any extension in case the filename contains a period
+	Assertion(!stristr(base, FS_CAMPAIGN_FILE_EXT), "The campaign should not have an extension at this point!");
 
-	filename = buf.str().c_str();
+	// build up filename for the savefile...
+	sprintf(filename, NOX("%s.%s.csg"), p->callsign, base);
 
 	// make sure that we can actually save this safely
 	if (m_data_invalid) {

code/scripting/api/objs/player.cpp

@@ -15,6 +15,7 @@
 #include "scripting/api/objs/shipclass.h"
 #include "scripting/lua/LuaTable.h"
 #include "ship/ship.h"
+#include "freespace.h"
 
 namespace scripting {
 namespace api {
@@ -309,8 +310,20 @@ ADE_FUNC(loadCampaignSavefile, l_Player, "string campaign = \"<current>\"", "Loa
 		return ADE_RETURN_FALSE;
 	}
 
+	char temp_buffer[MAX_FILENAME_LEN + 1];	// see current_campaign field in player class
+
 	if (savefile == nullptr) {
 		savefile = plh->get()->current_campaign;
+	} else {
+		memset(temp_buffer, 0, sizeof(temp_buffer));
+		strncpy(temp_buffer, savefile, MAX_FILENAME_LEN);
+
+		// drop the extension if it exists
+		auto p = stristr(temp_buffer, FS_CAMPAIGN_FILE_EXT);
+		if (p) {
+			*p = '\0';
+			savefile = temp_buffer;
+		}
 	}
 
 	pilotfile loader;

code/utils/string_utils.h

@@ -18,4 +18,19 @@ void split_string(const SCP_string& s, char delim, Out result)
 
 std::vector<std::string> split_string(const std::string& s, char delim);
 
+// get a filename minus any leading path
+template <typename T>
+T *get_file_part(T *path)
+{
+	T *p = path + strlen(path)-1;
+
+	// Move p to point to first character of filename (check both types of dir separator)
+	while( (p >= path) && (*p != '\\') && (*p != '/') && (*p != ':') )
+		p--;
+
+	p++;
+
+	return p;
+}
+
 } // namespace util

code/windows_stub/stubs.cpp

@@ -141,20 +141,6 @@ SCP_string dump_stacktrace()
 #endif
 }
 
-// get a filename minus any leading path
-char *clean_filename(char *name)
-{
-	char *p = name + strlen(name)-1;
-
-	// Move p to point to first letter of EXE filename
-	while( (p > name) && (*p != '\\') && (*p != '/') && (*p != ':') )
-		p--;
-
-	p++;
-
-	return p;
-}
-
 // retrieve the current working directory
 int _getcwd(char *out_buf, unsigned int len)
 {