EPPlus
EPPlus copied to clipboard
Fix generated file corruption for Pivot Tables with a named range as …
We've been having trouble generating some Excel workbooks with all versions after 4.0.5 (1/8/2016, f3e3852c718e0109e7384fa781491dc4cc51a071).
After several hours of research we tracked the issue down to two problems:
- Changeset 92b972b9d8b3d90f3ff263e5cdd1493fea7d747c introduced a bug where, if a row is inserted on a worksheet that has a named range of the type "$C:$F", then EPPlus will redefine the range with an ending address that is greater than ExcelPackage.MaxRows.
The problem is that ExcelWorksheet.InsertRow() calls ExcelNamedRangeCollection.InsertRows(), which redefines the named range with a new address namedRange.Start.Row + rows
without checking if this will result in an invalid address. I've added some rudimentary checks but a better solution would be to return from ExcelNamedRangeCollection.InsertRows() if the named range is of the type "$C:$F" (i.e. a named column range). However, I don't see how this could be done easily, because ExcelNamedRange does not have any property like "IsColumnReference" (or whatever). The same holds for InsertColumns.
In any case the fix works.
- Changeset 7b78083d1a0060dae2e12ff642dbab2fe704c60f introduced a bug where, if a pivot table refers to a named range, then the "SourceNamePath" is replaced with a "SourceAddressPath", leading to a corrupt workbook.
The problem is in ExcelWorksheet.SavePivotTables(), with this check:
if (pt.CacheDefinition.SourceRange.IsName)
This returns "false" even if the pivot table's source range is an ExcelNamedRange, because ExcelNamedRange.IsName only returns true if the named range refers to another named range (is that even possible?).
I've replaced this with a more exact condition, which fixes the problem:
if (pt.CacheDefinition.SourceRange is ExcelNamedRange && !string.IsNullOrEmpty(((ExcelNamedRange)pt.CacheDefinition.SourceRange).Name))
But again, there may be a better approach.
I've not added any unit tests as I've already spend 7+ hours tracking down these problems and am unfamiliar with how unit tests work on this project. The work was made more difficult by changeset 7b78083d1a0060dae2e12ff642dbab2fe704c60f which basically includes all files in the solution (with a minor change in the header) PLUS the one breaking change described in 2) above.
I believe this fix should help some of the folks who've commented on https://github.com/JanKallman/EPPlus/issues/38
How is work progressing on this issue? I really look forward to these corrections.
I cannot do any more than I already have in regards to this issue. @JanKallman would need to review and merge it into master if he approves of the fixes.