From cffe1b51e20913e325661ec30a49fda51241565d Mon Sep 17 00:00:00 2001
From: jean-pierre charras <jp.charras@wanadoo.fr>
Date: Tue, 24 Oct 2017 18:09:33 +0200
Subject: [PATCH] Gerbview: avoid a crash with malformed Gerber files. Fix also
 incorrect handling of %LN (Load Name) deprecated command

---
 gerbview/class_am_param.cpp       |  3 +-
 gerbview/class_aperture_macro.cpp | 50 +++++++++++++++++++++++--------
 gerbview/rs274x.cpp               | 18 +++++------
 3 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/gerbview/class_am_param.cpp b/gerbview/class_am_param.cpp
index e60c46375d..666e8a063e 100644
--- a/gerbview/class_am_param.cpp
+++ b/gerbview/class_am_param.cpp
@@ -127,7 +127,8 @@ double AM_PARAM::GetValue( const D_CODE* aDcode ) const
                 break;
 
             default:
-                wxLogDebug( "AM_PARAM::GetValue(): unexpected type\n" );
+                wxLogDebug( "AM_PARAM::GetValue(): dcode %d prm %d/%d: unexpected type %d",
+                            aDcode->m_Num_Dcode, ii, m_paramStack.size(), item.GetType() );
                 break;
         }
     }
