From 3f734eb1b561bf8593190a14422e48517c4d4fd8 Mon Sep 17 00:00:00 2001 From: Maciej Suminski Date: Fri, 13 Apr 2018 11:26:28 +0200 Subject: [PATCH] Improved validation of library and entry names Symbol/footprint library and entry have the same set of forbidden characters with a single exception, space character. To accommodate for this difference, LIB_ID validation and fix methods have been extended to specify the LIB_ID type that is checked (schematic/board). LIB_ID::HasIllegalChars() and LIB_ID::FixIllegalChars() had two different sets of characters treated as invalid in LIB_IDs. The set has been factored out to another function to avoid duplication. --- common/lib_id.cpp | 75 ++++++++++++----------- eeschema/dialogs/dialog_sym_lib_table.cpp | 7 ++- eeschema/project_rescue.cpp | 8 +-- eeschema/sch_eagle_plugin.cpp | 7 ++- include/lib_id.h | 21 ++++++- pcbnew/dialogs/dialog_fp_lib_table.cpp | 8 ++- 6 files changed, 76 insertions(+), 50 deletions(-) diff --git a/common/lib_id.cpp b/common/lib_id.cpp index a2b64af92c..388f0cbe93 100644 --- a/common/lib_id.cpp +++ b/common/lib_id.cpp @@ -359,21 +359,11 @@ int LIB_ID::compare( const LIB_ID& aLibId ) const } -bool LIB_ID::HasIllegalChars( const UTF8& aLibItemName ) +bool LIB_ID::HasIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ) { for( auto ch : aLibItemName ) { - switch( ch ) - { - case '\t': - case '\n': - case '\r': - case ':': - case '/': - return true; - } - - if( !wxIsascii( ch ) ) + if( !isLegalChar( ch, aType ) ) return true; } @@ -381,41 +371,56 @@ bool LIB_ID::HasIllegalChars( const UTF8& aLibItemName ) } -UTF8 LIB_ID::FixIllegalChars( const UTF8& aLibItemName ) +UTF8 LIB_ID::FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ) { UTF8 fixedName; for( UTF8::uni_iter chIt = aLibItemName.ubegin(); chIt < aLibItemName.uend(); ++chIt ) { auto ch = *chIt; - - if( !wxIsascii( ch ) ) - { - fixedName += '_'; - } - else - { - switch( ch ) - { - case '\t': - case '\n': - case '\r': - case ':': - case '/': - case '\\': - fixedName += '_'; - break; - - default: - fixedName += ch; - } - } + fixedName += isLegalChar( ch, aType ) ? ch : '_'; } return fixedName; } +unsigned LIB_ID::FindIllegalChar( const UTF8& aNickname, LIB_ID_TYPE aType ) +{ + for( unsigned ch : aNickname ) + { + if( !isLegalChar( ch, aType ) ) + return ch; + } + + return 0; +} + + +///> Set of characters not accepted in library and entry names +#define BASE_ILLEGAL_CHARS '\t', '\n', '\r', ':', '/', '\\' +const unsigned schIllegalChars[] = { BASE_ILLEGAL_CHARS, ' ' }; +const unsigned pcbIllegalChars[] = { BASE_ILLEGAL_CHARS, 0 }; +#define ILL_CHAR_SIZE (sizeof(schIllegalChars) / sizeof(int)) + +bool LIB_ID::isLegalChar( unsigned aChar, LIB_ID_TYPE aType ) +{ + const unsigned (&illegalChars)[ILL_CHAR_SIZE] = + aType == ID_SCH ? schIllegalChars : pcbIllegalChars; + + for( const unsigned ch : illegalChars ) + { + if( ch == aChar ) + return false; + } + + if( !wxIsascii( aChar ) ) + return false; + + return true; +} + + #if 0 && defined(DEBUG) // build this with Debug CMAKE_BUILD_TYPE diff --git a/eeschema/dialogs/dialog_sym_lib_table.cpp b/eeschema/dialogs/dialog_sym_lib_table.cpp index 6e2b0509e4..1f735ae98b 100644 --- a/eeschema/dialogs/dialog_sym_lib_table.cpp +++ b/eeschema/dialogs/dialog_sym_lib_table.cpp @@ -257,6 +257,7 @@ bool DIALOG_SYMBOL_LIB_TABLE::verifyTables() { wxString nick = model.GetValue( r, COL_NICKNAME ).Trim( false ).Trim(); wxString uri = model.GetValue( r, COL_URI ).Trim( false ).Trim(); + unsigned illegalCh = 0; if( !nick || !uri ) { @@ -266,11 +267,11 @@ bool DIALOG_SYMBOL_LIB_TABLE::verifyTables() // button. model.DeleteRows( r, 1 ); } - else if( nick.find( ':' ) != size_t( -1 ) || nick.find( ' ' ) != size_t( -1 ) ) + else if( ( illegalCh = LIB_ID::FindIllegalChar( nick, LIB_ID::ID_SCH ) ) ) { wxString msg = wxString::Format( - _( "Illegal character \"%s\" found in Nickname: \"%s\" in row %d" ), - ( nick.find( ':' ) != size_t( -1 ) ) ? ":" : " ", GetChars( nick ), r + 1 ); + _( "Illegal character \"%c\" found in Nickname: \"%s\" in row %d" ), + illegalCh, GetChars( nick ), r + 1 ); // show the tabbed panel holding the grid we have flunked: if( &model != cur_model() ) diff --git a/eeschema/project_rescue.cpp b/eeschema/project_rescue.cpp index b931d07caa..5bf514a734 100644 --- a/eeschema/project_rescue.cpp +++ b/eeschema/project_rescue.cpp @@ -260,11 +260,11 @@ void RESCUE_CACHE_CANDIDATE::FindRescues( RESCUER& aRescuer, // and the symbol name does not have any illegal characters. if( ( ( cache_match && lib_match && !cache_match->PinsConflictWith( *lib_match, true, true, true, true, false ) ) - || (!cache_match && lib_match ) ) && !LIB_ID::HasIllegalChars( part_name ) ) + || (!cache_match && lib_match ) ) && !LIB_ID::HasIllegalChars( part_name, LIB_ID::ID_SCH ) ) continue; // Check if the symbol has already been rescued. - wxString new_name = LIB_ID::FixIllegalChars( part_name ); + wxString new_name = LIB_ID::FixIllegalChars( part_name, LIB_ID::ID_SCH ); RESCUE_CACHE_CANDIDATE candidate( part_name, new_name, cache_match, lib_match ); @@ -382,11 +382,11 @@ void RESCUE_SYMBOL_LIB_TABLE_CANDIDATE::FindRescues( if( ( ( cache_match && lib_match && !cache_match->PinsConflictWith( *lib_match, true, true, true, true, false ) ) || (!cache_match && lib_match ) ) - && !LIB_ID::HasIllegalChars( part_id.GetLibItemName() ) ) + && !LIB_ID::HasIllegalChars( part_id.GetLibItemName(), LIB_ID::ID_SCH ) ) continue; // Fix illegal LIB_ID name characters. - wxString new_name = LIB_ID::FixIllegalChars( part_id.GetLibItemName() ); + wxString new_name = LIB_ID::FixIllegalChars( part_id.GetLibItemName(), LIB_ID::ID_SCH ); // Differentiate symbol name in the resue library by appending the symbol library // table nickname to the symbol name to prevent name clashes in the rescue library. diff --git a/eeschema/sch_eagle_plugin.cpp b/eeschema/sch_eagle_plugin.cpp index f0cbd7b589..70be0c79a7 100644 --- a/eeschema/sch_eagle_plugin.cpp +++ b/eeschema/sch_eagle_plugin.cpp @@ -131,7 +131,8 @@ wxString SCH_EAGLE_PLUGIN::getLibName() m_libName = "noname"; m_libName += "-eagle-import"; - m_libName = LIB_ID::FixIllegalChars( m_libName ); + // use ID_SCH as it is more restrictive + m_libName = LIB_ID::FixIllegalChars( m_libName, LIB_ID::ID_SCH ); } return m_libName; @@ -1073,7 +1074,7 @@ void SCH_EAGLE_PLUGIN::loadInstance( wxXmlNode* aInstanceNode ) package = p->second; } - wxString kisymbolname = LIB_ID::FixIllegalChars( symbolname ); + wxString kisymbolname = LIB_ID::FixIllegalChars( symbolname, LIB_ID::ID_SCH ); LIB_ALIAS* alias = m_pi->LoadSymbol( getLibFileName().GetFullPath(), kisymbolname, m_properties.get() ); @@ -1296,7 +1297,7 @@ EAGLE_LIBRARY* SCH_EAGLE_PLUGIN::loadLibrary( wxXmlNode* aLibraryNode, if( gates_count == 1 && ispower ) kpart->SetPower(); - wxString name = LIB_ID::FixIllegalChars( kpart->GetName() ); + wxString name = LIB_ID::FixIllegalChars( kpart->GetName(), LIB_ID::ID_SCH ); kpart->SetName( name ); m_pi->SaveSymbol( getLibFileName().GetFullPath(), new LIB_PART( *kpart.get() ), m_properties.get() ); diff --git a/include/lib_id.h b/include/lib_id.h index 66d86b3208..c7e5a49e07 100644 --- a/include/lib_id.h +++ b/include/lib_id.h @@ -68,6 +68,9 @@ public: LIB_ID( const wxString& aId ); + ///> Types of library identifiers + enum LIB_ID_TYPE { ID_SCH, ID_PCB }; + /** * This LIB_ID ctor is a special version which ignores the parsing due to symbol * names allowing '/' as a valid character. This was causing the symbol names to @@ -188,23 +191,37 @@ public: * Examine \a aLibItemName for invalid #LIB_ID item name characters. * * @param aLibItemName is the #LIB_ID name to test for illegal characters. + * @param aType is the library identifier type * @return true if \a aLibItemName contain illegal characters otherwise false. */ - static bool HasIllegalChars( const UTF8& aLibItemName ); + static bool HasIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ); /** * Replace illegal #LIB_ID item name characters with underscores '_'. * * @param aLibItemName is the #LIB_ID item name to replace illegal characters. + * @param aType is the library identifier type * @return the corrected version of \a aLibItemName. */ - static UTF8 FixIllegalChars( const UTF8& aLibItemName ); + static UTF8 FixIllegalChars( const UTF8& aLibItemName, LIB_ID_TYPE aType ); + + /** + * Looks for characters that are illegal in library and item names. + * + * @param aNickname is the logical library name to be tested. + * @param aType is the library identifier type + * @return Invalid character found in the name or 0 is the name is valid. + */ + static unsigned FindIllegalChar( const UTF8& aNickname, LIB_ID_TYPE aType ); #if defined(DEBUG) static void Test(); #endif protected: + ///> Tests whether a character is a legal LIB_ID character + static bool isLegalChar( unsigned aChar, LIB_ID_TYPE aType ); + UTF8 nickname; ///< The nickname of the library or empty. UTF8 item_name; ///< The name of the entry in the logical library. UTF8 revision; ///< The revision of the entry. diff --git a/pcbnew/dialogs/dialog_fp_lib_table.cpp b/pcbnew/dialogs/dialog_fp_lib_table.cpp index 7790b46df7..5cd45ab306 100644 --- a/pcbnew/dialogs/dialog_fp_lib_table.cpp +++ b/pcbnew/dialogs/dialog_fp_lib_table.cpp @@ -38,6 +38,7 @@ #include #include <3d_viewer.h> // for KISYS3DMOD #include +#include #include #include #include @@ -290,6 +291,7 @@ private: { wxString nick = model.GetValue( r, COL_NICKNAME ).Trim( false ).Trim(); wxString uri = model.GetValue( r, COL_URI ).Trim( false ).Trim(); + unsigned illegalCh = 0; if( !nick || !uri ) { @@ -299,11 +301,11 @@ private: // button. model.DeleteRows( r, 1 ); } - else if( nick.find( ':' ) != size_t( -1 ) ) + else if( ( illegalCh = LIB_ID::FindIllegalChar( nick, LIB_ID::ID_PCB ) ) ) { wxString msg = wxString::Format( - _( "Illegal character \"%s\" found in Nickname: \"%s\" in row %d" ), - ":", GetChars( nick ), r ); + _( "Illegal character \"%c\" found in Nickname: \"%s\" in row %d" ), + illegalCh, GetChars( nick ), r ); // show the tabbed panel holding the grid we have flunked: if( &model != cur_model() )