Add option to suppress validation of sheet titles (#186)

Add option to suppress validation of sheet titles

Based on a "lowest common denominator" approach to compatibility,
we will continue to enforce a 31-character limit for sheet titles.
However, this limit should not be enforced when loading an existing
file.

Added a new optional parameter to Worksheet::setTitle() and
Worksheet::setCodeName() to suppress validation and massaging,
based on the premise that existing files should be given a
best-effort approach to loading and parsing. Unfortunately, it's
not possible with the current architecture to prevent users from
making use of this functionality, aside from with a strongly-worded
warning.

Added test coverage. I didn't see any existing unit tests of the
Worksheet class, so I created a new test to cover these methods.

Fixes #176

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fix to getCell() method when cell reference includes a worksheet reference - @MarkBaker
 - Ignore inlineStr type if formula element exists - @ncrypthic [#570](https://github.com/PHPOffice/PHPExcel/issues/570)
 - Excel 2007 Reader freezes because of conditional formatting - @rentalhost [#575](https://github.com/PHPOffice/PHPExcel/issues/575)
+- Readers will now parse files containing worksheet titles over 31 characters [#176](https://github.com/PHPOffice/PhpSpreadsheet/pull/176)
 
 ### General
 

src/PhpSpreadsheet/Reader/Gnumeric.php

@@ -351,7 +351,7 @@ class Gnumeric extends BaseReader implements IReader
             //    Use false for $updateFormulaCellReferences to prevent adjustment of worksheet references in formula
             //        cells... during the load, all formulae should be correct, and we're simply bringing the worksheet
             //        name in line with the formula, not the reverse
-            $spreadsheet->getActiveSheet()->setTitle($worksheetName, false);
+            $spreadsheet->getActiveSheet()->setTitle($worksheetName, false, false);
 
             if ((!$this->readDataOnly) && (isset($sheet->PrintInformation))) {
                 if (isset($sheet->PrintInformation->Margins)) {

src/PhpSpreadsheet/Reader/Html.php

@@ -317,7 +317,7 @@ class Html extends BaseReader implements IReader
                         break;
                     case 'title':
                         $this->processDomElement($child, $sheet, $row, $column, $cellContent);
-                        $sheet->setTitle($cellContent);
+                        $sheet->setTitle($cellContent, true, false);
                         $cellContent = '';
                         break;
                     case 'span':

src/PhpSpreadsheet/Reader/Ods.php

@@ -440,7 +440,7 @@ class Ods extends BaseReader implements IReader
                     // Use false for $updateFormulaCellReferences to prevent adjustment of worksheet references in
                     // formula cells... during the load, all formulae should be correct, and we're simply
                     // bringing the worksheet name in line with the formula, not the reverse
-                    $spreadsheet->getActiveSheet()->setTitle($worksheetName, false);
+                    $spreadsheet->getActiveSheet()->setTitle($worksheetName, false, false);
                 }
 
                 // Go through every child of table element

src/PhpSpreadsheet/Reader/Xls.php

@@ -808,7 +808,7 @@ class Xls extends BaseReader implements IReader
             //    Use false for $updateFormulaCellReferences to prevent adjustment of worksheet references in formula
             //        cells... during the load, all formulae should be correct, and we're simply bringing the worksheet
             //        name in line with the formula, not the reverse
-            $this->phpSheet->setTitle($sheet['name'], false);
+            $this->phpSheet->setTitle($sheet['name'], false, false);
             $this->phpSheet->setSheetState($sheet['sheetState']);
 
             $this->pos = $sheet['offset'];

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -697,7 +697,7 @@ class Xlsx extends BaseReader implements IReader
                             //        references in formula cells... during the load, all formulae should be correct,
                             //        and we're simply bringing the worksheet name in line with the formula, not the
                             //        reverse
-                            $docSheet->setTitle((string) $eleSheet['name'], false);
+                            $docSheet->setTitle((string) $eleSheet['name'], false, false);
                             $fileWorksheet = $worksheets[(string) self::getArrayItem($eleSheet->attributes('http://schemas.openxmlformats.org/officeDocument/2006/relationships'), 'id')];
                             $xmlSheet = simplexml_load_string(
                                 //~ http://schemas.openxmlformats.org/spreadsheetml/2006/main"
@@ -766,7 +766,7 @@ class Xlsx extends BaseReader implements IReader
                                 }
                             }
                             if (isset($xmlSheet->sheetPr) && isset($xmlSheet->sheetPr['codeName'])) {
-                                $docSheet->setCodeName((string) $xmlSheet->sheetPr['codeName']);
+                                $docSheet->setCodeName((string) $xmlSheet->sheetPr['codeName'], false);
                             }
                             if (isset($xmlSheet->sheetPr) && isset($xmlSheet->sheetPr->outlinePr)) {
                                 if (isset($xmlSheet->sheetPr->outlinePr['summaryRight']) &&

src/PhpSpreadsheet/Reader/Xml.php

@@ -552,7 +552,7 @@ class Xml extends BaseReader implements IReader
                 //    Use false for $updateFormulaCellReferences to prevent adjustment of worksheet references in
                 //        formula cells... during the load, all formulae should be correct, and we're simply bringing
                 //        the worksheet name in line with the formula, not the reverse
-                $spreadsheet->getActiveSheet()->setTitle($worksheetName, false);
+                $spreadsheet->getActiveSheet()->setTitle($worksheetName, false, false);
             }
 
             $columnID = 'A';

src/PhpSpreadsheet/Worksheet.php

@@ -825,27 +825,30 @@ class Worksheet implements IComparable
      * Set title.
      *
      * @param string $pValue String containing the dimension of this worksheet
-     * @param string $updateFormulaCellReferences boolean Flag indicating whether cell references in formulae should
+     * @param bool $updateFormulaCellReferences Flag indicating whether cell references in formulae should
      *            be updated to reflect the new sheet name.
      *          This should be left as the default true, unless you are
      *          certain that no formula cells on any worksheet contain
      *          references to this worksheet
+     * @param bool $validate False to skip validation of new title. WARNING: This should only be set
+     *                       at parse time (by Readers), where titles can be assumed to be valid.
      *
      * @return Worksheet
      */
-    public function setTitle($pValue, $updateFormulaCellReferences = true)
+    public function setTitle($pValue, $updateFormulaCellReferences = true, $validate = true)
     {
         // Is this a 'rename' or not?
         if ($this->getTitle() == $pValue) {
             return $this;
         }
 
-        // Syntax check
-        self::checkSheetTitle($pValue);
-
         // Old title
         $oldTitle = $this->getTitle();
 
+        if ($validate) {
+            // Syntax check
+            self::checkSheetTitle($pValue);
+
             if ($this->parent) {
                 // Is there already such sheet name?
                 if ($this->parent->sheetNameExists($pValue)) {
@@ -868,9 +871,8 @@ class Worksheet implements IComparable
                         }
                     }
 
-                $altTitle = $pValue . ' ' . $i;
+                    $pValue .= " $i";
-
+                }
-                return $this->setTitle($altTitle, $updateFormulaCellReferences);
             }
         }
 
@@ -2976,20 +2978,25 @@ class Worksheet implements IComparable
     /**
      * Define the code name of the sheet.
      *
-     * @param null|string Same rule as Title minus space not allowed (but, like Excel, change silently space to underscore)
+     * @param string $pValue Same rule as Title minus space not allowed (but, like Excel, change
-     * @param null|mixed $pValue
+     *                       silently space to underscore)
+     * @param bool $validate False to skip validation of new title. WARNING: This should only be set
+     *                       at parse time (by Readers), where titles can be assumed to be valid.
      *
      * @throws Exception
      *
      * @return objWorksheet
      */
-    public function setCodeName($pValue)
+    public function setCodeName($pValue, $validate = true)
     {
         // Is this a 'rename' or not?
         if ($this->getCodeName() == $pValue) {
             return $this;
         }
+
+        if ($validate) {
             $pValue = str_replace(' ', '_', $pValue); //Excel does this automatically without flinching, we are doing the same
+
             // Syntax check
             // throw an exception if not valid
             self::checkSheetCodeName($pValue);
@@ -3019,8 +3026,7 @@ class Worksheet implements IComparable
                     }
 
                     $pValue = $pValue . '_' . $i; // ok, we have a valid name
-                //codeName is'nt used in formula : no need to call for an update
+                }
-                //return $this->setTitle($altTitle, $updateFormulaCellReferences);
             }
         }
 

tests/PhpSpreadsheetTests/WorksheetTest.php

@@ -0,0 +1,126 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Worksheet;
+use PHPUnit_Framework_TestCase;
+
+class WorksheetTest extends PHPUnit_Framework_TestCase
+{
+    public function testSetTitle()
+    {
+        $testTitle = str_repeat('a', 31);
+
+        $worksheet = new Worksheet();
+        $worksheet->setTitle($testTitle);
+        $this->assertSame($testTitle, $worksheet->getTitle());
+    }
+
+    public function setTitleInvalidProvider()
+    {
+        return [
+            [str_repeat('a', 32), 'Maximum 31 characters allowed in sheet title.'],
+            ['invalid*title', 'Invalid character found in sheet title'],
+        ];
+    }
+
+    /**
+     * @param string $title
+     * @param string $expectMessage
+     * @dataProvider setTitleInvalidProvider
+     */
+    public function testSetTitleInvalid($title, $expectMessage)
+    {
+        // First, test setting title with validation disabled -- should be successful
+        $worksheet = new Worksheet();
+        $worksheet->setTitle($title, true, false);
+
+        // Next, test again with validation enabled -- this time we should fail
+        $worksheet = new Worksheet();
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage($expectMessage);
+        $worksheet->setTitle($title);
+    }
+
+    public function testSetTitleDuplicate()
+    {
+        // Create a Spreadsheet with three Worksheets (the first is created automatically)
+        $spreadsheet = new Spreadsheet();
+        $spreadsheet->createSheet();
+        $spreadsheet->createSheet();
+
+        // Set unique title -- should be unchanged
+        $sheet = $spreadsheet->getSheet(0);
+        $sheet->setTitle('Test Title');
+        $this->assertSame('Test Title', $sheet->getTitle());
+
+        // Set duplicate title -- should have numeric suffix appended
+        $sheet = $spreadsheet->getSheet(1);
+        $sheet->setTitle('Test Title');
+        $this->assertSame('Test Title 1', $sheet->getTitle());
+
+        // Set duplicate title with validation disabled -- should be unchanged
+        $sheet = $spreadsheet->getSheet(2);
+        $sheet->setTitle('Test Title', true, false);
+        $this->assertSame('Test Title', $sheet->getTitle());
+    }
+
+    public function testSetCodeName()
+    {
+        $testCodeName = str_repeat('a', 31);
+
+        $worksheet = new Worksheet();
+        $worksheet->setCodeName($testCodeName);
+        $this->assertSame($testCodeName, $worksheet->getCodeName());
+    }
+
+    public function setCodeNameInvalidProvider()
+    {
+        return [
+            [str_repeat('a', 32), 'Maximum 31 characters allowed in sheet code name.'],
+            ['invalid*code*name', 'Invalid character found in sheet code name'],
+        ];
+    }
+
+    /**
+     * @param string $codeName
+     * @param string $expectMessage
+     * @dataProvider setCodeNameInvalidProvider
+     */
+    public function testSetCodeNameInvalid($codeName, $expectMessage)
+    {
+        // First, test setting code name with validation disabled -- should be successful
+        $worksheet = new Worksheet();
+        $worksheet->setCodeName($codeName, false);
+
+        // Next, test again with validation enabled -- this time we should fail
+        $worksheet = new Worksheet();
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessage($expectMessage);
+        $worksheet->setCodeName($codeName);
+    }
+
+    public function testSetCodeNameDuplicate()
+    {
+        // Create a Spreadsheet with three Worksheets (the first is created automatically)
+        $spreadsheet = new Spreadsheet();
+        $spreadsheet->createSheet();
+        $spreadsheet->createSheet();
+
+        // Set unique code name -- should be massaged to Snake_Case
+        $sheet = $spreadsheet->getSheet(0);
+        $sheet->setCodeName('Test Code Name');
+        $this->assertSame('Test_Code_Name', $sheet->getCodeName());
+
+        // Set duplicate code name -- should be massaged and have numeric suffix appended
+        $sheet = $spreadsheet->getSheet(1);
+        $sheet->setCodeName('Test Code Name');
+        $this->assertSame('Test_Code_Name_1', $sheet->getCodeName());
+
+        // Set duplicate code name with validation disabled -- should be unchanged, and unmassaged
+        $sheet = $spreadsheet->getSheet(2);
+        $sheet->setCodeName('Test Code Name', false);
+        $this->assertSame('Test Code Name', $sheet->getCodeName());
+    }
+}