[WICKET-144] wrong handling of exceptions for Resource#respond and ComponentResourceRequestTarget#respond

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.0 branch (discontinued), 1.2.4, 1.3.0-beta1
• Fix Version/s: 2.0 branch (discontinued), 1.2.4, 1.3.0-beta1
• Component/s: wicket
• Assignee: Eelco Hillenius
• Reporter: Eelco Hillenius
• Created: 06/Dec/06 01:48
• Updated: 09/Dec/06 18:53
• Resolved: 09/Dec/06 18:53

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.