Border style unintentionally shared between CellStyle instances

[vpstuart]

I have found in an application that the border formatting in XSSFCellStyle is a bit broken. It results in accidentally sharing a CT_Border instance.

From https://github.com/tonyqus/npoi/blob/master/ooxml/XSSF/UserModel/XSSFCellStyle.cs#L252

        public BorderStyle BorderBottom
        {
            get
            {
                if (!_cellXf.applyBorder) return BorderStyle.None;

                int idx = (int)_cellXf.borderId;
                CT_Border ct = _stylesSource.GetBorderAt(idx).GetCTBorder();
                if (!ct.IsSetBottom())
                {
                    return BorderStyle.None;
                }
                else
                {
                    return (BorderStyle)ct.bottom.style;
                }
            }
            set
            {
                CT_Border ct = GetCTBorder();
                CT_BorderPr pr = ct.IsSetBottom() ? ct.bottom : ct.AddNewBottom();
                if (value == BorderStyle.None) 
                    ct.UnsetBottom();
                else 
                    pr.style = (ST_BorderStyle)value;

                int idx = _stylesSource.PutBorder(new XSSFCellBorder(ct, _theme));

                _cellXf.borderId = (uint)idx;
                _cellXf.applyBorder = (true);
            }
        }

Each of the border setters has the same but, I have chosen BorderBottom as an example.

The first time you assign a value to a border property. E.g. cell.BorderBottom = BorderStyle.Thin; it will create a CT_Border object, assign the border to it and then call _stylesSource.PutBorder(new XSSFCellBorder(ct, _theme)) which will store the border object and return an ID.

If it finds a matching CT_Border it will return that instances id, causing it to be shared.

This seems to be from here:
https://github.com/tonyqus/npoi/blob/master/ooxml/XSSF/Model/StylesTable.cs#L285

        public int PutBorder(XSSFCellBorder border)
        {
            int idx = borders.IndexOf(border);
            if (idx != -1)
            {
                return idx;
            }
            borders.Add(border);
            border.SetThemesTable(theme);
            return borders.Count - 1;
        }

Using borders.IndexOf(border) will search by the current value of the border. The XSSFCellBorder class implements equality based on the XML value of the CT_Border.

I think the easiest and possibly best solution would be to delete the 5 lines:

            int idx = borders.IndexOf(border);
            if (idx != -1)
            {
                return idx;
            }

I'm not a regular contributer here, but if it would help I can try and put together a PR.

Warm Regards,
Stuart

[vpstuart]

On second look that will result in the pool of borders growing each time. Perhaps instead CT_Border ct = GetCTBorder(); needs to always make a clone.

[tonyqus]

@asypost After review, your issue is mainly about the CopyRowTo. I suggest you create a new issue for your case.

[yorban]

My team using NPOI has also experienced many headaches due to this issue.

[tonyqus]

Can you test NPOI 2.5.5? I think this was somewhat fixed. @yorban