Fix CopyFromInternal NullReferenceException on empty otherCell. #1461

NicholasNoise · 2020-06-10T08:51:24Z

I have upgraded to latest versions ClosedXML.Signed (from 0.95.0-beta1 to 0.95.3) and ClosedXML.Report (from 0.2.0-beta1 to 0.2.1). One of my test fails on generation of template with NRE with this stack trace:

2020-06-10T05:52:34.4899081Z       System.NullReferenceException : Object reference not set to an instance of an object
2020-06-10T05:52:34.4902904Z       Stack Trace:
2020-06-10T05:52:34.4903680Z           at ClosedXML.Excel.XLCell.CopyValuesFrom (ClosedXML.Excel.XLCell source) [0x00000] in <97c12c3cb9f34e07a7b9d562cbce5f3c>:0 
2020-06-10T05:52:34.4904626Z           at ClosedXML.Excel.XLCell.CopyFromInternal (ClosedXML.Excel.XLCell otherCell, ClosedXML.Excel.XLCellCopyOptions options) [0x00013] in <97c12c3cb9f34e07a7b9d562cbce5f3c>:0 
2020-06-10T05:52:34.4905631Z           at ClosedXML.Excel.XLCell.CopyFrom (ClosedXML.Excel.IXLCell otherCell, ClosedXML.Excel.XLCellCopyOptions options) [0x00007] in <97c12c3cb9f34e07a7b9d562cbce5f3c>:0 
2020-06-10T05:52:34.4906799Z           at ClosedXML.Excel.XLCell.CopyFrom (ClosedXML.Excel.IXLCell otherCell) [0x00000] in <97c12c3cb9f34e07a7b9d562cbce5f3c>:0 
2020-06-10T05:52:34.4907695Z           at ClosedXML.Report.Excel.TempSheetBuffer.WriteValue (System.Object value, ClosedXML.Excel.IXLCell settingCell) [0x00018] in <76c47c034f524695b27ad1ce94c1a128>:0 
2020-06-10T05:52:34.4909001Z           at ClosedXML.Report.RangeTemplate.RenderCell (ClosedXML.Report.FormulaEvaluator evaluator, ClosedXML.Report.TemplateCell cell, ClosedXML.Report.Parameter[] pars) [0x00152] in <76c47c034f524695b27ad1ce94c1a128>:0 
2020-06-10T05:52:34.4910065Z           at ClosedXML.Report.RangeTemplate.VerticalTable (System.Object[] items, ClosedXML.Report.FormulaEvaluator evaluator) [0x001b0] in <76c47c034f524695b27ad1ce94c1a128>:0 
2020-06-10T05:52:34.4911167Z           at ClosedXML.Report.RangeTemplate.Generate (System.Object[] items) [0x0007f] in <76c47c034f524695b27ad1ce94c1a128>:0 
2020-06-10T05:52:34.4912201Z           at ClosedXML.Report.RangeInterpreter.EvaluateValues (ClosedXML.Excel.IXLRange range, ClosedXML.Report.Parameter[] pars) [0x0039d] in <76c47c034f524695b27ad1ce94c1a128>:0 
2020-06-10T05:52:34.4913113Z           at ClosedXML.Report.RangeInterpreter.Evaluate (ClosedXML.Excel.IXLRange range) [0x0001c] in <76c47c034f524695b27ad1ce94c1a128>:0 
2020-06-10T05:52:34.4913880Z           at ClosedXML.Report.XLTemplate.Generate () [0x00055] in <76c47c034f524695b27ad1ce94c1a128>:0 
2020-06-10T05:52:34.4915167Z           at Iis.Eais.Payments.Reporting.ReportBuilders.OtchetPoPredostavleniiuMerSotcialnoiPodderzhkiGrazhdanamPodvergshimsiaVozdeistviiuRadiatciiBuilder.FormExcel (Iis.Eais.Payments.Reporting.DTO.ExportOtchetPoPredostavleniiuMerSotcialnoiPodderzhkiGrazhdanamPodvergshimsiaVozdeistviiuRadiatciiBuilderParameters param) [0x00763] in <d22116a551c040f681f2967d88b9abf6>:0 
2020-06-10T05:52:34.4916982Z           at Iis.Eais.Payments.Reporting.ReportBuilders.OtchetPoPredostavleniiuMerSotcialnoiPodderzhkiGrazhdanamPodvergshimsiaVozdeistviiuRadiatciiBuilder.Build (Iis.Eais.Payments.Reporting.DTO.ExportOtchetPoPredostavleniiuMerSotcialnoiPodderzhkiGrazhdanamPodvergshimsiaVozdeistviiuRadiatciiBuilderParameters param, System.Threading.CancellationToken ct) [0x00008] in <d22116a551c040f681f2967d88b9abf6>:0 
2020-06-10T05:52:34.4918542Z           at Iis.Eais.Reporting.IntegratedTests.Extensions.ReportBuilderExtensions.BuildWithTimer[TP] (Iis.Eais.Common.Interfaces.IReportBuilder`1[T] instance, TP param, Xunit.Abstractions.ITestOutputHelper output) [0x00087] in <2ddf1e8203e249f592b217c625448cde>:0 
2020-06-10T05:52:34.4919823Z           at Iis.Eais.Reporting.IntegratedTests.Build.PaymentsBuildTests.OtchetPoPredostavleniiuMerSotcialnoiPodderzhkiGrazhdanamPodvergshimsiaVozdeistviiuRadiatciiBuilderShouldBuild () [0x000d4] in <2ddf1e8203e249f592b217c625448cde>:0 
2020-06-10T05:52:34.4920891Z           at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) [0x00000] in <d2ec5c92492f4d6ba8c422bdf574b786>:0

I cant provide any additional info at this point of time.

igitur · 2020-06-10T09:08:37Z

Thanks. Please add a unit test.

NicholasNoise · 2020-06-10T09:48:10Z

@igitur, Thank for response!
Test CopyFromInternal with null parameter?

NicholasNoise · 2020-06-16T04:19:22Z

@igitur, hi!
Test has been added.
Is it possible to release a nuget package with this patch?

NicholasNoise · 2020-06-17T09:05:26Z

@igitur @b0bi79 @Pankraty
Response is needed please.

b0bi79 · 2020-06-17T10:22:57Z

@NicholasNoise I will research this until the end of the week.

igitur · 2020-06-18T05:43:54Z

ClosedXML_Tests/Excel/Cells/XLCellTests.cs

@@ -1083,5 +1083,24 @@ public void TryGetValueFormulaEvaluation()
                Assert.IsTrue(A3.TryGetValue(out String _));
            }
        }
+
+        [Test]
+        public void CopyFromInternalNullTest()


I'm still curious as to what usage of the public API led to the exposure of the bug. The use of internals in the test project isn't disallowed, but ideally we want a test using only the public API. Maybe we should discuss in the main thread first. If you can show me the code sample to reproduce the issue, that would be great.

Yesterday I have linked PR and also @b0bi79 has linked an issue in ClosedXML.Report.
Maybe throwing exception on null otherCell is better, but I am not too familiar with other usage scenarios.

NRE or other unhandled exceptions are no good for the internals nor the public API.
I think of this PR as a declaration of behavior: do nothing or throw ArgumentNullException.
Of course, the decision is up to you.

igitur · 2020-06-18T06:40:41Z

@igitur @b0bi79 @Pankraty
Response is needed please.

Your patience is appreciated. We're a small team with day jobs and families.

Fix CopyFromInternal NullReferenceException on empty otherCell.

c40fb17

Add NRE test CopyFromInternal

97f7ede

NicholasNoise mentioned this pull request Jun 16, 2020

Test inner range null reference exception. ClosedXML/ClosedXML.Report#169

Closed

b0bi79 mentioned this pull request Jun 17, 2020

System.NullReferenceException ClosedXML/ClosedXML.Report#170

Closed

igitur reviewed Jun 18, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix CopyFromInternal NullReferenceException on empty otherCell. #1461

Fix CopyFromInternal NullReferenceException on empty otherCell. #1461

NicholasNoise commented Jun 10, 2020 •

edited

igitur commented Jun 10, 2020

NicholasNoise commented Jun 10, 2020

NicholasNoise commented Jun 16, 2020

NicholasNoise commented Jun 17, 2020

b0bi79 commented Jun 17, 2020

igitur Jun 18, 2020

NicholasNoise Jun 18, 2020

NicholasNoise Jun 18, 2020

igitur commented Jun 18, 2020

Fix CopyFromInternal NullReferenceException on empty otherCell. #1461

Are you sure you want to change the base?

Fix CopyFromInternal NullReferenceException on empty otherCell. #1461

Conversation

NicholasNoise commented Jun 10, 2020 • edited

igitur commented Jun 10, 2020

NicholasNoise commented Jun 10, 2020

NicholasNoise commented Jun 16, 2020

NicholasNoise commented Jun 17, 2020

b0bi79 commented Jun 17, 2020

igitur Jun 18, 2020

Choose a reason for hiding this comment

NicholasNoise Jun 18, 2020

Choose a reason for hiding this comment

NicholasNoise Jun 18, 2020

Choose a reason for hiding this comment

igitur commented Jun 18, 2020

NicholasNoise commented Jun 10, 2020 •

edited