System.NullReferenceException on WorkSheet.Cells.Copy() / ExcelBorderItemXml.Copy()

Some spreadsheets cause an unhandled NullReferenceException to escape from WorkSheet.Cells.Copy().

I've attached a zip of a small MSVC C# console app that reproduces the failure. (EPPlus-bug.zip) Unzip into a folder, open, build, and run the project. It should throw an exception at line 24:

    srcSheet.Cells["A2"].Copy(dstSheet.Cells["A1"]);

The root of the error (imho) is that the ExcelBorderItemXml.Copy() implementation dereferences the _color member variable without testing this.Exists, _color==null, or wrapping in an exception handler.

Here is the code from github:

        internal ExcelBorderItemXml Copy()
        {
            ExcelBorderItemXml borderItem = new ExcelBorderItemXml(NameSpaceManager);
            borderItem.Style = _borderStyle;
            borderItem.Color = _color.Copy();
            return borderItem;
        }

The exception is setup by the ExcelBorderXml(nsm, topNode) ctor:

        internal ExcelBorderItemXml(XmlNamespaceManager nsm, XmlNode topNode) :
            base(nsm, topNode)
        {
            if (topNode != null)
            {
                _borderStyle = GetBorderStyle(GetXmlNodeString("@style"));
                _color = new ExcelColorXml(nsm, topNode.SelectSingleNode(_colorPath, nsm));
                Exists = true;
            }
            else
            {
                Exists = false;
            }
        }

Note that if topNode is null, both _borderStyle and _color will be null.

I suspect _border and _color should be set to ExcelBorderStyle.None and new ExcelColorXml(nsm) respectively (similar to the default ExcelBorderItemXml() ctor without a topNode). FWIW, I implemented this change in a local copy I grabbed from github and it eliminated the unhandled exception.

If _color is intended to be able to take on a null value (the code implies otherwise) then the tests for null should be added to the Copy() routine. And, any external use of the Color accessor should protect against null.

If you would like me to generate a pull request with the fix let me know.

-Mark

[1] The src.xlsx was generated by Crystal Reports 2013, for what it's worth. I suspect there may be an error in the spreadsheet's internal XML, but that does not distract from the issue with EPPlus. Src.xlsx is valid output from CR and it opens without error in MS Excel and in LibreOffice.

JanKallman / EPPlus

System.NullReferenceException on WorkSheet.Cells.Copy() / ExcelBorderItemXml.Copy() #597