Eeschema: fix corrupted schematic and symbol library files.

Prevent user from pasting text with carriage return and/or line feed
characters in field value edit control in the schematic symbol and symbol
library properties dialogs.

Make all objects derived from GRID_CELL_TEXT_BUTTON use validators
correctly.

Add validators to FIELDS_GRID_TABLE object for field values other than
the mandatory fields.

Create a validator to check that a string is a valid LIB_ID.

Fixes lp:1828214

https://bugs.launchpad.net/kicad/+bug/1828214

common/validators.cpp

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2013 Wayne Stambaugh <stambaughw@verizon.net>
- * Copyright (C) 2004-2013 KiCad Developers, see change_log.txt for contributors.
+ * Copyright (C) 2004-2019 KiCad Developers, see change_log.txt for contributors.
  * Copyright (C) 2018 CERN
  *
  * This program is free software; you can redistribute it and/or
@@ -162,6 +162,7 @@ void ENV_VAR_NAME_VALIDATOR::OnChar( wxKeyEvent& aEvent  )
         // not as first character
         long from, to;
         GetTextEntry()->GetSelection( &from, &to );
+
         if( from < 1 )
             wxBell();
         else
@@ -273,6 +274,50 @@ void REGEX_VALIDATOR::compileRegEx( const wxString& aRegEx, int aFlags )
 }
 
 
+bool LIB_ID_VALIDATOR::Validate( wxWindow *aParent )
+{
+    LIB_ID dummy;
+
+    // If window is disabled, simply return
+    if( !m_validatorWindow->IsEnabled() )
+        return true;
+
+    wxTextEntry* const text = GetTextEntry();
+
+    if( !text )
+        return false;
+
+    wxString msg;
+    wxString val( text->GetValue() );
+    wxString tmp = val.Clone();          // For trailing and leading white space tests.
+
+    if( tmp.Trim() != val )              // Trailing white space.
+    {
+        msg = _( "Entry contains trailing white space." );
+    }
+    else if( tmp.Trim( false ) != val )  // Leading white space.
+    {
+        msg = _( "Entry contains leading white space." );
+    }
+    else if( dummy.Parse( val, m_idType ) != -1 || !dummy.IsValid() )   // Is valid LIB_ID.
+    {
+        msg.Printf( _( "\"%s\" is not a valid library identifier format." ), val );
+    }
+
+    if( !msg.empty() )
+    {
+        m_validatorWindow->SetFocus();
+
+        wxMessageBox( msg, _( "Library Identifier Validation Error" ),
+                      wxOK | wxICON_EXCLAMATION, aParent );
+
+        return false;
+    }
+
+    return true;
+}
+
+
 void KIUI::ValidatorTransferToWindowWithoutEvents( wxValidator& aValidator )
 {
     wxWindow* ctrl = aValidator.GetWindow();

common/widgets/grid_text_button_helpers.cpp

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2018-2019 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -47,6 +47,7 @@ wxString GRID_CELL_TEXT_BUTTON::GetValue() const
     return Combo()->GetValue();
 }
 
+
 void GRID_CELL_TEXT_BUTTON::SetSize( const wxRect& aRect )
 {
     wxRect rect( aRect );
@@ -76,6 +77,7 @@ void GRID_CELL_TEXT_BUTTON::StartingKey( wxKeyEvent& event )
 
 #if wxUSE_UNICODE
     ch = event.GetUnicodeKey();
+
     if( ch != WXK_NONE )
         isPrintable = true;
     else
@@ -150,9 +152,13 @@ void GRID_CELL_TEXT_BUTTON::Reset()
 }
 
 
