diff --git a/src/EPPlus/Core/Worksheet/WorksheetCopyHelper.cs b/src/EPPlus/Core/Worksheet/WorksheetCopyHelper.cs index 5f0da53a6..45f5d1e93 100644 --- a/src/EPPlus/Core/Worksheet/WorksheetCopyHelper.cs +++ b/src/EPPlus/Core/Worksheet/WorksheetCopyHelper.cs @@ -1427,15 +1427,10 @@ private static void CopyThreadedComments(ExcelWorksheet copy, ExcelWorksheet add private static void CopyHeaderFooterPictures(ExcelWorksheet Copy, ExcelWorksheet added) { if (Copy.TopNode != null && Copy.GetNode("d:headerFooter") == null) return; - //Copy the texts - if (Copy.HeaderFooter._oddHeader != null) CopyText(Copy.HeaderFooter._oddHeader, added.HeaderFooter.OddHeader); - if (Copy.HeaderFooter._oddFooter != null) CopyText(Copy.HeaderFooter._oddFooter, added.HeaderFooter.OddFooter); - if (Copy.HeaderFooter._evenHeader != null) CopyText(Copy.HeaderFooter._evenHeader, added.HeaderFooter.EvenHeader); - if (Copy.HeaderFooter._evenFooter != null) CopyText(Copy.HeaderFooter._evenFooter, added.HeaderFooter.EvenFooter); - if (Copy.HeaderFooter._firstHeader != null) CopyText(Copy.HeaderFooter._firstHeader, added.HeaderFooter.FirstHeader); - if (Copy.HeaderFooter._firstFooter != null) CopyText(Copy.HeaderFooter._firstFooter, added.HeaderFooter.FirstFooter); - //Copy any images; + //Copy any images first, so the pictures exist on the target before the + //header/footer text is parsed. The text may contain the image code (&G), + //and parsing it reads added.HeaderFooter.Pictures. if (Copy.HeaderFooter.Pictures.Count > 0) { Uri source = Copy.HeaderFooter.Pictures.Uri; @@ -1456,11 +1451,26 @@ private static void CopyHeaderFooterPictures(ExcelWorksheet Copy, ExcelWorksheet } foreach (XmlAttribute att in pic.TopNode.Attributes) { + // Skip attributes that Pictures.Add/AddImage already set on the new + // shape node, otherwise we get duplicates (e.g. a second "type" + // attribute, which produces invalid VML that cannot be reopened). + if (att.LocalName == "id" || att.LocalName == "type" || att.LocalName == "style") + { + continue; + } (item.TopNode as XmlElement).SetAttribute(att.Name, att.Value); } item.TopNode.InnerXml = pic.TopNode.InnerXml; } } + + //Copy the texts + if (Copy.HeaderFooter._oddHeader != null) CopyText(Copy.HeaderFooter._oddHeader, added.HeaderFooter.OddHeader); + if (Copy.HeaderFooter._oddFooter != null) CopyText(Copy.HeaderFooter._oddFooter, added.HeaderFooter.OddFooter); + if (Copy.HeaderFooter._evenHeader != null) CopyText(Copy.HeaderFooter._evenHeader, added.HeaderFooter.EvenHeader); + if (Copy.HeaderFooter._evenFooter != null) CopyText(Copy.HeaderFooter._evenFooter, added.HeaderFooter.EvenFooter); + if (Copy.HeaderFooter._firstHeader != null) CopyText(Copy.HeaderFooter._firstHeader, added.HeaderFooter.FirstHeader); + if (Copy.HeaderFooter._firstFooter != null) CopyText(Copy.HeaderFooter._firstFooter, added.HeaderFooter.FirstFooter); } private static void CopyText(ExcelHeaderFooterText from, ExcelHeaderFooterText to) { diff --git a/src/EPPlusTest/Core/Worksheet/HeaderrFooterTests.cs b/src/EPPlusTest/Core/Worksheet/HeaderrFooterTests.cs new file mode 100644 index 000000000..57273bdcc --- /dev/null +++ b/src/EPPlusTest/Core/Worksheet/HeaderrFooterTests.cs @@ -0,0 +1,85 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; +using OfficeOpenXml; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Text; + +namespace EPPlusTest.Core.Worksheet +{ + [TestClass] + public class HeaderrFooterTests + { + [TestMethod] + public void Issue_CopyWorksheet_HeaderFooter_Throws() + { + // Direct C# translation of the reporter's "Reproduction — throws" script. + // Source is any workbook that has a header/footer picture. + byte[] sourceBytes = CreateWorkbookWithHeaderFooterPicture(); + + using (var sourceStream = new MemoryStream(sourceBytes)) + using (var source = new ExcelPackage(sourceStream)) + { + var sourceSheet = source.Workbook.Worksheets[0]; + + // The trigger: read a HeaderFooter property on the source first. + var trigger = sourceSheet.HeaderFooter.OddFooter.CenteredText; + + using (var target = new ExcelPackage()) + { + // Before the fix: throws NullReferenceException from + // WorksheetCopyHelper.CopyHeaderFooterPictures. + target.Workbook.Worksheets.Add("Copied", sourceSheet); + } + } + } + + [TestMethod] + public void Issue_CopyWorksheet_HeaderFooter_SilentDataLoss() + { + byte[] sourceBytes = CreateWorkbookWithHeaderFooterPicture(); + + byte[] outputBytes; + using (var sourceStream = new MemoryStream(sourceBytes)) + using (var source = new ExcelPackage(sourceStream)) + { + var sourceSheet = source.Workbook.Worksheets[0]; + + // No HeaderFooter read here. + + using (var target = new ExcelPackage()) + { + var copiedSheet = target.Workbook.Worksheets.Add("Copied", sourceSheet); + + Assert.AreEqual(1, copiedSheet.HeaderFooter.Pictures.Count, + "Header/footer picture missing right after copy."); + + outputBytes = target.GetAsByteArray(); + } + } + + // After save + reopen. + using (var reopenStream = new MemoryStream(outputBytes)) + using (var reopen = new ExcelPackage(reopenStream)) + { + var reopenedSheet = reopen.Workbook.Worksheets["Copied"]; + Assert.AreEqual(1, reopenedSheet.HeaderFooter.Pictures.Count, + "Header/footer picture was lost after save and reopen."); + } + } + + private static byte[] CreateWorkbookWithHeaderFooterPicture() + { + // Stands in for the reporter's "/path/to/any-workbook-with-a-footer.xlsx". + using (var source = new ExcelPackage()) + { + var ws = source.Workbook.Worksheets.Add("Sheet1"); + ws.HeaderFooter.OddFooter.CenteredText = "MyFooter"; + ws.HeaderFooter.OddFooter.InsertPicture(Properties.Resources.Test1, PictureAlignment.Centered); + return source.GetAsByteArray(); + } + } + } +}