Bug 53531 - ExpandWar.expand does not check the return value of File.mkdir and File.mkdirs
Summary: ExpandWar.expand does not check the return value of File.mkdir and File.mkdirs
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Catalina (show other bugs)
Version: Nightly Build
Hardware: All All
: P2 trivial (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
Depends on:
Reported: 2012-07-11 01:42 UTC by Christopher Schultz
Modified: 2012-10-01 08:25 UTC (History)
0 users

Patch against current 5.5.x trunk (r1359975) (1.74 KB, patch)
2012-07-11 01:49 UTC, Christopher Schultz
Details | Diff
Patch for 6.0.x against r1359053 (1.92 KB, patch)
2012-07-11 19:36 UTC, Christopher Schultz
Details | Diff
2012-08-11_tc55_53531.patch (2.65 KB, patch)
2012-08-11 19:18 UTC, Konstantin Kolinko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Schultz 2012-07-11 01:42:02 UTC

o.a.c.startup.ExpandWar.expand() calls File.mkdirs on line 165 and does not check the return value. If the call fails, a subsequent attempt to create a new FileOutputStream for a file in the (failed-to-create) directory results in a FileNotFoundException with the whole filename as the message. Here is an example from the above tomcat-user original post:

SEVERE: Exception fixing docBase: {0}
/home/surgnet/public_html/sample/META-INF/MANIFEST.MF (No such file or
            at java.io.FileOutputStream.open(Native Method)
            at java.io.FileOutputStream.<init>(FileOutputStream.java:194)
            at java.io.FileOutputStream.<init>(FileOutputStream.java:145)
            at org.apache.catalina.startup.ExpandWar.expand(ExpandWar.java:457)
            at org.apache.catalina.startup.ExpandWar.expand(ExpandWar.java:173)

This is confusing to users because the real issue has been hidden: the inability to create the parent directory for the file.

The analogous code in the current trunk is:

                    if (!parent.mkdirs() && !parent.isDirectory()) {
                        throw new IOException(
                                sm.getString("expandWar.createFailed", parent));

I believe this is the correct approach, which should be adopted for Tomcat 5.5.
Comment 1 Christopher Schultz 2012-07-11 01:49:12 UTC
Created attachment 29045 [details]
Patch against current 5.5.x trunk (r1359975)
Comment 2 Christopher Schultz 2012-07-11 02:01:52 UTC
This bug exists in Tomcat 6.0.x, so I'm re-assigning since it's a bit more relevant. The fix can be thought of as a back-port, then, I guess.
Comment 3 Christopher Schultz 2012-07-11 19:36:07 UTC
Created attachment 29050 [details]
Patch for 6.0.x against r1359053
Comment 4 Christopher Schultz 2012-07-11 19:41:17 UTC
Comment on attachment 29045 [details]
Patch against current 5.5.x trunk (r1359975)

Patch is obsolete: patch proposed for 6.0.x.
Comment 5 Christopher Schultz 2012-08-09 18:18:07 UTC
Fixed in 6.0.x. Will be included in 6.0.36.
Comment 6 Konstantin Kolinko 2012-08-11 19:18:03 UTC
Created attachment 29212 [details]

Patch for Tomcat 5.5.

Includes a fix for missing message argument in ContextConfig.init(). That is where the thrown IOException is caught and logged.
Comment 7 Mark Thomas 2012-10-01 08:25:05 UTC
Fixed in 5.5.x and will be included in 5.5.36 onwards.