tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: svn commit: r1654524 - /tomcat/trunk/java/org/apache/catalina/servlets/CGIServlet.java
Date Sat, 24 Jan 2015 16:13:19 GMT
Felix,

On 1/24/15 9:42 AM, fschumacher@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).

>                  // 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?

Thanks,
-chris

> +                    }
>                      if (debug >= 2) {
>                          log("expandCGIScript: expanded '" + srcPath + "' to '" + destPath
+ "'");
>                      }
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 


Mime
View raw message