Re: Is WicketFilter.checkCharacterEncoding() safe?

2010-08-16 Thread Martin Grigorov
As solution I can see using HttpServletRequestWrapper instead of dealing
directly with the HttpServletRequest passed by the container.
This way we can provide custom implementation of HttpServletRequestWrapper
that overrides javax.servlet.ServletRequestWrapper.getCharacterEncoding()
and if super.getCharacterEncoding() returns 'null' then use the
preconfigured one in the settings and cache it.

If other devs think this solution is ok then please create a ticket and we
will improve it.

On Sun, Aug 15, 2010 at 6:32 PM, Harald Wellmann harald.wellm...@multi-m.de
 wrote:

 I'm not sure if this is severe or not, but I think there's a gap in the
 implicit assumptions underlying WicketFilter.checkCharacterEncoding():

 It seems the author's intention was to guarantee a postcondition of

servletRequest.getCharacterEncoding() != null,

 but this postcondition does not hold in all cases.

 I ran into this issue after upgrading my Glassfish app server from 3.0.1 to
 3.1-b12, containing a newer release of Weld which causes the problem to
 appear. After this upgrade, my log was full of warnings of the following
 type:

 WARN  o.apache.catalina.connector.Request - PWC4011: Unable to set request
 character encoding to UTF-8 from context /xxx, because request parameters
 have already been read, or ServletRequest.getReader() has already been
 called

 Setting a couple of breakpoints revealed that Weld registers a
 ServletRequestListener and calls request.getParameter() in that listener. By
 specification, this listener gets called before all servlet filters,
 including WicketFilter, and again by specification, the invocation of
 servlet.request.setCharacterEncoding() from WicketFilter has no effect,
 since a request parameter has already been read by the WeldListener.

 So it there are other parts of Wicket relying on the request character
 encoding to be non-null, then this looks like a bug to me. Otherwise, if
 Wicket handles a null character encoding gracefully, then
 checkCharacterEncoding() is redundant, because it cannot be guaranteed to
 have any effect.

 Regards,

 Harald

 -
 To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
 For additional commands, e-mail: users-h...@wicket.apache.org




Is WicketFilter.checkCharacterEncoding() safe?

2010-08-15 Thread Harald Wellmann
I'm not sure if this is severe or not, but I think there's a gap in the 
implicit assumptions underlying WicketFilter.checkCharacterEncoding():

It seems the author's intention was to guarantee a postcondition of

servletRequest.getCharacterEncoding() != null,

but this postcondition does not hold in all cases.

I ran into this issue after upgrading my Glassfish app server from 3.0.1 to 
3.1-b12, containing a newer release of Weld which causes the problem to appear. 
After this upgrade, my log was full of warnings of the following type:

WARN  o.apache.catalina.connector.Request - PWC4011: Unable to set request 
character encoding to UTF-8 from context /xxx, because request parameters have 
already been read, or ServletRequest.getReader() has already been called

Setting a couple of breakpoints revealed that Weld registers a 
ServletRequestListener and calls request.getParameter() in that listener. By 
specification, this listener gets called before all servlet filters, including 
WicketFilter, and again by specification, the invocation of 
servlet.request.setCharacterEncoding() from WicketFilter has no effect, since a 
request parameter has already been read by the WeldListener.

So it there are other parts of Wicket relying on the request character encoding 
to be non-null, then this looks like a bug to me. Otherwise, if Wicket handles 
a null character encoding gracefully, then checkCharacterEncoding() is 
redundant, because it cannot be guaranteed to have any effect.

Regards,

Harald

-
To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org
For additional commands, e-mail: users-h...@wicket.apache.org