Uploaded image for project: 'FOP'
  1. FOP
  2. FOP-2461

NullPointerException in ListItemLayoutManager since Temp_BasicSideFloat Merge

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: trunk
    • Fix Version/s: 2.1
    • Component/s: layout/unqualified
    • Labels:
      None

      Description

      The attached fo test cases causes a NullPointerException inside ListItemLayoutManager. This is the stack trace:

      java.lang.NullPointerException
      at org.apache.fop.layoutmgr.list.ListItemLayoutManager.getCombinedKnuthElementsForListItem(ListItemLayoutManager.java:405)
      at org.apache.fop.layoutmgr.list.ListItemLayoutManager.getNextKnuthElements(ListItemLayoutManager.java:326)
      at org.apache.fop.layoutmgr.BlockStackingLayoutManager.getNextKnuthElements(BlockStackingLayoutManager.java:239)
      at org.apache.fop.layoutmgr.BlockStackingLayoutManager.getNextChildElements(BlockStackingLayoutManager.java:498)
      at org.apache.fop.layoutmgr.BlockStackingLayoutManager.getNextKnuthElements(BlockStackingLayoutManager.java:289)
      at org.apache.fop.layoutmgr.list.ListBlockLayoutManager.getNextKnuthElements(ListBlockLayoutManager.java:103)
      at org.apache.fop.layoutmgr.BlockStackingLayoutManager.getNextKnuthElements(BlockStackingLayoutManager.java:239)
      at org.apache.fop.layoutmgr.BlockLayoutManager.getNextChildElements(BlockLayoutManager.java:141)
      at org.apache.fop.layoutmgr.BlockStackingLayoutManager.getNextKnuthElements(BlockStackingLayoutManager.java:289)
      at org.apache.fop.layoutmgr.BlockLayoutManager.getNextKnuthElements(BlockLayoutManager.java:113)
      at org.apache.fop.layoutmgr.BlockLayoutManager.getNextKnuthElements(BlockLayoutManager.java:105)
      at org.apache.fop.layoutmgr.table.TableCellLayoutManager.getNextKnuthElements(TableCellLayoutManager.java:191)
      at org.apache.fop.layoutmgr.table.RowGroupLayoutManager.createElementsForRowGroup(RowGroupLayoutManager.java:120)
      at org.apache.fop.layoutmgr.table.RowGroupLayoutManager.getNextKnuthElements(RowGroupLayoutManager.java:63)
      at org.apache.fop.layoutmgr.table.TableContentLayoutManager.getKnuthElementsForRowIterator(TableContentLayoutManager.java:270)
      at org.apache.fop.layoutmgr.table.TableContentLayoutManager.getNextKnuthElements(TableContentLayoutManager.java:212)
      at org.apache.fop.layoutmgr.table.TableLayoutManager.getNextKnuthElements(TableLayoutManager.java:273)
      at org.apache.fop.layoutmgr.FlowLayoutManager.getNextChildElements(FlowLayoutManager.java:223)
      at org.apache.fop.layoutmgr.FlowLayoutManager.addChildElements(FlowLayoutManager.java:148)
      at org.apache.fop.layoutmgr.FlowLayoutManager.getNextKnuthElements(FlowLayoutManager.java:116)
      at org.apache.fop.layoutmgr.FlowLayoutManager.getNextKnuthElements(FlowLayoutManager.java:69)
      at org.apache.fop.layoutmgr.PageBreaker.getNextKnuthElements(PageBreaker.java:252)
      at org.apache.fop.layoutmgr.AbstractBreaker.getNextBlockList(AbstractBreaker.java:643)
      at org.apache.fop.layoutmgr.PageBreaker.getNextBlockList(PageBreaker.java:178)
      at org.apache.fop.layoutmgr.PageBreaker.getNextBlockList(PageBreaker.java:158)
      at org.apache.fop.layoutmgr.AbstractBreaker.doLayout(AbstractBreaker.java:384)
      at org.apache.fop.layoutmgr.PageBreaker.doLayout(PageBreaker.java:112)
      at org.apache.fop.layoutmgr.PageSequenceLayoutManager.activateLayout(PageSequenceLayoutManager.java:135)
      at org.apache.fop.area.AreaTreeHandler.endPageSequence(AreaTreeHandler.java:267)
      at org.apache.fop.fo.pagination.PageSequence.endOfNode(PageSequence.java:130)
      at org.apache.fop.fo.FOTreeBuilder$MainFOHandler.endElement(FOTreeBuilder.java:360)
      at org.apache.fop.fo.FOTreeBuilder.endElement(FOTreeBuilder.java:190)
      at org.apache.xalan.transformer.TransformerIdentityImpl.endElement(TransformerIdentityImpl.java:1101)
      at org.apache.xerces.parsers.AbstractSAXParser.endElement(Unknown Source)
      at org.apache.xerces.xinclude.XIncludeHandler.endElement(Unknown Source)
      at org.apache.xerces.impl.XMLNSDocumentScannerImpl.scanEndElement(Unknown Source)
      at org.apache.xerces.impl.XMLDocumentFragmentScannerImpl$FragmentContentDispatcher.dispatch(Unknown Source)
      at org.apache.xerces.impl.XMLDocumentFragmentScannerImpl.scanDocument(Unknown Source)
      at org.apache.xerces.parsers.XML11Configuration.parse(Unknown Source)
      at org.apache.xerces.parsers.XML11Configuration.parse(Unknown Source)
      at org.apache.xerces.parsers.XMLParser.parse(Unknown Source)
      at org.apache.xerces.parsers.AbstractSAXParser.parse(Unknown Source)
      at org.apache.xerces.jaxp.SAXParserImpl$JAXPSAXParser.parse(Unknown Source)
      at org.apache.xalan.transformer.TransformerIdentityImpl.transform(TransformerIdentityImpl.java:484)
      at org.apache.fop.cli.InputHandler.transformTo(InputHandler.java:285)
      at org.apache.fop.cli.InputHandler.renderTo(InputHandler.java:115)
      at org.apache.fop.cli.Main.startFOP(Main.java:186)
      at org.apache.fop.cli.Main.main(Main.java:217)

      1. fop-2461-test-case.xml
        1 kB
        Matthias Reischenbacher
      2. listNPE.patch
        9 kB
        Thanasis Giannimaras

        Issue Links

          Activity

          Hide
          matthias8283 Matthias Reischenbacher added a comment -

          Test case

          Show
          matthias8283 Matthias Reischenbacher added a comment - Test case
          Hide
          adelmelle Andreas L. Delmelle added a comment -

          The auxiliary layout elements generated for padding-after have their Position set to null, so the second call in the chain "endEl.getPosition().getPosition()" causes the NPE.
          The statement at that line was indeed included in the merged branch, and was always bound to fail at some point. Best guess is that there are no tests yet that use padding-after on the last block in list item content, and the assumption was made that endEl ''always'' returned a wrapped Position

          Lazy workaround:
          As there is already a null-check for endEl itself, but none for the returned Position, the check can be extended

          (endEl != null && endEl.getPosition() != null) ? endEl.getPosition().getPosition() ...

          Works and does not seem to break anything, but is not exactly robust. The same would have to be done a few lines up, or it would be just a matter of time before the same issue arises again, when someone decides to try padding-after in the label...

          More robust alternative(s)?
          Create a single final Position (ListElement.NULL_POSITION?), and use that as the default in the ListElement constructor and in ListElement.setPosition(), in case a literal null is passed. May want to make setPosition() final as well, since it is effectively overridden nowhere, and should be kept that way.
          At least the semantics would then be clearer, and can be documented as such in the javadoc:

          • ListElement.getPosition() --> safe, always returns a Position instance
          • Position.getPosition() --> unsafe, potentially returns null, depending on the implementation
          Show
          adelmelle Andreas L. Delmelle added a comment - The auxiliary layout elements generated for padding-after have their Position set to null, so the second call in the chain "endEl.getPosition().getPosition()" causes the NPE. The statement at that line was indeed included in the merged branch, and was always bound to fail at some point. Best guess is that there are no tests yet that use padding-after on the last block in list item content, and the assumption was made that endEl ''always'' returned a wrapped Position Lazy workaround: As there is already a null-check for endEl itself, but none for the returned Position, the check can be extended (endEl != null && endEl.getPosition() != null ) ? endEl.getPosition().getPosition() ... Works and does not seem to break anything, but is not exactly robust. The same would have to be done a few lines up, or it would be just a matter of time before the same issue arises again, when someone decides to try padding-after in the label... More robust alternative(s)? Create a single final Position (ListElement.NULL_POSITION?), and use that as the default in the ListElement constructor and in ListElement.setPosition(), in case a literal null is passed. May want to make setPosition() final as well, since it is effectively overridden nowhere, and should be kept that way. At least the semantics would then be clearer, and can be documented as such in the javadoc: ListElement.getPosition() --> safe, always returns a Position instance Position.getPosition() --> unsafe, potentially returns null, depending on the implementation
          Hide
          philipolson Philip Olson added a comment -

          We also suffer from this problem, and the workaround allowed us to test FOP 2.

          Show
          philipolson Philip Olson added a comment - We also suffer from this problem, and the workaround allowed us to test FOP 2.
          Hide
          gspeicher Geoff Speicher added a comment -

          NB: the FreeBSD port of FOP 2.0 includes the above lazy workaround as a local patch until this issue is resolved. Otherwise FOP 2.0 fails to build some other ports (notably, misc/freebsd-doc-*).

          References:

          Show
          gspeicher Geoff Speicher added a comment - NB: the FreeBSD port of FOP 2.0 includes the above lazy workaround as a local patch until this issue is resolved. Otherwise FOP 2.0 fails to build some other ports (notably, misc/freebsd-doc-*). References: http://www.freshports.org/textproc/fop/ https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=201007 https://svnweb.freebsd.org/ports?view=revision&revision=390702
          Hide
          adelmelle Andreas L. Delmelle added a comment -

          Lazy workaround committed as a preliminary fix – see http://svn.apache.org/r1690396

          Keeping the issue open for now, as I still feel adding the extra not-null check is not robust enough... At least, if we were to do a point / fix release, this would no longer be a blocker for people to upgrade.

          Show
          adelmelle Andreas L. Delmelle added a comment - Lazy workaround committed as a preliminary fix – see http://svn.apache.org/r1690396 Keeping the issue open for now, as I still feel adding the extra not-null check is not robust enough... At least, if we were to do a point / fix release, this would no longer be a blocker for people to upgrade.
          Hide
          AGiannimaras Thanasis Giannimaras added a comment -

          I tried to implement Andreas suggestions (about creating a single "default position) and came up with this fix (listNPE.patch) . Goes without saying any suggestions for improvements are more than welcome!

          Thanks,

          Show
          AGiannimaras Thanasis Giannimaras added a comment - I tried to implement Andreas suggestions (about creating a single "default position) and came up with this fix (listNPE.patch) . Goes without saying any suggestions for improvements are more than welcome! Thanks,
          Hide
          ssteiner1 simon steiner added a comment -

          Should this be closed

          Show
          ssteiner1 simon steiner added a comment - Should this be closed

            People

            • Assignee:
              Unassigned
              Reporter:
              matthias8283 Matthias Reischenbacher
            • Votes:
              4 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development