Wicket
  1. Wicket
  2. WICKET-943

java.lang.NullPointerException at wicket.markup.html.list.ListView.renderItem(ListView.java:676)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.2.6
    • Fix Version/s: 1.2.7
    • Component/s: wicket
    • Labels:
      None
    • Environment:
      Window XP SP3, Java 1.6, Tomcat 5.0_09

      Description

      This problem happens if parent component has been hidden by authorization strategy during the previous page render, but on the next render parent component becomes visible. This problem is common for all repeaters. The problem here is in internalOnAttach() method of repeaters. This method will not populate items if it not visible in hierarchy, but when it checks this it use old visible state (Component.FLAG_IS_RENDER_ALLOWED) of component (itself and parent components), this state is changed after setRenderAllowed() method in Component.java on 1630 line returns, but internalAttach() method is called earlier in Component.java on 1624 line. After that component become visible and will be rendered. But it can't be rendered because items were not populated, so null pointer exception occurs.

      This problem can be fixed if method calls of internalAttach() and setRenderAllowed(isActionAuthorized(RENDER)) will be swapped or logic of repeaters (ListView, DataView, etc) components will be changed.

        Activity

        Hide
        Johan Compagner added a comment -

        is this also an issue for 1.3?

        also i dont know if we can easily fix this is see comments in the code like:

        // check authorization
        // first the component itself
        // (after attach as otherwise list views etc wont work)
        setRenderAllowed();

        So the component structure must be there before the setRenderedAllowed call?

        Show
        Johan Compagner added a comment - is this also an issue for 1.3? also i dont know if we can easily fix this is see comments in the code like: // check authorization // first the component itself // (after attach as otherwise list views etc wont work) setRenderAllowed(); So the component structure must be there before the setRenderedAllowed call?
        Hide
        Matej Knopp added a comment -

        setRenderAllowed(isActionAuthorized(RENDER)) must be called after beforeRender(). Can't we just call setRenderAllowed(true) in component detach?

        Show
        Matej Knopp added a comment - setRenderAllowed(isActionAuthorized(RENDER)) must be called after beforeRender(). Can't we just call setRenderAllowed(true) in component detach?
        Hide
        Johan Compagner added a comment -

        Then the are not rendered but aren't we then opening a security hole?
        Because then the component is visible in the request/interface call phase so if one could guess the path it would be able to execute something right?

        So then we have to quickly set it to visible before the onRender ?

        Show
        Johan Compagner added a comment - Then the are not rendered but aren't we then opening a security hole? Because then the component is visible in the request/interface call phase so if one could guess the path it would be able to execute something right? So then we have to quickly set it to visible before the onRender ?
        Hide
        Alexander Suslov added a comment -

        I think that it a logical issue and need to be fixed here (in renderComponent method by switching method call), because logic of many components is based on visibility status. If after that some components will work incorrectly, then this components need to be fixed. This problem look likes a result of some quick crunch that was made earlier.

        Or this bug also can be fixed by removing check of visibility status in attach method, but this will reduce system performance.

        Show
        Alexander Suslov added a comment - I think that it a logical issue and need to be fixed here (in renderComponent method by switching method call), because logic of many components is based on visibility status. If after that some components will work incorrectly, then this components need to be fixed. This problem look likes a result of some quick crunch that was made earlier. Or this bug also can be fixed by removing check of visibility status in attach method, but this will reduce system performance.
        Hide
        Johan Compagner added a comment -

        we now only call isVisible() instead of isVisibleInHierarchy()
        in 1.3 we already do that

        Show
        Johan Compagner added a comment - we now only call isVisible() instead of isVisibleInHierarchy() in 1.3 we already do that

          People

          • Assignee:
            Johan Compagner
            Reporter:
            Alexander Suslov
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development