-/**
+#if wxUSE_VALIDATORS
- * Symbol Picker
+void GRID_CELL_TEXT_BUTTON::SetValidator( const wxValidator& validator )
- */
+{
+    m_validator.reset( static_cast< wxValidator* >( validator.Clone() ) );
+}
+#endif
+
 
 class TEXT_BUTTON_SYMBOL_CHOOSER : public wxComboCtrl
 {
@@ -202,10 +208,6 @@ void GRID_CELL_SYMBOL_ID_EDITOR::Create( wxWindow* aParent, wxWindowID aId,
 }
 
 
-/**
- * Footprint Picker
- */
-
 class TEXT_BUTTON_FP_CHOOSER : public wxComboCtrl
 {
 public:
@@ -250,14 +252,18 @@ void GRID_CELL_FOOTPRINT_ID_EDITOR::Create( wxWindow* aParent, wxWindowID aId,
 {
     m_control = new TEXT_BUTTON_FP_CHOOSER( aParent, m_dlg, m_preselect );
 
+#if wxUSE_VALIDATORS
+    // validate text in textctrl, if validator is set
+    if ( m_validator )
+    {
+        Combo()->SetValidator( *m_validator );
+    }
+#endif
+
     wxGridCellEditor::Create( aParent, aId, aEventHandler );
 }
 
 
-/**
- * URL Viewer
- */
-
 class TEXT_BUTTON_URL : public wxComboCtrl
 {
 public:
@@ -291,14 +297,18 @@ void GRID_CELL_URL_EDITOR::Create( wxWindow* aParent, wxWindowID aId,
 {
     m_control = new TEXT_BUTTON_URL( aParent, m_dlg );
 
+#if wxUSE_VALIDATORS
+    // validate text in textctrl, if validator is set
+    if ( m_validator )
+    {
+        Combo()->SetValidator( *m_validator );
+    }
+#endif
+
     wxGridCellEditor::Create( aParent, aId, aEventHandler );
 }
 
 
-/**
- * Path Picker
- */
-
 class TEXT_BUTTON_FILE_BROWSER : public wxComboCtrl
 {
 public:
@@ -362,6 +372,14 @@ void GRID_CELL_PATH_EDITOR::Create( wxWindow* aParent, wxWindowID aId,
 {
     m_control = new TEXT_BUTTON_FILE_BROWSER( aParent, m_dlg, m_currentDir );
 
+#if wxUSE_VALIDATORS
+    // validate text in textctrl, if validator is set
+    if ( m_validator )
+    {
+        Combo()->SetValidator( *m_validator );
+    }
+#endif
+
     wxGridCellEditor::Create( aParent, aId, aEventHandler );
 }
 
@@ -371,6 +389,14 @@ void GRID_CELL_SYMLIB_EDITOR::Create( wxWindow* aParent, wxWindowID aId,
 {
     m_control = new TEXT_BUTTON_FILE_BROWSER( aParent, m_dlg, m_currentDir, &m_ext );
 
+#if wxUSE_VALIDATORS
+    // validate text in textctrl, if validator is set
+    if ( m_validator )
+    {
+        Combo()->SetValidator( *m_validator );
+    }
+#endif
+
     wxGridCellEditor::Create( aParent, aId, aEventHandler );
 }
 

eeschema/dialogs/dialog_edit_component_in_schematic.cpp

@@ -39,6 +39,7 @@
 #include <sch_draw_panel.h>
 #include <sch_edit_frame.h>
 #include <sch_reference_list.h>
+#include <sch_validators.h>
 #include <symbol_lib_table.h>
 
 #ifdef KICAD_SPICE
@@ -97,7 +98,9 @@ DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::DIALOG_EDIT_COMPONENT_IN_SCHEMATIC( SCH_EDIT
     m_bpMoveDown->SetBitmap( KiBitmap( small_down_xpm ) );
 
     // wxFormBuilder doesn't include this event...
-    m_grid->Connect( wxEVT_GRID_CELL_CHANGING, wxGridEventHandler( DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnGridCellChanging ), NULL, this );
+    m_grid->Connect( wxEVT_GRID_CELL_CHANGING,
+                     wxGridEventHandler( DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnGridCellChanging ),
+                     NULL, this );
 
     FinishDialogSettings();
 }
@@ -110,7 +113,9 @@ DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::~DIALOG_EDIT_COMPONENT_IN_SCHEMATIC()
     // Prevents crash bug in wxGrid's d'tor
     m_grid->DestroyTable( m_fields );
 
-    m_grid->Disconnect( wxEVT_GRID_CELL_CHANGING, wxGridEventHandler( DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnGridCellChanging ), NULL, this );
+    m_grid->Disconnect( wxEVT_GRID_CELL_CHANGING,
+                        wxGridEventHandler( DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::OnGridCellChanging ),
+                        NULL, this );
 
     // Delete the GRID_TRICKS.
     m_grid->PopEventHandler( true );
@@ -316,7 +321,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::Validate()
     wxString msg;
     LIB_ID   id;
 
-    if( !m_grid->CommitPendingChanges() )
+    if( !m_grid->CommitPendingChanges() || !m_grid->Validate() )
         return false;
 
     if( !SCH_COMPONENT::IsReferenceStringValid( m_fields->at( REFERENCE ).GetText() ) )
@@ -388,7 +393,7 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::Validate()
 
 bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::TransferDataFromWindow()
 {
-    if( !Validate() )
+    if( !wxDialog::TransferDataFromWindow() )  // Calls our Validate() method.
         return false;
 
     // save old cmp in undo list if not already in edit, or moving ...
@@ -484,9 +489,11 @@ bool DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::TransferDataFromWindow()
 
         SCH_REFERENCE_LIST components;
         GetParent()->GetCurrentSheet().GetComponents( components );
+
         for( unsigned i = 0; i < components.GetCount(); i++ )
         {
             SCH_REFERENCE component = components[i];
+
             if( component.GetLibPart()->GetLibId() == thisLibId
                     && component.GetRef() == thisRef
                     && component.GetUnit() != thisUnit )

eeschema/fields_grid_table.cpp

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2018-2019 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -75,10 +75,21 @@ FIELDS_GRID_TABLE<T>::FIELDS_GRID_TABLE( DIALOG_SHIM* aDialog, SCH_BASE_FRAME* a
     m_valueAttr->SetEditor( valueEditor );
 
     m_footprintAttr = new wxGridCellAttr;
-    m_footprintAttr->SetEditor( new GRID_CELL_FOOTPRINT_ID_EDITOR( aDialog ) );
+    GRID_CELL_FOOTPRINT_ID_EDITOR* fpIdEditor = new GRID_CELL_FOOTPRINT_ID_EDITOR( aDialog );
+    fpIdEditor->SetValidator( LIB_ID_VALIDATOR( LIB_ID::ID_PCB ) );
+    m_footprintAttr->SetEditor( fpIdEditor );
 
     m_urlAttr = new wxGridCellAttr;
-    m_urlAttr->SetEditor( new GRID_CELL_URL_EDITOR( aDialog ) );
+    GRID_CELL_URL_EDITOR* urlEditor = new GRID_CELL_URL_EDITOR( aDialog );
+    urlEditor->SetValidator( SCH_FIELD_VALIDATOR( aFrame->IsType( FRAME_SCH_LIB_EDITOR ),
+                                                  DATASHEET ) );
+    m_urlAttr->SetEditor( urlEditor );
+
+    m_nonUrlAttr = new wxGridCellAttr;
+    wxGridCellTextEditor* nonUrlEditor = new wxGridCellTextEditor();
+    nonUrlEditor->SetValidator( SCH_FIELD_VALIDATOR( aFrame->IsType( FRAME_SCH_LIB_EDITOR ),
+                                                     DATASHEET ) );
+    m_nonUrlAttr->SetEditor( nonUrlEditor );
 
     m_boolAttr = new wxGridCellAttr;
     m_boolAttr->SetRenderer( new wxGridCellBoolRenderer() );
@@ -239,6 +250,11 @@ wxGridCellAttr* FIELDS_GRID_TABLE<T>::GetAttr( int aRow, int aCol, wxGridCellAtt
                 m_urlAttr->IncRef();
                 return m_urlAttr;
             }
+            else
+            {
+                m_nonUrlAttr->IncRef();
+                return m_nonUrlAttr;
+            }
         }
         return nullptr;
 
@@ -281,7 +297,7 @@ wxString FIELDS_GRID_TABLE<T>::GetValue( int aRow, int aCol )
     switch( aCol )
     {
     case FDC_NAME:
-        // Use default field name for mandatory fields, because they are transalted
+        // Use default field name for mandatory fields, because they are translated
         // according to the current locale
         if( aRow < MANDATORY_FIELDS )
             return TEMPLATE_FIELDNAME::GetDefaultFieldName( aRow );
@@ -488,17 +504,18 @@ template class FIELDS_GRID_TABLE<SCH_FIELD>;
 template class FIELDS_GRID_TABLE<LIB_FIELD>;
 
 
-
 void FIELDS_GRID_TRICKS::showPopupMenu( wxMenu& menu )
 {
     if( m_grid->GetGridCursorRow() == FOOTPRINT && m_grid->GetGridCursorCol() == FDC_VALUE )
     {
-        menu.Append( MYID_SELECT_FOOTPRINT, _( "Select Footprint..." ), _( "Browse for footprint" ) );
+        menu.Append( MYID_SELECT_FOOTPRINT, _( "Select Footprint..." ),
+                     _( "Browse for footprint" ) );
         menu.AppendSeparator();
     }
     else if( m_grid->GetGridCursorRow() == DATASHEET && m_grid->GetGridCursorCol() == FDC_VALUE )
     {
-        menu.Append( MYID_SHOW_DATASHEET,   _( "Show Datasheet" ),      _( "Show datasheet in browser" ) );
+        menu.Append( MYID_SHOW_DATASHEET, _( "Show Datasheet" ),
+                     _( "Show datasheet in browser" ) );
         menu.AppendSeparator();
     }
 
@@ -530,6 +547,7 @@ void FIELDS_GRID_TRICKS::doPopupSelection( wxCommandEvent& event )
     }
 }
 
+
 template <class T>
 wxString FIELDS_GRID_TABLE<T>::StringFromBool( bool aValue )
 {
@@ -539,20 +557,22 @@ wxString FIELDS_GRID_TABLE<T>::StringFromBool( bool aValue )
         return wxT( "0" );
 }
 
+
 template <class T>
 bool FIELDS_GRID_TABLE<T>::BoolFromString( wxString aValue )
 {
-    if( aValue == wxT( "1" ) )
+    if( aValue == "1" )
     {
         return true;
     }
-    else if( aValue == wxT( "0" ) )
+    else if( aValue == "0" )
     {
         return false;
     }
     else
     {
-        wxFAIL_MSG( wxString::Format( wxT( "string \"%s\" can't be converted to boolean correctly, it will have been perceived as FALSE" ), aValue ) );
+        wxFAIL_MSG( wxString::Format( "string \"%s\" can't be converted to boolean "
+                                      "correctly, it will have been perceived as FALSE", aValue ) );
         return false;
     }
 }

eeschema/fields_grid_table.h

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2018-2019 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -112,6 +112,7 @@ private:
     wxGridCellAttr*       m_valueAttr;
     wxGridCellAttr*       m_footprintAttr;
     wxGridCellAttr*       m_urlAttr;
+    wxGridCellAttr*       m_nonUrlAttr;
     wxGridCellAttr*       m_boolAttr;
     wxGridCellAttr*       m_vAlignAttr;
     wxGridCellAttr*       m_hAlignAttr;

eeschema/sch_validators.h

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2016 Wayne Stambaugh, stambaughw@gmail.com
- * Copyright (C) 2016 KiCad Developers, see change_log.txt for contributors.
+ * Copyright (C) 2016-2019 KiCad Developers, see change_log.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -24,7 +24,7 @@
 
 /**
  * @file sch_validators.h
- * @brief Defintions of control validators for schematic dialogs.
+ * @brief Definitions of control validators for schematic dialogs.
  */
 
 #ifndef _SCH_VALIDATORS_H_
@@ -37,15 +37,12 @@
 
 
 /**
- * class SCH_FILED_VALIDATOR
+ * A text control validator used for validating the text allowed in library and
- *
- * is the text control validator used for validating the text allowed in library and
  * schematic component fields.
- * Note
+ *
- * Reference field does not accept spaces
+ * - The reference field does not accept spaces.
- * Value field does not accept spaces in Component Library Editor, because in .lib component
+ * - The value field does not accept spaces in the symbol library editor because in symbol
- * libraries, the value field is the component name in lib, and spaces are not allowed
+ *   libraries, the value field is the symbol name in the library.
- * in component names in lib
  */
 class SCH_FIELD_VALIDATOR : public wxTextValidator
 {
@@ -60,9 +57,7 @@ public:
     virtual wxObject* Clone() const override { return new SCH_FIELD_VALIDATOR( *this ); }
 
     /**
-     * Function Validate
+     * Override the default Validate() function provided by wxTextValidator to provide
-     *
-     * overrides the default Validate() function provided by wxTextValidate to provide
      * better error messages.
      *
      * @param aParent - a pointer to the parent window of the error message dialog.

include/validators.h

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2013 Wayne Stambaugh <stambaughw@verizon.net>
- * Copyright (C) 2004-2013 KiCad Developers, see change_log.txt for contributors.
+ * Copyright (C) 2004-2019 KiCad Developers, see change_log.txt for contributors.
  * Copyright (C) 2018 CERN
  *
  * This program is free software; you can redistribute it and/or
@@ -35,9 +35,9 @@
 #include <wx/grid.h>
 #include <wx/regex.h>
 
+#include <lib_id.h>
+
 /**
- * Class GRID_CELL_TEXT_EDITOR
- *
  * This class works around a bug in wxGrid where the first keystroke doesn't get sent through
  * the validator if the editor wasn't already open.
  */
@@ -55,8 +55,6 @@ protected:
 
 
 /**
- * Class FILE_NAME_CHAR_VALIDATOR
- *
  * This class provides a custom wxValidator object for limiting the allowable characters when
  * defining footprint names.  Since the introduction of the PRETTY footprint library format,
  * footprint names cannot have any characters that would prevent file creation on any platform.
@@ -70,8 +68,6 @@ public:
 
 
 /**
- * Class FILE_NAME_WITH_PATH_CHAR_VALIDATOR
- *
  * This class provides a custom wxValidator object for limiting the allowable characters when
  * defining file names with path, for instance in schematic sheet file names.
  * The characters *?|"<> are illegal and filtered by the validator,
@@ -85,10 +81,8 @@ public:
 
 
 /**
- * Class ENV_VAR_NAME_VALIDATOR
- *
  * This class provides a custom wxValidator object for limiting the allowable characters
- * when defining an environment varaible name in a text edit control.  Only uppercase,
+ * when defining an environment variable name in a text edit control.  Only uppercase,
  * numbers, and underscore (_) characters are valid and the first character of the name
  * cannot start with a number.  This is according to IEEE Std 1003.1-2001.  Even though
  * most systems support other characters, these characters guarantee compatibility for
@@ -123,8 +117,6 @@ class REGEX_VALIDATOR : public wxTextValidator
 {
 public:
     /**
-     * Constructor.
-     *
      * @param aRegEx is a regular expression to validate strings.
      * @param aValue is a pointer to a wxString containing the value to validate.
      */
@@ -135,8 +127,6 @@ public:
     }
 
     /**
-     * Constructor.
-     *
      * @param aRegEx is a regular expression to validate strings.
      * @param aFlags are compilation flags (normally wxRE_DEFAULT).
      * @param aValue is a pointer to a wxString containing the value to validate.
@@ -174,10 +164,38 @@ protected:
     wxRegEx m_regEx;
 };
 
+/**
+ * Custom validator that verifies that a string defines a valid #LIB_ID.
+ */
+class LIB_ID_VALIDATOR : public wxTextValidator
+{
+public:
+    /**
+     * @param aLibIdType is the type of #LIB_ID object to validate.
+     * @param aValue is a pointer to a wxString containing the value to validate.
+     */
+    LIB_ID_VALIDATOR( LIB_ID::LIB_ID_TYPE aLibIdType, wxString* aValue = NULL ) :
+        wxTextValidator( wxFILTER_EXCLUDE_CHAR_LIST, aValue ),
+        m_idType( aLibIdType )
+    {
+        SetCharExcludes( "\r\n\t" );
+    }
+
+    virtual wxObject* Clone() const override
+    {
+        return new LIB_ID_VALIDATOR( *this );
+    }
+
+    bool Validate( wxWindow* aParent ) override;
+
+protected:
+    LIB_ID::LIB_ID_TYPE m_idType;
+};
+
 namespace KIUI
 {
 /**
- * Call a text validator's TransferToWindow method without firing
+ * Call a text validator's TransferDataToWindow method without firing
  * a text change event.
  *
  * This is useful when you want to keep a validator in sync with other data,

include/widgets/grid_text_button_helpers.h

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2018-2019 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -48,9 +48,17 @@ public:
     void ApplyEdit( int aRow, int aCol, wxGrid* aGrid ) override;
     void Reset() override;
 
+#if wxUSE_VALIDATORS
+    void SetValidator( const wxValidator& validator );
+#endif
+
 protected:
     wxComboCtrl* Combo() const { return static_cast<wxComboCtrl*>( m_control ); }
 
+#if wxUSE_VALIDATORS
+    wxScopedPtr< wxValidator > m_validator;
+#endif
+
     wxString     m_value;
 
     wxDECLARE_NO_COPY_CLASS( GRID_CELL_TEXT_BUTTON );