wrong handling of exceptions for Resource#respond and ComponentResourceRequestTarget#respond

• Key: WICKET-144
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Eelco Hillenius
• Reporter: Eelco Hillenius
• Component/s: wicket
• Affects Version/s: 1.2.4, 1.3.0-beta1, 2.0 branch (discontinued)
• Fix Version/s: 1.2.4, 1.3.0-beta1, 2.0 branch (discontinued)

Description

Resource's exception handling is too fragile. Currently (before fix) it looks like this:

private final void respond(final IResourceStream resourceStream, final Response response)
{
try
{
final OutputStream out = response.getOutputStream();
try
{
Streams.copy(resourceStream.getInputStream(), out);
}
finally
{
resourceStream.close();
out.flush();
}
}
catch (Exception e)
{
Throwable throwable = e;
boolean ignoreException = false;
while (throwable != null)
{
if (throwable instanceof SocketException)
{
String message = throwable.getMessage();
ignoreException = message != null
&& (message.indexOf("Connection reset by peer") != -1 || message
.indexOf("Software caused connection abort") != -1);
}
else
{
ignoreException = throwable.getClass().getName()
.indexOf("ClientAbortException") >= 0;
if (ignoreException)
{
if (log.isDebugEnabled())
{
log.debug("Socket exception ignored for sending Resource "
+ "response to client (ClientAbort)", e);
}
break;
}
}
throwable = throwable.getCause();
}
if (!ignoreException)
{
throw new WicketRuntimeException("Unable to render resource stream "
+ resourceStream, e);
}
}
}

and ComponentResourceRequestTarget re-threw this exception, resulting in Wicket trying to render an error page. One problem with the above code is that it is simply does not cover all appservers/versions/os-ses, nor it that something we should want to support. For instance, an aborted request in Jetty on Linux/ OSX results in an IOException (or more concrete EofException).

Besides the fact that this can get you weird behavior in general, handling the exception like we did there is never a good idea. We should never try to render an error page for any exception during a resource request but instead set an appropriate status code. I propose status code 500 (INTERNAL_SERVER_ERROR) for this and put that in ComponentResourceRequestTarget, and alter Resource so that it always re-throws any exception.