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

NPE in RewriteNavigationHandler at Before RestoreView time. #142

Closed
fabmars opened this issue Oct 22, 2013 · 13 comments
Closed

NPE in RewriteNavigationHandler at Before RestoreView time. #142

fabmars opened this issue Oct 22, 2013 · 13 comments

Comments

@fabmars
Copy link
Contributor

@fabmars fabmars commented Oct 22, 2013

Environment: Glassfish 3.1.2.2 patched with JSF 2.1.26 and JDK 1.7.0_45.
This is somehow related to issue 103.

It's currently impossible to perform navigation at Before RestoreView time. Only from After RestoreView because RewriteNavigationHandler requires a valid ViewRoot before navigation. It throws a NPE during #handleNavigation, line 53 (String originalViewId = context.getViewRoot().getViewId();)

It's annoying to me as I have a PhaseListener which checks access rights and such (mainly before restoreView) and performs navigation to appropriate fallback/error pages when needed. Of course there's an easy workaround: save the outcome in the request or a request-scoped bean or FC attributes till After RestoreView.

Why not null-check the "original" ViewRoot too ? Maybe there's a case I don't see.

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Oct 22, 2013

Hey Fabien!

I think a null check here is a good idea, would you like to submit a pull request and test case? I believe this should be reproducible on most JSF implementations. Either way :)

@fabmars
Copy link
Contributor Author

@fabmars fabmars commented Oct 23, 2013

Sure.

@fabmars
Copy link
Contributor Author

@fabmars fabmars commented Nov 6, 2013

Will take a little while extra. My health as usual...
Though i'm wondering what should happen if both the origin and the destination viewIds are null. The navigation is complete anyway, right?

On a side note you can see Rewrite 2.0.8 running in prod on our modest label site: suntriprecords.com. Apache+mod_jk => glassfish3+ajp listener => Rewrite. Works.

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Nov 10, 2013

That's awesome! Great work! I love seeing the end result :) PS. Glad you are still here!

@fabmars
Copy link
Contributor Author

@fabmars fabmars commented Nov 11, 2013

As usual I can't figure out how to make a test there.
Should I try to use some PhaseListener ?
Should I use a managed bean with @RequestAction @deferred(before=Phase.RESTORE_VIEW) and navigate from there?

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Nov 11, 2013

As you suggested, I think just using a managed bean with a @deferred @RequestAction would be fine. But to simplify, you could just have that method set a value that is rendered to the view. Check out this test for example:

https://github.com/ocpsoft/rewrite/blob/master/config-prettyfaces-tests/src/test/java/org/ocpsoft/rewrite/prettyfaces/ruleNaming/MapingIdNamingTest.java
https://github.com/ocpsoft/rewrite/blob/master/config-prettyfaces-tests/src/test/resources/rule_naming/index.xhtml

In fact, you could copy this rule-naming test structure and use it almost verbatim. Just add a managed bean with a @Join and @deferred action, instead of pretty-config.xml.

What do you think?

@fabmars
Copy link
Contributor Author

@fabmars fabmars commented Nov 15, 2013

Thanks. OK I think I did it. But now I can't pass the test suite. Using -P JBOSS_AS_MANAGED_7.X, it stops at rewrite-config-servlet, apparently on the org.ocpsoft.rewrite.servlet.config.JoinEncodingConfigurationTest test. Do you experience the same or did I forget to rebase something ?

[INFO] rewrite-config-servlet ............................ FAILURE [46.355s]

21:59:48,549 ERROR org.apache.catalina.core.ContainerBase.[jboss.web].[default-host].[/rewrite-test].[default] Servlet.service() for servlet default threw exception: java.lang.IllegalStateException
at org.apache.catalina.connector.ResponseFacade.sendError(ResponseFacade.java:408) [jbossweb-7.0.13.Final.jar:]
at javax.servlet.http.HttpServletResponseWrapper.sendError(HttpServletResponseWrapper.java:152) [jboss-servlet-api_3.0_spec-1.0.0.Final.jar:1.0.0.Final]
at org.ocpsoft.rewrite.servlet.impl.HttpRewriteWrappedResponse.sendError(HttpRewriteWrappedResponse.java:452) [rewrite-servlet.jar:]
at org.apache.catalina.servlets.DefaultServlet.serveResource(DefaultServlet.java:661) [jbossweb-7.0.13.Final.jar:]
at org.apache.catalina.servlets.DefaultServlet.doGet(DefaultServlet.java:344) [jbossweb-7.0.13.Final.jar:]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:734) [jboss-servlet-api_3.0_spec-1.0.0.Final.jar:1.0.0.Final]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:847) [jboss-servlet-api_3.0_spec-1.0.0.Final.jar:1.0.0.Final]
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:329) [jbossweb-7.0.13.Final.jar:]
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:248) [jbossweb-7.0.13.Final.jar:]
at org.ocpsoft.rewrite.servlet.RewriteFilter.doFilter(RewriteFilter.java:200) [rewrite-servlet.jar:]
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:280) [jbossweb-7.0.13.Final.jar:]
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:248) [jbossweb-7.0.13.Final.jar:]
at org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:840) [jbossweb-7.0.13.Final.jar:]
at org.apache.catalina.core.ApplicationDispatcher.processRequest(ApplicationDispatcher.java:622) [jbossweb-7.0.13.Final.jar:]
at org.apache.catalina.core.ApplicationDispatcher.doForward(ApplicationDispatcher.java:560) [jbossweb-7.0.13.Final.jar:]
at org.apache.catalina.core.ApplicationDispatcher.forward(ApplicationDispatcher.java:488) [jbossweb-7.0.13.Final.jar:]
at org.ocpsoft.rewrite.servlet.impl.HttpRewriteResultHandler.handleResult(HttpRewriteResultHandler.java:38) [rewrite-servlet.jar:]
at org.ocpsoft.rewrite.servlet.RewriteFilter.rewrite(RewriteFilter.java:263) [rewrite-servlet.jar:]
at org.ocpsoft.rewrite.servlet.RewriteFilter.doFilter(RewriteFilter.java:188) [rewrite-servlet.jar:]

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Nov 15, 2013

Hmmm... that looks like The Rewrite ConfigurationProvider used in your test
is doing something bad :) Could you show me the code you have so far? Maybe
create a preliminary pull request so I can take a look? Thanks!

On Fri, Nov 15, 2013 at 9:04 AM, Fabien Marsaud notifications@github.comwrote:

Thanks. OK I think I did it. But now I can't pass the test suite. Using -P
JBOSS_AS_MANAGED_7.X, it stops at rewrite-config-servlet, apparently on the
org.ocpsoft.rewrite.servlet.config.JoinEncodingConfigurationTest test. Do
you experience the same or did I forget to rebase something ?

[INFO] rewrite-config-servlet ............................ FAILURE
[46.355s]

21:59:48,549 ERROR
org.apache.catalina.core.ContainerBase.[jboss.web].[default-host].[/rewrite-test].[default]http://http--127.0.0.1-8080-2Servlet.service() for servlet default threw exception:
java.lang.IllegalStateException
at
org.apache.catalina.connector.ResponseFacade.sendError(ResponseFacade.java:408)
[jbossweb-7.0.13.Final.jar:]
at
javax.servlet.http.HttpServletResponseWrapper.sendError(HttpServletResponseWrapper.java:152)
[jboss-servlet-api_3.0_spec-1.0.0.Final.jar:1.0.0.Final]
at
org.ocpsoft.rewrite.servlet.impl.HttpRewriteWrappedResponse.sendError(HttpRewriteWrappedResponse.java:452)
[rewrite-servlet.jar:]
at
org.apache.catalina.servlets.DefaultServlet.serveResource(DefaultServlet.java:661)
[jbossweb-7.0.13.Final.jar:]
at
org.apache.catalina.servlets.DefaultServlet.doGet(DefaultServlet.java:344)
[jbossweb-7.0.13.Final.jar:]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:734)
[jboss-servlet-api_3.0_spec-1.0.0.Final.jar:1.0.0.Final]
at javax.servlet.http.HttpServlet.service(HttpServlet.java:847)
[jboss-servlet-api_3.0_spec-1.0.0.Final.jar:1.0.0.Final]
at
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:329)
[jbossweb-7.0.13.Final.jar:]
at
org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:248)
[jbossweb-7.0.13.Final.jar:]
at
org.ocpsoft.rewrite.servlet.RewriteFilter.doFilter(RewriteFilter.java:200)
[rewrite-servlet.jar:]
at
org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:280)
[jbossweb-7.0.13.Final.jar:]
at
org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:248)
[jbossweb-7.0.13.Final.jar:]
at
org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:840)
[jbossweb-7.0.13.Final.jar:]
at
org.apache.catalina.core.ApplicationDispatcher.processRequest(ApplicationDispatcher.java:622)
[jbossweb-7.0.13.Final.jar:]
at
org.apache.catalina.core.ApplicationDispatcher.doForward(ApplicationDispatcher.java:560)
[jbossweb-7.0.13.Final.jar:]
at
org.apache.catalina.core.ApplicationDispatcher.forward(ApplicationDispatcher.java:488)
[jbossweb-7.0.13.Final.jar:]
at
org.ocpsoft.rewrite.servlet.impl.HttpRewriteResultHandler.handleResult(HttpRewriteResultHandler.java:38)
[rewrite-servlet.jar:]
at
org.ocpsoft.rewrite.servlet.RewriteFilter.rewrite(RewriteFilter.java:263)
[rewrite-servlet.jar:]
at
org.ocpsoft.rewrite.servlet.RewriteFilter.doFilter(RewriteFilter.java:188)
[rewrite-servlet.jar:]


Reply to this email directly or view it on GitHubhttps://github.com//issues/142#issuecomment-28570746
.

Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."

@fabmars
Copy link
Contributor Author

@fabmars fabmars commented Nov 15, 2013

No, if my test was the source of the issue, the failure would be on the rewrite-integration-faces-tests module, not that early in the reactor chain. Of course I tried running the suite without my test too, same result. That's why I was wondering if you guys have some WIP or something.

@lincolnthree
Copy link
Member

@lincolnthree lincolnthree commented Nov 15, 2013

AH! Sorry, I'm with you now. I don't believe this test should be failing.
Let me verify. However, in the mean time, if you just want to run your
test, verify that, and submit a PR, I can verify the full suite later.

On Fri, Nov 15, 2013 at 11:46 AM, Fabien Marsaud
notifications@github.comwrote:

No, if my test was the source of the issue, the failure would be on the
rewrite-integration-faces-tests module, not that early in the reactor
chain. Of course I tried running the suite without my test too, same
result. That's why I was wondering if you guys have some WIP or something.


Reply to this email directly or view it on GitHubhttps://github.com//issues/142#issuecomment-28583832
.

Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."

@chkal
Copy link
Member

@chkal chkal commented Nov 25, 2013

So we can close this now?

@fabmars
Copy link
Contributor Author

@fabmars fabmars commented Nov 25, 2013

I guess so, even though I'm not sure about my test case. But as Travis is saying ok...

@chkal
Copy link
Member

@chkal chkal commented Nov 28, 2013

If the test runs fine on Travis it should be OK. However, it's really weird that some tests are failing for you...

@chkal chkal closed this Nov 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.