Hi Tim,

Tim Funk wrote:
This is just a nit, but in JspServlet.serviceJspFile() there is ...
synchronized(this) {
wrapper = (JspServletWrapper) rctxt.getWrapper(jspUri);
if (wrapper == null) {
// Check if the requested JSP page exists, to avoid
// creating unnecessary directories and files.
InputStream resourceStream =
context.getResourceAsStream(jspUri);
if (resourceStream == null) {
response.sendError(HttpServletResponse.SC_NOT_FOUND,
jspUri);
return;
} else {
try {
resourceStream.close();
} catch(IOException e) { /* ignore */ }
}
...
}
}


Is there any reason we are getting the stream instead of checking for a null URL, for example:

synchronized(this) {
wrapper = (JspServletWrapper) rctxt.getWrapper(jspUri);
if (wrapper == null) {
// Check if the requested JSP page exists, to avoid
// creating unnecessary directories and files.
if (null==context.getResource(jspUri)) {
response.sendError(HttpServletResponse.SC_NOT_FOUND,
jspUri);
return;
}
...
}
}


This way the existence of the file is checked instead of opening it.

Good point! There must have been a reason for using getResourceAsStream() in the past, but I agree it is no longer necessary and just committed your proposed patch.


Jan





---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to