Prevent error when close() called while writer already closed (#402)

Instead of across all workbooks (in case of multiple spreadsheets being created at the same time).

README.md

@@ -3,6 +3,7 @@
 [![Latest Stable Version](https://poser.pugx.org/box/spout/v/stable)](https://packagist.org/packages/box/spout)
 [![Project Status](http://opensource.box.com/badges/active.svg)](http://opensource.box.com/badges)
 [![Build Status](https://travis-ci.org/box/spout.svg?branch=master)](https://travis-ci.org/box/spout)
+[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/box/spout/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/box/spout/?branch=master)
 [![Code Coverage](https://scrutinizer-ci.com/g/box/spout/badges/coverage.png?b=master)](https://scrutinizer-ci.com/g/box/spout/?branch=master)
 [![Total Downloads](https://poser.pugx.org/box/spout/downloads)](https://packagist.org/packages/box/spout)
 
@@ -199,16 +200,16 @@ $writer->setDefaultRowStyle($defaultStyle)
 
 Unfortunately, Spout does not support all the possible formatting options yet. But you can find the most important ones:
 
-Category  | Property      | API
-----------|---------------|---------------------------------------
-Font      | Bold          | `StyleBuilder::setFontBold()`
-          | Italic        | `StyleBuilder::setFontItalic()`
-          | Underline     | `StyleBuilder::setFontUnderline()`
-          | Strikethrough | `StyleBuilder::setFontStrikethrough()`
-          | Font name     | `StyleBuilder::setFontName('Arial')`
-          | Font size     | `StyleBuilder::setFontSize(14)`
-          | Font color    | `StyleBuilder::setFontColor(Color::BLUE)`<br>`StyleBuilder::setFontColor(Color::rgb(0, 128, 255))`
-Alignment | Wrap text     | `StyleBuilder::setShouldWrapText(true|false)`
+| Category  | Property      | API
+|-----------|---------------|---------------------------------------
+| Font      | Bold          | `StyleBuilder::setFontBold()`
+|           | Italic        | `StyleBuilder::setFontItalic()`
+|           | Underline     | `StyleBuilder::setFontUnderline()`
+|           | Strikethrough | `StyleBuilder::setFontStrikethrough()`
+|           | Font name     | `StyleBuilder::setFontName('Arial')`
+|           | Font size     | `StyleBuilder::setFontSize(14)`
+|           | Font color    | `StyleBuilder::setFontColor(Color::BLUE)`<br>`StyleBuilder::setFontColor(Color::rgb(0, 128, 255))`
+| Alignment | Wrap text     | `StyleBuilder::setShouldWrapText(true|false)`
 
 #### New sheet creation
 

composer.json

@@ -31,7 +31,7 @@
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "2.7.x-dev"
+            "dev-master": "2.8.x-dev"
         }
     }
 

src/Spout/Common/Escaper/ODS.php

@@ -22,7 +22,26 @@ class ODS implements EscaperInterface
      */
     public function escape($string)
     {
-        return htmlspecialchars($string, ENT_QUOTES);
+        if (defined('ENT_DISALLOWED')) {
+            // 'ENT_DISALLOWED' ensures that invalid characters in the given document type are replaced.
+            // Otherwise control characters like a vertical tab "\v" will make the XML document unreadable by the XML processor
+            // @link https://github.com/box/spout/issues/329
+            $replacedString = htmlspecialchars($string, ENT_QUOTES | ENT_DISALLOWED);
+        } else {
+            // We are on hhvm or any other engine that does not support ENT_DISALLOWED
+            $escapedString =  htmlspecialchars($string, ENT_QUOTES);
+
+            // control characters values are from 0 to 1F (hex values) in the ASCII table
+            // some characters should not be escaped though: "\t", "\r" and "\n".
+            $regexPattern = '[\x00-\x08' .
+                            // skipping "\t" (0x9) and "\n" (0xA)
+                            '\x0B-\x0C' .
+                            // skipping "\r" (0xD)
+                            '\x0E-\x1F]';
+            $replacedString = preg_replace("/$regexPattern/", '�', $escapedString);
+        }
+
+        return $replacedString;
     }
 
     /**

src/Spout/Reader/CSV/RowIterator.php

@@ -14,10 +14,9 @@ use Box\Spout\Common\Helper\EncodingHelper;
 class RowIterator implements IteratorInterface
 {
     /**
-     * If no value is given to fgetcsv(), it defaults to 8192 (which may be too low).
-     * Alignement with other functions like fgets() is discussed here: https://bugs.php.net/bug.php?id=48421
+     * Value passed to fgetcsv. 0 means "unlimited" (slightly slower but accomodates for very long lines).
      */
-    const MAX_READ_BYTES_PER_LINE = 32768;
+    const MAX_READ_BYTES_PER_LINE = 0;
 
     /** @var resource Pointer to the CSV file to read */
     protected $filePointer;

src/Spout/Reader/ODS/SheetIterator.php

@@ -116,7 +116,7 @@ class SheetIterator implements IteratorInterface
         $escapedSheetName = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_TABLE_NAME);
         $sheetName = $this->escaper->unescape($escapedSheetName);
 
-        return new Sheet($this->xmlReader, $sheetName, $this->currentSheetIndex, $this->options);
+        return new Sheet($this->xmlReader, $this->currentSheetIndex, $sheetName, $this->options);
     }
 
     /**

src/Spout/Reader/XLSX/Helper/CellHelper.php

@@ -31,6 +31,9 @@ class CellHelper
      */
     public static function fillMissingArrayIndexes($dataArray, $fillValue = '')
     {
+        if (empty($dataArray)) {
+            return [];
+        }
         $existingIndexes = array_keys($dataArray);
 
         $newIndexes = array_fill_keys(range(0, max($existingIndexes)), $fillValue);

src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php

@@ -234,11 +234,6 @@ class SharedStringsHelper
         $textValue = '';
 
         foreach ($textNodes as $nodeIndex => $textNode) {
-            if ($nodeIndex !== 0) {
-                // add a space between each "t" node
-                $textValue .= ' ';
-            }
-
             $textNodeAsString = $textNode->__toString();
             $shouldPreserveWhitespace = $this->shouldPreserveWhitespace($textNode);
 

src/Spout/Reader/XLSX/RowIterator.php

@@ -220,9 +220,8 @@ class RowIterator implements IteratorInterface
     {
         // Read dimensions of the sheet
         $dimensionRef = $xmlReader->getAttribute(self::XML_ATTRIBUTE_REF); // returns 'A1:M13' for instance (or 'A1' for empty sheet)
-        if (preg_match('/[A-Z\d]+:([A-Z\d]+)/', $dimensionRef, $matches)) {
-            $lastCellIndex = $matches[1];
-            $this->numColumns = CellHelper::getColumnIndexFromCellIndex($lastCellIndex) + 1;
+        if (preg_match('/[A-Z]+\d+:([A-Z]+\d+)/', $dimensionRef, $matches)) {
+            $this->numColumns = CellHelper::getColumnIndexFromCellIndex($matches[1]) + 1;
         }
 
         return XMLProcessor::PROCESSING_CONTINUE;

src/Spout/Writer/AbstractWriter.php

@@ -350,6 +350,10 @@ abstract class AbstractWriter implements WriterInterface
      */
     public function close()
     {
+        if (!$this->isWriterOpened) {
+            return;
+        }
+
         $this->closeWriter();
 
         if (is_resource($this->filePointer)) {
@@ -378,4 +382,3 @@ abstract class AbstractWriter implements WriterInterface
         }
     }
 }
-

src/Spout/Writer/Common/Internal/AbstractWorkbook.php

@@ -16,6 +16,9 @@ abstract class AbstractWorkbook implements WorkbookInterface
     /** @var bool Whether new sheets should be automatically created when the max rows limit per sheet is reached */
     protected $shouldCreateNewSheetsAutomatically;
 
+    /** @var string Timestamp based unique ID identifying the workbook */
+    protected $internalId;
+
     /** @var WorksheetInterface[] Array containing the workbook's sheets */
     protected $worksheets = [];
 
@@ -30,6 +33,7 @@ abstract class AbstractWorkbook implements WorkbookInterface
     public function __construct($shouldCreateNewSheetsAutomatically, $defaultRowStyle)
     {
         $this->shouldCreateNewSheetsAutomatically = $shouldCreateNewSheetsAutomatically;
+        $this->internalId = uniqid();
     }
 
     /**

src/Spout/Writer/Common/Sheet.php

@@ -7,7 +7,7 @@ use Box\Spout\Writer\Exception\InvalidSheetNameException;
 
 /**
  * Class Sheet
- * External representation of a worksheet within a ODS file
+ * External representation of a worksheet
  *
  * @package Box\Spout\Writer\Common
  */
@@ -21,12 +21,15 @@ class Sheet
     /** @var array Invalid characters that cannot be contained in the sheet name */
     private static $INVALID_CHARACTERS_IN_SHEET_NAME = ['\\', '/', '?', '*', ':', '[', ']'];
 
-    /** @var array Associative array [SHEET_INDEX] => [SHEET_NAME] keeping track of sheets' name to enforce uniqueness */
+    /** @var array Associative array [WORKBOOK_ID] => [[SHEET_INDEX] => [SHEET_NAME]] keeping track of sheets' name to enforce uniqueness per workbook */
     protected static $SHEETS_NAME_USED = [];
 
     /** @var int Index of the sheet, based on order in the workbook (zero-based) */
     protected $index;
 
+    /** @var string ID of the sheet's associated workbook. Used to restrict sheet name uniqueness enforcement to a single workbook */
+    protected $associatedWorkbookId;
+
     /** @var string Name of the sheet */
     protected $name;
 
@@ -35,10 +38,16 @@ class Sheet
 
     /**
      * @param int $sheetIndex Index of the sheet, based on order in the workbook (zero-based)
+     * @param string $associatedWorkbookId ID of the sheet's associated workbook
      */
-    public function __construct($sheetIndex)
+    public function __construct($sheetIndex, $associatedWorkbookId)
     {
         $this->index = $sheetIndex;
+        $this->associatedWorkbookId = $associatedWorkbookId;
+        if (!isset(self::$SHEETS_NAME_USED[$associatedWorkbookId])) {
+            self::$SHEETS_NAME_USED[$associatedWorkbookId] = [];
+        }
+
         $this->stringHelper = new StringHelper();
         $this->setName(self::DEFAULT_SHEET_NAME_PREFIX . ($sheetIndex + 1));
     }
@@ -75,43 +84,58 @@ class Sheet
      */
     public function setName($name)
     {
-        if (!$this->isNameValid($name)) {
-            $errorMessage = "The sheet's name is invalid. It did not meet at least one of these requirements:\n";
-            $errorMessage .= " - It should not be blank\n";
-            $errorMessage .= " - It should not exceed 31 characters\n";
-            $errorMessage .= " - It should not contain these characters: \\ / ? * : [ or ]\n";
-            $errorMessage .= " - It should be unique";
-            throw new InvalidSheetNameException($errorMessage);
-        }
+        $this->throwIfNameIsInvalid($name);
 
         $this->name = $name;
-        self::$SHEETS_NAME_USED[$this->index] = $name;
+        self::$SHEETS_NAME_USED[$this->associatedWorkbookId][$this->index] = $name;
 
         return $this;
     }
 
     /**
-     * Returns whether the given sheet's name is valid.
+     * Throws an exception if the given sheet's name is not valid.
      * @see Sheet::setName for validity rules.
      *
      * @param string $name
-     * @return bool TRUE if the name is valid, FALSE otherwise.
+     * @return void
+     * @throws \Box\Spout\Writer\Exception\InvalidSheetNameException If the sheet's name is invalid.
      */
-    protected function isNameValid($name)
+    protected function throwIfNameIsInvalid($name)
     {
         if (!is_string($name)) {
-            return false;
+            $actualType = gettype($name);
+            $errorMessage = "The sheet's name is invalid. It must be a string ($actualType given).";
+            throw new InvalidSheetNameException($errorMessage);
         }
 
+        $failedRequirements = [];
         $nameLength = $this->stringHelper->getStringLength($name);
 
-        return (
-            $nameLength > 0 &&
-            $nameLength <= self::MAX_LENGTH_SHEET_NAME &&
-            !$this->doesContainInvalidCharacters($name) &&
-            $this->isNameUnique($name) &&
-            !$this->doesStartOrEndWithSingleQuote($name)
-        );
+        if (!$this->isNameUnique($name)) {
+            $failedRequirements[] = 'It should be unique';
+        } else {
+            if ($nameLength === 0) {
+                $failedRequirements[] = 'It should not be blank';
+            } else {
+                if ($nameLength > self::MAX_LENGTH_SHEET_NAME) {
+                    $failedRequirements[] = 'It should not exceed 31 characters';
+                }
+
+                if ($this->doesContainInvalidCharacters($name)) {
+                    $failedRequirements[] = 'It should not contain these characters: \\ / ? * : [ or ]';
+                }
+
+                if ($this->doesStartOrEndWithSingleQuote($name)) {
+                    $failedRequirements[] = 'It should not start or end with a single quote';
+                }
+            }
+        }
+
+        if (count($failedRequirements) !== 0) {
+            $errorMessage = "The sheet's name (\"$name\") is invalid. It did not respect these rules:\n - ";
+            $errorMessage .= implode("\n - ", $failedRequirements);
+            throw new InvalidSheetNameException($errorMessage);
+        }
     }
 
     /**
@@ -148,7 +172,7 @@ class Sheet
      */
     protected function isNameUnique($name)
     {
-        foreach (self::$SHEETS_NAME_USED as $sheetIndex => $sheetName) {
+        foreach (self::$SHEETS_NAME_USED[$this->associatedWorkbookId] as $sheetIndex => $sheetName) {
             if ($sheetIndex !== $this->index && $sheetName === $name) {
                 return false;
             }

src/Spout/Writer/ODS/Helper/StyleHelper.php

@@ -214,64 +214,143 @@ EOD;
      */
     protected function getStyleSectionContent($style)
     {
-        $defaultStyle = $this->getDefaultStyle();
         $styleIndex = $style->getId() + 1; // 1-based
 
         $content = '<style:style style:data-style-name="N0" style:family="table-cell" style:name="ce' . $styleIndex . '" style:parent-style-name="Default">';
 
-        if ($style->shouldApplyFont()) {
-            $content .= '<style:text-properties';
-
-            $fontColor = $style->getFontColor();
-            if ($fontColor !== $defaultStyle->getFontColor()) {
-                $content .= ' fo:color="#' . $fontColor . '"';
-            }
-
-            $fontName = $style->getFontName();
-            if ($fontName !== $defaultStyle->getFontName()) {
-                $content .= ' style:font-name="' . $fontName . '" style:font-name-asian="' . $fontName . '" style:font-name-complex="' . $fontName . '"';
-            }
-
-            $fontSize = $style->getFontSize();
-            if ($fontSize !== $defaultStyle->getFontSize()) {
-                $content .= ' fo:font-size="' . $fontSize . 'pt" style:font-size-asian="' . $fontSize . 'pt" style:font-size-complex="' . $fontSize . 'pt"';
-            }
-
-            if ($style->isFontBold()) {
-                $content .= ' fo:font-weight="bold" style:font-weight-asian="bold" style:font-weight-complex="bold"';
-            }
-            if ($style->isFontItalic()) {
-                $content .= ' fo:font-style="italic" style:font-style-asian="italic" style:font-style-complex="italic"';
-            }
-            if ($style->isFontUnderline()) {
-                $content .= ' style:text-underline-style="solid" style:text-underline-type="single"';
-            }
-            if ($style->isFontStrikethrough()) {
-                $content .= ' style:text-line-through-style="solid"';
-            }
-
-            $content .= '/>';
-        }
-
-        if ($style->shouldWrapText()) {
-            $content .= '<style:table-cell-properties fo:wrap-option="wrap" style:vertical-align="automatic"/>';
-        }
-
-        if ($style->shouldApplyBorder()) {
-            $borderProperty = '<style:table-cell-properties %s />';
-            $borders = array_map(function (BorderPart $borderPart) {
-                return BorderHelper::serializeBorderPart($borderPart);
-            }, $style->getBorder()->getParts());
-            $content .= sprintf($borderProperty, implode(' ', $borders));
-        }
-
-        if ($style->shouldApplyBackgroundColor()) {
-            $content .= sprintf('
-                <style:table-cell-properties fo:background-color="#%s"/>', $style->getBackgroundColor());
-        }
+        $content .= $this->getTextPropertiesSectionContent($style);
+        $content .= $this->getTableCellPropertiesSectionContent($style);
 
         $content .= '</style:style>';
 
         return $content;
     }
+
+    /**
+     * Returns the contents of the "<style:text-properties>" section, inside "<style:style>" section
+     *
+     * @param \Box\Spout\Writer\Style\Style $style
+     * @return string
+     */
+    private function getTextPropertiesSectionContent($style)
+    {
+        $content = '';
+
+        if ($style->shouldApplyFont()) {
+            $content .= $this->getFontSectionContent($style);
+        }
+
+        return $content;
+    }
+
+    /**
+     * Returns the contents of the "<style:text-properties>" section, inside "<style:style>" section
+     *
+     * @param \Box\Spout\Writer\Style\Style $style
+     * @return string
+     */
+    private function getFontSectionContent($style)
+    {
+        $defaultStyle = $this->getDefaultStyle();
+
+        $content = '<style:text-properties';
+
+        $fontColor = $style->getFontColor();
+        if ($fontColor !== $defaultStyle->getFontColor()) {
+            $content .= ' fo:color="#' . $fontColor . '"';
+        }
+
+        $fontName = $style->getFontName();
+        if ($fontName !== $defaultStyle->getFontName()) {
+            $content .= ' style:font-name="' . $fontName . '" style:font-name-asian="' . $fontName . '" style:font-name-complex="' . $fontName . '"';
+        }
+
+        $fontSize = $style->getFontSize();
+        if ($fontSize !== $defaultStyle->getFontSize()) {
+            $content .= ' fo:font-size="' . $fontSize . 'pt" style:font-size-asian="' . $fontSize . 'pt" style:font-size-complex="' . $fontSize . 'pt"';
+        }
+
+        if ($style->isFontBold()) {
+            $content .= ' fo:font-weight="bold" style:font-weight-asian="bold" style:font-weight-complex="bold"';
+        }
+        if ($style->isFontItalic()) {
+            $content .= ' fo:font-style="italic" style:font-style-asian="italic" style:font-style-complex="italic"';
+        }
+        if ($style->isFontUnderline()) {
+            $content .= ' style:text-underline-style="solid" style:text-underline-type="single"';
+        }
+        if ($style->isFontStrikethrough()) {
+            $content .= ' style:text-line-through-style="solid"';
+        }
+
+        $content .= '/>';
+
+        return $content;
+    }
+
+    /**
+     * Returns the contents of the "<style:table-cell-properties>" section, inside "<style:style>" section
+     *
+     * @param \Box\Spout\Writer\Style\Style $style
+     * @return string
+     */
+    private function getTableCellPropertiesSectionContent($style)
+    {
+        $content = '';
+
+        if ($style->shouldWrapText()) {
+            $content .= $this->getWrapTextXMLContent();
+        }
+
+        if ($style->shouldApplyBorder()) {
+            $content .= $this->getBorderXMLContent($style);
+        }
+
+        if ($style->shouldApplyBackgroundColor()) {
+            $content .= $this->getBackgroundColorXMLContent($style);
+        }
+
+        return $content;
+    }
+
+    /**
+     * Returns the contents of the wrap text definition for the "<style:table-cell-properties>" section
+     *
+     * @return string
+     */
+    private function getWrapTextXMLContent()
+    {
+        return '<style:table-cell-properties fo:wrap-option="wrap" style:vertical-align="automatic"/>';
+    }
+
+    /**
+     * Returns the contents of the borders definition for the "<style:table-cell-properties>" section
+     *
+     * @param \Box\Spout\Writer\Style\Style $style
+     * @return string
+     */
+    private function getBorderXMLContent($style)
+    {
+        $borderProperty = '<style:table-cell-properties %s />';
+
+        $borders = array_map(function (BorderPart $borderPart) {
+            return BorderHelper::serializeBorderPart($borderPart);
+        }, $style->getBorder()->getParts());
+
+        return sprintf($borderProperty, implode(' ', $borders));
+    }
+
+    /**
+     * Returns the contents of the background color definition for the "<style:table-cell-properties>" section
+     *
+     * @param \Box\Spout\Writer\Style\Style $style
+     * @return string
+     */
+    private function getBackgroundColorXMLContent($style)
+    {
+        return sprintf(
+            '<style:table-cell-properties fo:background-color="#%s"/>',
+            $style->getBackgroundColor()
+        );
+    }
 }

src/Spout/Writer/ODS/Internal/Workbook.php

@@ -69,7 +69,7 @@ class Workbook extends AbstractWorkbook
     public function addNewSheet()
     {
         $newSheetIndex = count($this->worksheets);
-        $sheet = new Sheet($newSheetIndex);
+        $sheet = new Sheet($newSheetIndex, $this->internalId);
 
         $sheetsContentTempFolder = $this->fileSystemHelper->getSheetsContentTempFolder();
         $worksheet = new Worksheet($sheet, $sheetsContentTempFolder);

src/Spout/Writer/Style/Style.php

@@ -106,6 +106,7 @@ class Style
 
     /**
      * @param Border $border
+     * @return Style
      */
     public function setBorder(Border $border)
     {
@@ -359,37 +360,67 @@ class Style
     {
         $mergedStyle = clone $this;
 
-        if (!$this->hasSetFontBold && $baseStyle->isFontBold()) {
-            $mergedStyle->setFontBold();
-        }
-        if (!$this->hasSetFontItalic && $baseStyle->isFontItalic()) {
-            $mergedStyle->setFontItalic();
-        }
-        if (!$this->hasSetFontUnderline && $baseStyle->isFontUnderline()) {
-            $mergedStyle->setFontUnderline();
-        }
-        if (!$this->hasSetFontStrikethrough && $baseStyle->isFontStrikethrough()) {
-            $mergedStyle->setFontStrikethrough();
-        }
-        if (!$this->hasSetFontSize && $baseStyle->getFontSize() !== self::DEFAULT_FONT_SIZE) {
-            $mergedStyle->setFontSize($baseStyle->getFontSize());
-        }
-        if (!$this->hasSetFontColor && $baseStyle->getFontColor() !== self::DEFAULT_FONT_COLOR) {
-            $mergedStyle->setFontColor($baseStyle->getFontColor());
-        }
-        if (!$this->hasSetFontName && $baseStyle->getFontName() !== self::DEFAULT_FONT_NAME) {
-            $mergedStyle->setFontName($baseStyle->getFontName());
-        }
-        if (!$this->hasSetWrapText && $baseStyle->shouldWrapText()) {
-            $mergedStyle->setShouldWrapText();
-        }
-        if (!$this->getBorder() && $baseStyle->shouldApplyBorder()) {
-            $mergedStyle->setBorder($baseStyle->getBorder());
-        }
-        if (!$this->hasSetBackgroundColor && $baseStyle->shouldApplyBackgroundColor()) {
-            $mergedStyle->setBackgroundColor($baseStyle->getBackgroundColor());
-        }
+        $this->mergeFontStyles($mergedStyle, $baseStyle);
+        $this->mergeOtherFontProperties($mergedStyle, $baseStyle);
+        $this->mergeCellProperties($mergedStyle, $baseStyle);
 
         return $mergedStyle;
     }
+
+    /**
+     * @param Style $styleToUpdate (passed as reference)
+     * @param Style $baseStyle
+     * @return void
+     */
+    private function mergeFontStyles($styleToUpdate, $baseStyle)
+    {
+        if (!$this->hasSetFontBold && $baseStyle->isFontBold()) {
+            $styleToUpdate->setFontBold();
+        }
+        if (!$this->hasSetFontItalic && $baseStyle->isFontItalic()) {
+            $styleToUpdate->setFontItalic();
+        }
+        if (!$this->hasSetFontUnderline && $baseStyle->isFontUnderline()) {
+            $styleToUpdate->setFontUnderline();
+        }
+        if (!$this->hasSetFontStrikethrough && $baseStyle->isFontStrikethrough()) {
+            $styleToUpdate->setFontStrikethrough();
+        }
+    }
+
+    /**
+     * @param Style $styleToUpdate Style to update (passed as reference)
+     * @param Style $baseStyle
+     * @return void
+     */
+    private function mergeOtherFontProperties($styleToUpdate, $baseStyle)
+    {
+        if (!$this->hasSetFontSize && $baseStyle->getFontSize() !== self::DEFAULT_FONT_SIZE) {
+            $styleToUpdate->setFontSize($baseStyle->getFontSize());
+        }
+        if (!$this->hasSetFontColor && $baseStyle->getFontColor() !== self::DEFAULT_FONT_COLOR) {
+            $styleToUpdate->setFontColor($baseStyle->getFontColor());
+        }
+        if (!$this->hasSetFontName && $baseStyle->getFontName() !== self::DEFAULT_FONT_NAME) {
+            $styleToUpdate->setFontName($baseStyle->getFontName());
+        }
+    }
+
+    /**
+     * @param Style $styleToUpdate Style to update (passed as reference)
+     * @param Style $baseStyle
+     * @return void
+     */
+    private function mergeCellProperties($styleToUpdate, $baseStyle)
+    {
+        if (!$this->hasSetWrapText && $baseStyle->shouldWrapText()) {
+            $styleToUpdate->setShouldWrapText();
+        }
+        if (!$this->getBorder() && $baseStyle->shouldApplyBorder()) {
+            $styleToUpdate->setBorder($baseStyle->getBorder());
+        }
+        if (!$this->hasSetBackgroundColor && $baseStyle->shouldApplyBackgroundColor()) {
+            $styleToUpdate->setBackgroundColor($baseStyle->getBackgroundColor());
+        }
+    }
 }

src/Spout/Writer/XLSX/Internal/Workbook.php

@@ -83,7 +83,7 @@ class Workbook extends AbstractWorkbook
     public function addNewSheet()
     {
         $newSheetIndex = count($this->worksheets);
-        $sheet = new Sheet($newSheetIndex);
+        $sheet = new Sheet($newSheetIndex, $this->internalId);
 
         $worksheetFilesFolder = $this->fileSystemHelper->getXlWorksheetsFolder();
         $worksheet = new Worksheet($sheet, $worksheetFilesFolder, $this->sharedStringsHelper, $this->styleHelper, $this->shouldUseInlineStrings);

src/Spout/Writer/XLSX/Internal/Worksheet.php

@@ -4,6 +4,7 @@ namespace Box\Spout\Writer\XLSX\Internal;
 
 use Box\Spout\Common\Exception\InvalidArgumentException;
 use Box\Spout\Common\Exception\IOException;
+use Box\Spout\Common\Helper\StringHelper;
 use Box\Spout\Writer\Common\Helper\CellHelper;
 use Box\Spout\Writer\Common\Internal\WorksheetInterface;
 
@@ -16,6 +17,14 @@ use Box\Spout\Writer\Common\Internal\WorksheetInterface;
  */
 class Worksheet implements WorksheetInterface
 {
+    /**
+     * Maximum number of characters a cell can contain
+     * @see https://support.office.com/en-us/article/Excel-specifications-and-limits-16c69c74-3d6a-4aaf-ba35-e6eb276e8eaa [Excel 2007]
+     * @see https://support.office.com/en-us/article/Excel-specifications-and-limits-1672b34d-7043-467e-8e27-269d656771c3 [Excel 2010]
+     * @see https://support.office.com/en-us/article/Excel-specifications-and-limits-ca36e2dc-1f09-4620-b726-67c00b05040f [Excel 2013/2016]
+     */
+    const MAX_CHARACTERS_PER_CELL = 32767;
+
     const SHEET_XML_FILE_HEADER = <<<EOD
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships">
@@ -39,6 +48,9 @@ EOD;
     /** @var \Box\Spout\Common\Escaper\XLSX Strings escaper */
     protected $stringsEscaper;
 
+    /** @var \Box\Spout\Common\Helper\StringHelper String helper */
+    protected $stringHelper;
+
     /** @var Resource Pointer to the sheet data file (e.g. xl/worksheets/sheet1.xml) */
     protected $sheetFilePointer;
 
@@ -62,6 +74,7 @@ EOD;
 
         /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
         $this->stringsEscaper = \Box\Spout\Common\Escaper\XLSX::getInstance();
+        $this->stringHelper = new StringHelper();
 
         $this->worksheetFilePath = $worksheetFilesFolder . '/' . strtolower($this->externalSheet->getName()) . '.xml';
         $this->startSheet();
@@ -149,7 +162,8 @@ EOD;
     private function isEmptyRow($dataRow)
     {
         $numCells = count($dataRow);
-        return ($numCells === 1 && CellHelper::isEmpty($dataRow[0]));
+        // using "reset()" instead of "$dataRow[0]" because $dataRow can be an associative array
+        return ($numCells === 1 && CellHelper::isEmpty(reset($dataRow)));
     }
 
     /**
@@ -191,7 +205,7 @@ EOD;
      * @param mixed $cellValue
      * @param int $styleId
      * @return string
-     * @throws InvalidArgumentException
+     * @throws InvalidArgumentException If the given value cannot be processed
      */
     private function getCellXML($rowIndex, $cellNumber, $cellValue, $styleId)
     {
@@ -200,12 +214,7 @@ EOD;
         $cellXML .= ' s="' . $styleId . '"';
 
         if (CellHelper::isNonEmptyString($cellValue)) {
-            if ($this->shouldUseInlineStrings) {
-                $cellXML .= ' t="inlineStr"><is><t>' . $this->stringsEscaper->escape($cellValue) . '</t></is></c>';
-            } else {
-                $sharedStringId = $this->sharedStringsHelper->writeString($cellValue);
-                $cellXML .= ' t="s"><v>' . $sharedStringId . '</v></c>';
-            }
+            $cellXML .= $this->getCellXMLFragmentForNonEmptyString($cellValue);
         } else if (CellHelper::isBoolean($cellValue)) {
             $cellXML .= ' t="b"><v>' . intval($cellValue) . '</v></c>';
         } else if (CellHelper::isNumeric($cellValue)) {
@@ -225,6 +234,29 @@ EOD;
         return $cellXML;
     }
 
+    /**
+     * Returns the XML fragment for a cell containing a non empty string
+     *
+     * @param string $cellValue The cell value
+     * @return string The XML fragment representing the cell
+     * @throws InvalidArgumentException If the string exceeds the maximum number of characters allowed per cell
+     */
+    private function getCellXMLFragmentForNonEmptyString($cellValue)
+    {
+        if ($this->stringHelper->getStringLength($cellValue) > self::MAX_CHARACTERS_PER_CELL) {
+            throw new InvalidArgumentException('Trying to add a value that exceeds the maximum number of characters allowed in a cell (32,767)');
+        }
+
+        if ($this->shouldUseInlineStrings) {
+            $cellXMLFragment = ' t="inlineStr"><is><t>' . $this->stringsEscaper->escape($cellValue) . '</t></is></c>';
+        } else {
+            $sharedStringId = $this->sharedStringsHelper->writeString($cellValue);
+            $cellXMLFragment = ' t="s"><v>' . $sharedStringId . '</v></c>';
+        }
+
+        return $cellXMLFragment;
+    }
+
     /**
      * Closes the worksheet
      *

tests/Spout/Common/Escaper/ODSTest.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace Box\Spout\Common\Escaper;
+
+/**
+ * Class ODSTest
+ *
+ * @package Box\Spout\Common\Escaper
+ */
+class ODSTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @return array
+     */
+    public function dataProviderForTestEscape()
+    {
+        return [
+            ['test', 'test'],
+            ['carl\'s "pokemon"', 'carl&#039;s &quot;pokemon&quot;'],
+            ["\n", "\n"],
+            ["\r", "\r"],
+            ["\t", "\t"],
+            ["\v", "�"],
+            ["\f", "�"],
+        ];
+    }
+
+    /**
+     * @dataProvider dataProviderForTestEscape
+     *
+     * @param string $stringToEscape
+     * @param string $expectedEscapedString
+     * @return void
+     */
+    public function testEscape($stringToEscape, $expectedEscapedString)
+    {
+        $escaper = \Box\Spout\Common\Escaper\ODS::getInstance();
+        $escapedString = $escaper->escape($stringToEscape);
+
+        $this->assertEquals($expectedEscapedString, $escapedString, 'Incorrect escaped string');
+    }
+}

tests/Spout/Reader/ODS/SheetTest.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace Box\Spout\Reader\ODS;
+
+use Box\Spout\Common\Type;
+use Box\Spout\Reader\ReaderFactory;
+use Box\Spout\TestUsingResource;
+
+/**
+ * Class SheetTest
+ *
+ * @package Box\Spout\Reader\ODS
+ */
+class SheetTest extends \PHPUnit_Framework_TestCase
+{
+    use TestUsingResource;
+
+    /**
+     * @return void
+     */
+    public function testNextSheetShouldReturnCorrectSheetInfos()
+    {
+        $sheets = $this->openFileAndReturnSheets('two_sheets_with_custom_names.ods');
+
+        $this->assertEquals('Sheet First', $sheets[0]->getName());
+        $this->assertEquals(0, $sheets[0]->getIndex());
+
+        $this->assertEquals('Sheet Last', $sheets[1]->getName());
+        $this->assertEquals(1, $sheets[1]->getIndex());
+    }
+
+    /**
+     * @param string $fileName
+     * @return Sheet[]
+     */
+    private function openFileAndReturnSheets($fileName)
+    {
+        $resourcePath = $this->getResourcePath($fileName);
+        $reader = ReaderFactory::create(Type::ODS);
+        $reader->open($resourcePath);
+
+        $sheets = [];
+        foreach ($reader->getSheetIterator() as $sheet) {
+            $sheets[] = $sheet;
+        }
+
+        $reader->close();
+
+        return $sheets;
+    }
+}

tests/Spout/Reader/XLSX/Helper/CellHelperTest.php

@@ -10,14 +10,25 @@ namespace Box\Spout\Reader\XLSX\Helper;
 class CellHelperTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * @return void
+     * @return array
      */
-    public function testFillMissingArrayIndexes()
+    public function dataProviderForTestFillMissingArrayIndexes()
     {
-        $arrayToFill = [1 => 1, 3 => 3];
-        $filledArray = CellHelper::fillMissingArrayIndexes($arrayToFill, 'FILL');
+        return [
+            [ null, [] ],
+            [ [], [] ],
+            [ [1 => 1, 3 => 3], ['FILL', 1, 'FILL', 3] ]
+        ];
+    }
 
-        $expectedFilledArray = ['FILL', 1, 'FILL', 3];
+    /**
+     * @dataProvider dataProviderForTestFillMissingArrayIndexes
+     * @param array $arrayToFill
+     * @param array $expectedFilledArray
+     */
+    public function testFillMissingArrayIndexes($arrayToFill, array $expectedFilledArray)
+    {
+        $filledArray = CellHelper::fillMissingArrayIndexes($arrayToFill, 'FILL');
         $this->assertEquals($expectedFilledArray, $filledArray);
     }
 

tests/Spout/Writer/CSV/WriterTest.php

@@ -61,6 +61,23 @@ class WriterTest extends \PHPUnit_Framework_TestCase
         $writer->close();
     }
 
+    /**
+     * @return void
+     */
+    public function testCloseShouldNoopWhenWriterIsNotOpened()
+    {
+        $fileName = 'test_double_close_calls.csv';
+        $this->createGeneratedFolderIfNeeded($fileName);
+        $resourcePath = $this->getGeneratedResourcePath($fileName);
+
+        $writer = WriterFactory::create(Type::CSV);
+        $writer->close(); // This call should not cause any error
+
+        $writer->openToFile($fileName);
+        $writer->close();
+        $writer->close(); // This call should not cause any error
+    }
+
     /**
      * @return void
      */

tests/Spout/Writer/Common/SheetTest.php

@@ -14,7 +14,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
      */
     public function testGetSheetName()
     {
-        $sheets = [new Sheet(0), new Sheet(1)];
+        $sheets = [new Sheet(0, 'workbookId1'), new Sheet(1, 'workbookId1')];
 
         $this->assertEquals('Sheet1', $sheets[0]->getName(), 'Invalid name for the first sheet');
         $this->assertEquals('Sheet2', $sheets[1]->getName(), 'Invalid name for the second sheet');
@@ -26,7 +26,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
     public function testSetSheetNameShouldCreateSheetWithCustomName()
     {
         $customSheetName = 'CustomName';
-        $sheet = new Sheet(0);
+        $sheet = new Sheet(0, 'workbookId1');
         $sheet->setName($customSheetName);
 
         $this->assertEquals($customSheetName, $sheet->getName(), "The sheet name should have been changed to '$customSheetName'");
@@ -63,7 +63,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
      */
     public function testSetSheetNameShouldThrowOnInvalidName($customSheetName)
     {
-        (new Sheet(0))->setName($customSheetName);
+        (new Sheet(0, 'workbookId1'))->setName($customSheetName);
     }
 
     /**
@@ -72,7 +72,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
     public function testSetSheetNameShouldNotThrowWhenSettingSameNameAsCurrentOne()
     {
         $customSheetName = 'Sheet name';
-        $sheet = new Sheet(0);
+        $sheet = new Sheet(0, 'workbookId1');
         $sheet->setName($customSheetName);
         $sheet->setName($customSheetName);
     }
@@ -85,10 +85,27 @@ class SheetTest extends \PHPUnit_Framework_TestCase
     {
         $customSheetName = 'Sheet name';
 
-        $sheet = new Sheet(0);
+        $sheet = new Sheet(0, 'workbookId1');
         $sheet->setName($customSheetName);
 
-        $sheet = new Sheet(1);
+        $sheet = new Sheet(1, 'workbookId1');
+        $sheet->setName($customSheetName);
+    }
+
+    /**
+     * @return void
+     */
+    public function testSetSheetNameShouldNotThrowWhenSameNameUsedInDifferentWorkbooks()
+    {
+        $customSheetName = 'Sheet name';
+
+        $sheet = new Sheet(0, 'workbookId1');
+        $sheet->setName($customSheetName);
+
+        $sheet = new Sheet(0, 'workbookId2');
+        $sheet->setName($customSheetName);
+
+        $sheet = new Sheet(1, 'workbookId3');
         $sheet->setName($customSheetName);
     }
 }

tests/Spout/Writer/ODS/WriterTest.php

@@ -67,7 +67,7 @@ class WriterTest extends \PHPUnit_Framework_TestCase
     /**
      * @expectedException \Box\Spout\Writer\Exception\WriterAlreadyOpenedException
      */
-    public function testsetShouldCreateNewSheetsAutomaticallyShouldThrowExceptionIfCalledAfterOpeningWriter()
+    public function testSetShouldCreateNewSheetsAutomaticallyShouldThrowExceptionIfCalledAfterOpeningWriter()
     {
         $fileName = 'file_that_wont_be_written.ods';
         $filePath = $this->getGeneratedResourcePath($fileName);
@@ -168,6 +168,23 @@ class WriterTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($firstSheet, $writer->getCurrentSheet(), 'The current sheet should be the first one.');
     }
 
+    /**
+     * @return void
+     */
+    public function testCloseShouldNoopWhenWriterIsNotOpened()
+    {
+        $fileName = 'test_double_close_calls.ods';
+        $this->createGeneratedFolderIfNeeded($fileName);
+        $resourcePath = $this->getGeneratedResourcePath($fileName);
+
+        $writer = WriterFactory::create(Type::ODS);
+        $writer->close(); // This call should not cause any error
+
+        $writer->openToFile($fileName);
+        $writer->close();
+        $writer->close(); // This call should not cause any error
+    }
+
     /**
      * @return void
      */

tests/Spout/Writer/XLSX/WriterTest.php

@@ -6,6 +6,7 @@ use Box\Spout\Common\Exception\SpoutException;
 use Box\Spout\Common\Type;
 use Box\Spout\TestUsingResource;
 use Box\Spout\Writer\WriterFactory;
+use Box\Spout\Writer\XLSX\Internal\Worksheet;
 
 /**
  * Class WriterTest
@@ -98,6 +99,19 @@ class WriterTest extends \PHPUnit_Framework_TestCase
     public function testAddRowShouldThrowExceptionIfUnsupportedDataTypePassedIn()
     {
         $fileName = 'test_add_row_should_throw_exception_if_unsupported_data_type_passed_in.xlsx';
+        $dataRows = [
+            [str_repeat('a', Worksheet::MAX_CHARACTERS_PER_CELL + 1)],
+        ];
+
+        $this->writeToXLSXFile($dataRows, $fileName);
+    }
+
+    /**
+     * @expectedException \Box\Spout\Common\Exception\InvalidArgumentException
+     */
+    public function testAddRowShouldThrowExceptionIfWritingStringExceedingMaxNumberOfCharactersAllowedPerCell()
+    {
+        $fileName = 'test_add_row_should_throw_exception_if_string_exceeds_max_num_chars_allowed_per_cell.xlsx';
         $dataRows = [
             [new \stdClass()],
         ];
@@ -180,6 +194,23 @@ class WriterTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($firstSheet, $writer->getCurrentSheet(), 'The current sheet should be the first one.');
     }
 
+    /**
+     * @return void
+     */
+    public function testCloseShouldNoopWhenWriterIsNotOpened()
+    {
+        $fileName = 'test_double_close_calls.xlsx';
+        $this->createGeneratedFolderIfNeeded($fileName);
+        $resourcePath = $this->getGeneratedResourcePath($fileName);
+
+        $writer = WriterFactory::create(Type::XLSX);
+        $writer->close(); // This call should not cause any error
+
+        $writer->openToFile($fileName);
+        $writer->close();
+        $writer->close(); // This call should not cause any error
+    }
+
     /**
      * @return void
      */
@@ -294,7 +325,7 @@ class WriterTest extends \PHPUnit_Framework_TestCase
         $dataRows = [
             [''],
             ['xlsx--21', 'xlsx--22'],
-            [''],
+            ['key' => ''],
             [''],
             ['xlsx--51', 'xlsx--52'],
         ];