Bug 60601

Summary: Hyperlinks in original worksheet are not removed if there are no more hyperlinks on write.
Product: POI Reporter: Joachim Piketz <pik>
Component: XSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal Keywords: PatchAvailable
Priority: P2    
Version: 3.16-dev   
Target Milestone: ---   
Hardware: All   
OS: All   

Description Joachim Piketz 2017-01-18 15:05:27 UTC
If a workbook is loaded that contains hyperlinks, and all hyperlinks are removed via cell.removeHyperlink(), the written workbook still contains all hyperlinks.


The following change in XSSFSheet fixes this issue:

        // Now re-generate our CTHyperlinks, if needed
        if(hyperlinks.size() > 0) 
        {
            if(worksheet.getHyperlinks() == null) {
                worksheet.addNewHyperlinks();
            }
            CTHyperlink[] ctHls = new CTHyperlink[hyperlinks.size()];
            for(int i=0; i<ctHls.length; i++) {
                // If our sheet has hyperlinks, have them add
                //  any relationships that they might need
                XSSFHyperlink hyperlink = hyperlinks.get(i);
                hyperlink.generateRelationIfNeeded(getPackagePart());
                // Now grab their underling object
                ctHls[i] = hyperlink.getCTHyperlink();
            }
            worksheet.getHyperlinks().setHyperlinkArray(ctHls);
        }
        else                                  // line added
          worksheet.unsetHyperlinks();        // line added
Comment 1 Javen O'Neal 2017-01-19 07:46:28 UTC
Thanks for finding this edge case.

Fixed on trunk in r1779426. This will be fixed in the next beta release, 3.16 beta 2, and subsequent releases.
Comment 2 Javen O'Neal 2017-01-19 08:10:22 UTC
The proposed fix broke several other unit tests.

I changed the code to remove the hyperlinks from the hyperlink array one at a time. r1779429
Comment 3 Joachim Piketz 2017-01-19 08:29:10 UTC
Your fix causes Excel 2013 to produce the following error on open:

"We found a problem with some content in '...'. Do you want us to try to recover as much as we can? If you trust the soruce of this workbook, click Yes.
Comment 4 Javen O'Neal 2017-01-19 09:02:55 UTC
Looks like removing the hyperlinks one by one then unsetting the hyperlink array after there are no hyperlinks left passes the current POI unit tests.

Could you please check this with Excel 2013 to see if the code no longer produces corrupt Excel files?

r1779437.
Comment 5 Joachim Piketz 2017-01-19 09:09:37 UTC
This fix works now. Thanks.
Comment 6 Joachim Piketz 2017-02-10 07:41:50 UTC
In WildFly 9 the following Exception is thrown:

java.lang.IndexOutOfBoundsException
 at org.apache.xmlbeans.impl.store.Xobj.removeElement(Xobj.java:2206)
 at org.apache.xmlbeans.impl.store.Xobj.remove_element(Xobj.java:2236)
 at org.openxmlformats.schemas.spreadsheetml.x2006.main.impl.CTWorksheetImpl.unsetHyperlinks(Unknown Source)
 ...

But not in WildFly 10!?

However i fixed it by applying an empty try/catch as a hotfix:

            try
            {
              worksheet.unsetHyperlinks();
            }
            catch (Exception ex)
            {
            }
Comment 7 Javen O'Neal 2017-02-10 13:43:47 UTC
I don't like the idea of suppressing exceptions, as they may catch an exception that we didn't intend to check.

Does

if (worksheet.isSetHyperlinks()) {
    worksheet.unsetHylerlinks();
}

Fix the problem on WildFly 9 and 10?
Comment 8 Joachim Piketz 2017-02-16 09:42:35 UTC
We have replaces WF 9 with WF 10 meanwhile and can't reproduce the exception.
Comment 9 Dominik Stadler 2017-02-18 21:42:29 UTC
Mostly fixed as far as I could read from the comments, please report another bug if you still see issues with the isSet/unset-combination.