diff --git a/gerbview/class_aperture_macro.cpp b/gerbview/class_aperture_macro.cpp
index 9896ff1b51..07417f97f6 100644
--- a/gerbview/class_aperture_macro.cpp
+++ b/gerbview/class_aperture_macro.cpp
@@ -352,19 +352,34 @@ void AM_PRIMITIVE::DrawBasicShape( GERBER_DRAW_ITEM* aParent,
         */
         /* Generated by an aperture macro declaration like:
          * "4,1,3,0.0,0.0,0.0,0.5,0.5,0.5,0.5,0.0,-25"
-         * type(4), exposure, corners count, corner1.x, corner.1y, ..., rotation
+         * type(4), exposure, corners count, corner1.x, corner.1y, ..., corner1.x, corner.1y, rotation
          * type is not stored in parameters list, so the first parameter is exposure
          */
-        int numPoints = (int) params[1].GetValue( tool );
-        rotation  = params[numPoints * 2 + 4].GetValue( tool ) * 10.0;
+        // params[0] is the exposure and params[1] is the corners count after the first corner
+        int numCorners = (int) params[1].GetValue( tool );
+        // the shape rotation is the last param of list, after corners
+        int last_prm = params.size() - 1;
+        rotation  = params[last_prm].GetValue( tool ) * 10.0;
         wxPoint pos;
-        // Read points. numPoints does not include the starting point, so add 1.
-        for( int i = 0; i<numPoints + 1; ++i )
+
+        // Read points.
+        // Note: numCorners is the polygon corner count, following the first corner
+        // * the polygon is always closed,
+        // * therefore the last XY coordinate is the same as the first
+        int prm_idx = 2;    //  params[2] is the first X coordinate
+        for( int i = 0; i <= numCorners; ++i )
         {
-            int jj = i * 2 + 2;
-            pos.x = scaletoIU( params[jj].GetValue( tool ), m_GerbMetric );
-            pos.y = scaletoIU( params[jj + 1].GetValue( tool ), m_GerbMetric );
+            pos.x = scaletoIU( params[prm_idx].GetValue( tool ), m_GerbMetric );
+            prm_idx++;
+            pos.y = scaletoIU( params[prm_idx].GetValue( tool ), m_GerbMetric );
+            prm_idx++;
             polybuffer.push_back(pos);
+
+            // Guard: ensure prm_idx < last_prm
+            // I saw malformed gerber files with numCorners = number
+            // of coordinates instead of number of coordinates following the first point
+            if( prm_idx >= last_prm )
+                break;
         }
         // rotate polygon and move it to the actual position
         // shape rotation:
@@ -701,15 +716,20 @@ int AM_PRIMITIVE::GetShapeDim( GERBER_DRAW_ITEM* aParent )
     // dim = min side of the bounding box (this is a poor criteria, but what is a good criteria b?)
     {
         // exposure, corners count, corner1.x, corner.1y, ..., rotation
+        // note: corners count is the count of corners following corner1
         int numPoints = (int) params[1].GetValue( tool );
         // Read points. numPoints does not include the starting point, so add 1.
         // and calculate the bounding box;
         wxSize pos_min, pos_max, pos;
-        for( int i = 0; i<numPoints + 1; ++i )
+        int prm_idx = 2;    //  params[2] is the first X coordinate
+        int last_prm = params.size() - 1;
+
+        for( int i = 0; i<= numPoints; ++i )
         {
-            int jj = i * 2 + 2;
-            pos.x = scaletoIU( params[jj].GetValue( tool ), m_GerbMetric );
-            pos.y = scaletoIU( params[jj + 1].GetValue( tool ), m_GerbMetric );
+            pos.x = scaletoIU( params[prm_idx].GetValue( tool ), m_GerbMetric );
+            prm_idx++;
+            pos.y = scaletoIU( params[prm_idx].GetValue( tool ), m_GerbMetric );
+            prm_idx++;
             if( i == 0 )
                 pos_min = pos_max = pos;
             else
@@ -725,6 +745,12 @@ int AM_PRIMITIVE::GetShapeDim( GERBER_DRAW_ITEM* aParent )
                 if( pos_max.y < pos.y )
                     pos_max.y = pos.y;
             }
+
+            // Guard: ensure prm_idx < last_prm (last prm is orientation)
+            // I saw malformed gerber files with numCorners = number
+            // of coordinates instead of number of coordinates following the first point
+            if( prm_idx >= last_prm )
+                break;
         }
         // calculate dim
         wxSize size;
diff --git a/gerbview/rs274x.cpp b/gerbview/rs274x.cpp
index 37c236c6e3..06251432c6 100644
--- a/gerbview/rs274x.cpp
+++ b/gerbview/rs274x.cpp
@@ -65,7 +65,7 @@ enum RS274X_PARAMETERS {
     SCALE_FACTOR   = CODE( 'S', 'F' ),          // Default:  A = 1.0, B = 1.0
 
     // Image parameters:
-    // commands used only once at the beginning of the file
+    // commands used only once at the beginning of the file, and are deprecated
     IMAGE_JUSTIFY  = CODE( 'I', 'J' ),          // Default: no justification
     IMAGE_NAME     = CODE( 'I', 'N' ),          // Default: void
     IMAGE_OFFSET   = CODE( 'I', 'O' ),          // Default: A = 0, B = 0
@@ -102,11 +102,12 @@ enum RS274X_PARAMETERS {
     // May be used singly or may be layer specfic
     // theses parameters are at the beginning of the file or layer
     // and reset some layer parameters (like interpolation)
-    LAYER_NAME      = CODE( 'L', 'N' ),         // Default: Positive
-    LAYER_POLARITY  = CODE( 'L', 'P' ),
     KNOCKOUT = CODE( 'K', 'O' ),                // Default: off
     STEP_AND_REPEAT = CODE( 'S', 'R' ),         //  Default: A = 1, B = 1
     ROTATE = CODE( 'R', 'O' ),                  //  Default: 0
+
+    LOAD_POLARITY  = CODE( 'L', 'P' ),          //LPC or LPD. Default: Dark (LPD)
+    LOAD_NAME      = CODE( 'L', 'N' ),          // Deprecated: equivalent to G04
 };
 
 
@@ -660,12 +661,11 @@ bool GERBER_FILE_IMAGE::ExecuteRS274XCommand( int command, char* buff, char*& te
 
         break;
 
-    case LAYER_NAME:
-        m_Iterpolation = GERB_INTERPOL_LINEAR_1X;       // Start a new Gerber layer
-        GetLayerParams( ).m_LayerName.Empty();
-        while( *text != '*' )
+    case LOAD_NAME:
+        // %LN is a (deprecated) equivalentto G04: a comment
+        while( *text && *text != '*' )
         {
-            GetLayerParams( ).m_LayerName.Append( *text++ );
+            text++; // Skip text
         }
 
         break;
@@ -679,7 +679,7 @@ bool GERBER_FILE_IMAGE::ExecuteRS274XCommand( int command, char* buff, char*& te
                    m_ImageNegative ? "true" : "false" ); )
         break;
 
-    case LAYER_POLARITY:
+    case LOAD_POLARITY:
         if( *text == 'C' )
             GetLayerParams().m_LayerNegative = true;
         else