Skip to content
This repository has been archived by the owner on Mar 9, 2020. It is now read-only.

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

Open
mmoo9154 opened this issue Dec 19, 2019 · 1 comment

Comments

@mmoo9154
Copy link

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.

@vadimkatsman
Copy link

Experienced same exception when changing the border style of the cell for the document generated by DevExpress.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants