ParameterValidator doesnt call validators

[GoogleCodeExporter]

Hi,

i'm using prettyfaces-jsf2-3.2.1 with jsf mojarra 2.1.0 having a problem 
getting my path parameter validated. Having this configuration in my 
pretty-config:

<url-mapping id="item" >
 <pattern value="/item/#{/.*/ parentId}">
  <validate index="0" validator="#{validator.validateQuery}"/>
 </pattern>
 <view-id value="/jcr/items.xhtml" />
</url-mapping>

the validator is never called. I tracked it till ParameterValidator.java line 
80 which prevents it from getting called:  

if (!!"".equals(validators) && (validators != null)){}

Please have look, or correct me if i am wrong.

Greetings

Original issue reported on code.google.com by gnalFF@gmail.com on 18 Apr 2011 at 11:55

[GoogleCodeExporter]

Thanks for bringing this up! This really looks like a typo. I'll have a look at 
it ASAP.

Original comment by chkalt on 20 Apr 2011 at 4:44

• Changed state: Accepted

[GoogleCodeExporter]

I'm also not sure this feature supports EL. I think it only supports 
ValidatorIDs.

Original comment by lincolnb...@gmail.com on 20 Apr 2011 at 4:57

[GoogleCodeExporter]

I added support for validation methods some time ago. The "validator" attribute 
is used for validator methods and the "validatorIds" attribute can be used to 
reference JSF validator IDs.

See: 

http://ocpsoft.com/docs/prettyfaces/3.2.0/en-US/html/Configuration.html#config.v
alidation

Original comment by chkalt on 20 Apr 2011 at 5:25

[GoogleCodeExporter]

ValidatorIDs don't work, too. Its a general problem at this point...

Original comment by gnalFF@gmail.com on 20 Apr 2011 at 5:25

[GoogleCodeExporter]

BTW: I can confirm that ParameterValidator contains a typo which currently 
prevents validation of path parameters. I fixed this but now I see other 
strange exceptions showing up. I'll have to take a deeper look as soon as I 
find some time (maybe this evening or tomorrow).

Original comment by chkalt on 20 Apr 2011 at 5:27

[GoogleCodeExporter]

I just committed a fix for this issue. Could you perhaps give 3.3.0-SNAPSHOT a 
try?

See this wiki page for details on how to use the snapshots:

https://github.com/ocpsoft/prettyfaces/wiki/Snapshots

Original comment by chkalt on 20 Apr 2011 at 6:01

[GoogleCodeExporter]

The validator is now called. But if validation fails for the parameter, the 
framework is not able to send a 404 and continues instead to dispatch to jsf. I 
am not able to debug right now but I can see this in the logs:

Validation: ROO
20.04.2011 12:08:03 com.sun.faces.context.ExceptionHandlerImpl throwIt
INFO: Exception when handling error trying to reset the response.

My current configuration is like this:

<rewrite match="^/$" substitute="/item/ROOT" redirect="301"/>

<url-mapping id="item" >
 <pattern value="/item/#{/.*/ parentId}">
  <validate index="0" validator="#{validator.validateQuery}" />
 </pattern>
 <view-id value="/jcr/items.xhtml" />
</url-mapping>

my validator is like this:
@ManagedBean(eager = true)
@ApplicationScoped
public class Validator {

@ManagedProperty("#{#services.securityContext}")
private SecurityContext securityContext;

public void validateQuery(FacesContext context, UIComponent component, Object 
value) {
 System.out.println("Validation: " + value);
 if (Strings.isNullOrEmpty((String) value) || "ROOT".equals(value)) {
    return;
 }
 try {
  JCRNode.byId(securityContext.getCurrentSession(), (String) value);
 } catch (ItemNotFoundException ex) {
  throw new ValidatorException(new FacesMessage("the item does not exist any more"));
 }
}

public void setSecurityContext(SecurityContext securityContext) {
 this.securityContext = securityContext;
}
}

Original comment by gnalFF@gmail.com on 20 Apr 2011 at 10:14

[GoogleCodeExporter]

Thanks for testing the snapshot!

That's exactly the behavior I saw while fixing this. But it occurred only 
sometimes in my tests. Strange. I'll have a look at this later today. I guess 
there is just a FacesContext.responseComplete() missing.

I'll come back to you ASAP.

Original comment by chkalt on 20 Apr 2011 at 11:36

[GoogleCodeExporter]

@gnalFF:

I think this issue is now fixed. I just committed a patch that will correctly 
abort the JSF lifecycle if the parameter validation fails. Could you perhaps 
test the latest snapshots again (prettyfaces-jsf2-3.3.0-20110420.153615-6.jar)?

@Lincoln:

Could you perhaps take a look at my commit? Just to make sure everything is OK 
with it?

https://github.com/ocpsoft/prettyfaces/commit/5b770e419ed607383fbbfe9b0cd47d5156
e1b104

I'm a but unsure about this part of the code. I don't really get why you are 
just skipping parameter validation and injection if 
FacesContext.getResponseComplete() returns false. The dynaview code is still 
executed. Wouldn't it make sense to completely abort the processing of the JSF 
phase in this case (like I am doing it now if the parameter validation fails)? 
You introduced this check here:

https://github.com/ocpsoft/prettyfaces/commit/736a4d15cd39d2a5c16e32c2e4f6971630
24fe82

Thanks :)

Original comment by chkalt on 20 Apr 2011 at 3:40

[GoogleCodeExporter]

Hmmm, do be honest, I don't know why I did it that way. It looks like an 
oversight/mistake. I think your fix is what should really be happening.

Original comment by lincolnb...@gmail.com on 20 Apr 2011 at 3:54

[GoogleCodeExporter]

I'm not sure if the current code (including my patch) is correct because we 
currently continue processing the request even if 
FacesContext.getResponseComplete() is false. I think its dangerous to continue 
in this case because the response might already be committed and so we could 
run in all kind of problems. Wouldn't it make sense to change the code to 
something like this:

   public void afterPhase(final PhaseEvent event)
   {

      if (PhaseId.RESTORE_VIEW.equals(event.getPhaseId()) && !event.getFacesContext().getResponseComplete())
      {

         // run parameter validation
         validator.validateParameters(event.getFacesContext());

         // abort if validation failed, 404 response code has already been set
         if(event.getFacesContext().getResponseComplete()) {
            return;
         }

         // inject parameters
         injector.injectParameters(event.getFacesContext());

         // Do all the rest... Process dynaview, etc....

The reason we introduced the getResponseComplete() check was that PrimeFaces 
uses a PhaseListener to send image data (or something like this) in 
RESTORE_VIEW and after that calls FacesContext.responseComplete().

Original comment by chkalt on 21 Apr 2011 at 4:44

[GoogleCodeExporter]

Hi,

i tested the latest snapshot, validation now correctly returns 404. However on 
postback there is still this coming up:

21.04.2011 08:14:54 com.sun.faces.context.ExceptionHandlerImpl throwIt
INFO: Exception when handling error trying to reset the response.

Thx for your help so far, i am really happy about this fast response time. 
Makes me confident to use pretty faces in production.

Original comment by gnalFF@gmail.com on 21 Apr 2011 at 6:22

[GoogleCodeExporter]

@gnalFF:

Thanks for testing the snapshot! :)

Is there anything more in the logs? A stacktrace or something like this? Look 
here at this link for example.

http://java.net/jira/browse/GLASSFISH-15905?focusedCommentId=302106&page=com.atl
assian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_302106

This report shows a similar message but there is also a stacktrace with more 
information.

Original comment by chkalt on 21 Apr 2011 at 6:49

[GoogleCodeExporter]

so, i debugged the code now. And i think the problem is in the flow of it all. 
1) In a postback with a wrong path my requestscoped bean's init method is 
called before the validator is actually running. This causes an exception, 
which is queued for the exceptionhandler. (Restore_view)
2) After that the Paramtervalidator is running and setting responseComplete.
3) The exceptionhandler is now running with the queued exception and is trying 
to reset the response (This is where this message comes from) 
4) finally the exception handler renders an errorpage 
This is all happening in this class: 
(com.sun.faces.context.ExceptionHandlerImpl)

I am a bit wondering why you do not validate the url directly in the 
prettyfilter, before dispatching to the actual jsf?

Original comment by gnalFF@gmail.com on 21 Apr 2011 at 8:47

[GoogleCodeExporter]

Hmmm. This explains the exception you are seeing.

We are currently in the process of modularizing PrettyFaces in a way that many 
features will be usable even in a plain Servlet environment without JSF. As the 
PrettyFilter is one of the core classes of PrettyFaces, it therefore cannot 
depend on JSF. Unfortunately the validation is using the JSF API and so it has 
been moved to the PhaseListener.

I'm a bit surprised that your application performs a postback to an invalid (in 
regard to validation) URL. As the postback is always performed to the URL the 
page was originally rendered for, it seems like the URL was valid before. Could 
you explain your usecase a bit more?

Original comment by chkalt on 22 Apr 2011 at 9:02

[GoogleCodeExporter]

there is no actual usecase, i just modified the postback url to test the 
behaviour.

Great to hear about your ongoing efforts. I understand that in this case the 
Filter should not have dependencies to JSF.
 I dont know how your plans are and which features should be available for servlets as well, but i could imagine that url validation could be usefull for servlets, too. Therefore the validation would be also great within the filter. Perhaps you are able to wrap this in a generell interface which than would delegate to a specific validation mechanism.

Since there is no special usecase about postback validation (except user trying 
to cheat :-)), for me the bug is resolved.

Thx for your help!

Original comment by gnalFF@gmail.com on 22 Apr 2011 at 9:18

[GoogleCodeExporter]

In a Servlet environment (and also when running JSF) path parameters are 
transformed into request parameters that can be accessed by using 
HttpServletRequest.getParameter().

See: 

http://ocpsoft.com/docs/prettyfaces/3.2.0/en-US/html/Configuration.html#config.p
athparams.named

I totally agree with you that it makes sense to move the validation into the 
filter somehow. Some kind of SPI would be perfect for this. This way we could 
integrate various validation frameworks (JSF, JSR303, OVal, etc). I think we 
will able to take the modularization much further in 3.3.0. Much work to do! :)

Thank you very much for your bug report and the intensive debugging. We really 
appreciate this.

Feel free to open tickets for bugs or feature requests any time. We are always 
happy to get all kind of feedback! :)

Original comment by chkalt on 22 Apr 2011 at 9:43

• Changed state: Fixed