Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get a copy of the CT_Border when about to mutate it #519

Merged
merged 1 commit into from
Aug 24, 2021

Conversation

jonopare
Copy link

GetCTBorder() returns a shared instance when applyBorder is true. When GetCTBorder() is invoked from inside a property setter, that shared instance is then mutated and all other XSSFCellStyle instances that referred to the original (before mutation) CT_Border are changed as well. This appears to be unintentional. If we know we are inside a setter (and therefore about to mutate the state of the CT_Border) then we should perform that mutation on a copy.

GetCTBorder() returns a shared instance when applyBorder is true. When GetCTBorder() is invoked from inside a property setter, that shared instance is then mutated and all other XSSFCellStyle instances that referred to the original (before mutation) CT_Border are changed as well. This appears to be unintentional. If we know we are inside a setter (and therefore about to mutate the state of the CT_Border) then we should perform that mutation on a copy.
@jonopare
Copy link
Author

This change resolves #288 and #499

@jonopare
Copy link
Author

This bug can be demonstrated with the following code snippet:

var workbook = new XSSFWorkbook();

var sheet = workbook.CreateSheet("Bug");

var row = sheet.CreateRow(0);

XSSFCell cell;
XSSFCellStyle cellStyle;

cell = (XSSFCell)row.CreateCell(0);
cellStyle = (XSSFCellStyle)workbook.CreateCellStyle();
cellStyle.BorderLeft = BorderStyle.Thin;
cell.CellStyle = cellStyle;
cell.SetCellValue("left:thin");

cell = (XSSFCell)row.CreateCell(1);
cellStyle = (XSSFCellStyle)workbook.CreateCellStyle();
cellStyle.BorderLeft = BorderStyle.Thin;
cellStyle.BorderBottom = BorderStyle.Double;
cell.CellStyle = cellStyle;
cell.SetCellValue("left:thin;bottom:double");

cell = (XSSFCell)row.CreateCell(2);
cellStyle = (XSSFCellStyle)workbook.CreateCellStyle();
cellStyle.BorderLeft = BorderStyle.Thin;
cellStyle.BorderBottom = BorderStyle.Double;
cellStyle.SetBorderColor(BorderSide.LEFT, new XSSFColor(IndexedColors.Red));
cell.CellStyle = cellStyle;
cell.SetCellValue("left:thin+red;bottom:double");

using (var stream = File.Create($"out-{DateTime.Now:yyyyMMddHHmmss}.xlsx"))
{
	workbook.Write(stream);
}

@tonyqus tonyqus added this to the NPOI 2.5.5 milestone May 26, 2021
@tonyqus
Copy link
Member

tonyqus commented Jun 22, 2021

Your fix works in this case. But I worry that this change may cause huge number of unnecessary CTBorder clone for any sub-properties settings on border.

@tonyqus tonyqus changed the base branch from master to copyStyles August 24, 2021 21:13
@tonyqus tonyqus merged commit 52db178 into nissl-lab:copyStyles Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants