Am 24.01.2015 um 17:13 schrieb Christopher Schultz:
Felix,
On 1/24/15 9:42 AM, fschumac...@apache.org wrote:
Author: fschumacher
Date: Sat Jan 24 14:42:27 2015
New Revision: 1654524
URL: http://svn.apache.org/r1654524
Log:
Close input and output streams in expandCGIScript to
avoid resource leaks. Issue reported by Coverity Scan.
Modified:
tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java
Modified: tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java?rev=1654524&r1=1654523&r2=1654524&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java (original)
+++ tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java Sat Jan 24
14:42:27 2015
@@ -1133,6 +1133,10 @@ public final class CGIServlet extends Ht
File f = new File(destPath.toString());
if (f.exists()) {
+ try {
+ is.close();
+ } catch (IOException ignore) {
+ }
Should this be logged? It should very rarely happen, but it would be
good to know if there was a problem (which might represent a resource leak).
I looked for other examples in the source code before and the first few
examples I found where ignoring the exception while closing, too. So I
thought it would be ok, do ignore this exception.
If we don't want this exception ignored, at what level should the
information be logged? I would go for debug or info.
// Don't need to expand if it already exists
return;
}
@@ -1162,10 +1166,16 @@ public final class CGIServlet extends Ht
}
FileOutputStream fos = new FileOutputStream(f);
- // copy data
- IOTools.flow(is, fos);
- is.close();
- fos.close();
+ try {
+ // copy data
+ IOTools.flow(is, fos);
+ } finally {
+ try {
+ is.close();
+ } catch (IOException ignore) {
+ }
Same here.
+ fos.close();
Why not just call fos.close() to close everything all at once?
Do you mean is.close() and fos.close() inside the same try block? I
separated them, so that a failed is.close() will not interfere with the
close call on fos. I didn't put fos.close in a try catch block, since it
was not in one before the fix.
Felix
Thanks,
-chris
+ }
if (debug >= 2) {
log("expandCGIScript: expanded '" + srcPath + "' to '" +
destPath + "'");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
