You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@wicket.apache.org by Harald Wellmann <Ha...@multi-m.de> on 2010/08/15 18:32:40 UTC

Is WicketFilter.checkCharacterEncoding() safe?

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-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org


Re: Is WicketFilter.checkCharacterEncoding() safe?

Posted by Martin Grigorov <mg...@apache.org>.
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.Wellmann@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-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>

Re: Is WicketFilter.checkCharacterEncoding() safe?

Posted by Martin Grigorov <mg...@apache.org>.
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.Wellmann@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-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>