From 82ff2c0e0fb5c8dd85f98dd9637865572dc5bd33 Mon Sep 17 00:00:00 2001
From: jean-pierre charras <jp.charras@wanadoo.fr>
Date: Fri, 25 Oct 2024 11:29:09 +0200
Subject: [PATCH] eeschema, DIALOG_SHEET_PROPERTIES: add test for valid sheet
 filename

Fixes https://gitlab.com/kicad/code/kicad/-/issues/18981
---
 common/string_utils.cpp                      | 28 ++++++++++++++++++++
 eeschema/dialogs/dialog_sheet_properties.cpp |  9 +++++++
 include/string_utils.h                       |  9 ++++++-
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/common/string_utils.cpp b/common/string_utils.cpp
index a45188cfe7..c70680c7b3 100644
--- a/common/string_utils.cpp
+++ b/common/string_utils.cpp
@@ -45,6 +45,34 @@
 static const char illegalFileNameChars[] = "\\/:\"<>|*?";
 
 
+// Checks if a full filename is valid, i.e. does not contains illegal chars
+bool IsFullFileNameValid( const wxString& aFullFilename )
+{
+
+    // Test for forbidden chars in aFullFilename.
+    // '\'and '/' are allowed here because aFullFilename can be a full path, and
+    // ':' is allowed on Windows as second char in string.
+    // So remove allowed separators from string to test
+    wxString filtered_fullpath = aFullFilename;
+
+#ifdef __WINDOWS__
+    // On MSW, the list returned by wxFileName::GetForbiddenChars() contains separators
+    // '\'and '/'
+    filtered_fullpath.Replace( "/", "_" );
+    filtered_fullpath.Replace( "\\", "_" );
+
+    // A disk identifier is allowed, and therefore remove its separator
+    if( filtered_fullpath.Length() > 1 && filtered_fullpath[1] == ':' )
+        filtered_fullpath[1] = ' ';
+#endif
+
+    if( wxString::npos != filtered_fullpath.find_first_of( wxFileName::GetForbiddenChars() ) )
+        return false;
+
+    return true;
+}
+
+
 wxString ConvertToNewOverbarNotation( const wxString& aOldStr )
 {
     wxString newStr;
diff --git a/eeschema/dialogs/dialog_sheet_properties.cpp b/eeschema/dialogs/dialog_sheet_properties.cpp
index f3d9ed08ab..70db6e40ec 100644
--- a/eeschema/dialogs/dialog_sheet_properties.cpp
+++ b/eeschema/dialogs/dialog_sheet_properties.cpp
@@ -47,6 +47,7 @@
 #include <trace_helpers.h>
 #include "panel_eeschema_color_settings.h"
 #include "wx/dcclient.h"
+#include "string_utils.h"
 
 DIALOG_SHEET_PROPERTIES::DIALOG_SHEET_PROPERTIES( SCH_EDIT_FRAME* aParent, SCH_SHEET* aSheet,
                                                   bool* aIsUndoable, bool* aClearAnnotationNewItems,
@@ -280,7 +281,15 @@ bool DIALOG_SHEET_PROPERTIES::TransferDataFromWindow()
     // but unedited data from existing files can be bad.)
     sheetFileName = EnsureFileExtension( sheetFileName, FILEEXT::KiCadSchematicFileExtension );
 
+    // Ensure sheetFileName is legal
+    if( !IsFullFileNameValid( sheetFileName ) )
+    {
+        DisplayError( this, _( "A sheet must have a valid file name." ) );
+        return false;
+    }
+
     wxFileName fn( sheetFileName );
+
     wxString newRelativeFilename = fn.GetFullPath();
 
     // Inside Eeschema, filenames are stored using unix notation
diff --git a/include/string_utils.h b/include/string_utils.h
index d3220dbc9a..8dac917ecb 100644
--- a/include/string_utils.h
+++ b/include/string_utils.h
@@ -231,6 +231,13 @@ KICOMMON_API int GetTrailingInt( const wxString& aStr );
  */
 KICOMMON_API wxString GetIllegalFileNameWxChars();
 
+/**
+ * Checks if a full filename is valid, i.e. does not contains illegal chars
+ * path separators are allowed
+ * @return true if OK.
+ */
+KICOMMON_API bool IsFullFileNameValid( const wxString& aFullFilename );
+
 /**
  * Checks \a aName for illegal file name characters.
  *
@@ -392,7 +399,7 @@ KICOMMON_API std::string FormatDouble2Str( double aValue );
 
 /**
  * Convert an expected UTF8 encoded std::string to a wxString.
- * If fails, tray to convert using current locale
+ * If fails, try to convert using current locale
  * If still fails, return the initial string (can be already a converted string)
  */
 KICOMMON_API wxString From_UTF8( const std::string& aString );