commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stefan Bodewig (JIRA)" <>
Subject [jira] Commented: (SANDBOX-284) TarArchiveEntry(File) now crashes on file system roots
Date Tue, 17 Mar 2009 12:44:50 GMT


Stefan Bodewig commented on SANDBOX-284:

The code has been that way since the very first version in Ant

Strangely, it only affects the File-arg constructor, maybe you've been using the String-arg
constructor in the past.

I'm currently writing a few tests to see whether a file name of "/" causes any problems (GNU
tar complains but simply strips the leading /) and will check in a fix - that applies to all
constructors - soon.

> TarArchiveEntry(File) now crashes on file system roots
> ------------------------------------------------------
>                 Key: SANDBOX-284
>                 URL:
>             Project: Commons Sandbox
>          Issue Type: Bug
>          Components: Compress
>    Affects Versions: Nightly Builds
>         Environment: win xp sp3, but this is probably irrelevant
>            Reporter: Sam Smith
>            Assignee: Stefan Bodewig
>            Priority: Blocker
>         Attachments: patch-filerootcrash.txt
> The TarArchiveEntry(File) constructor now crashes if the File argument is a file system
> For example, on my windows box, I want to backup the entire contents of my F drive, so
I am supplying a File argument that is constructed as
> 	new File("F:\\")
> That particular file causes the TarArchiveEntry(File) constructor to fail as follows:
> 	Caused by: java.lang.StringIndexOutOfBoundsException: String index out of range: -1
> 		at java.lang.StringBuffer.charAt(
> 		at org.apache.commons.compress.archivers.tar.TarArchiveEntry.<init>(
> Looking at the code (I downloaded revision 743098 yesterday), it is easy to see why this
> 1) the
> 	if (osname != null) {
> logic will strip the "F:" from my path name of "F:\", leaving just the "\"
> 2) that "\" will then be turned into a single "/" by the 
> 	fileName = fileName.replace(File.separatorChar, '/');
> line
> 3) that single "/" will then be removed by the
> 	while (fileName.startsWith("/")) {
> logic, leaving the empty string "".
> 4) then line #245
> 	if ( - 1) != '/') {
> must crash, because it falsely assumes that fileName has content.
> I am not sure when this current behavior of TarArchiveEntry was introduced; a very old
codebase (from 2+ years ago) of compress that I used to use handled file system roots just
> There are many ways to fix this.  For instance, if it is, in fact, OK for the name field
to be empty, then you can simply put a check on line #245 as follows:
>             if ( (name.length() > 0) && (name.charAt(name.length() - 1) !=
'/') ) {
> (NOTE on coding style: do you really need to use "this." in the constructor when there
is no possible ambiguity?  Makes your code wordier and therefore harder to read.)
> My guess, not knowing your full codebase well, is that it is NOT OK for name to be blank.
 For example, you seem to want directories to end with a '/' char, and file ssystem roots
are always directories.
> Therefore, you have some decisions to make:
> a) is it OK for the name field to simply be "/" in the case of file system roots?
> b) if a) is not good for some reason, then you must introduce an artificial root name,
so that name takes on a value like
> 	"filesystemRoot/"
> or
> 	"filesystemRoot_F/"
> or whatever.
> This bug, by the way, brings up another issue: there currently are no javadocs regarding
field contracts.  Every field's javadocs needs its constraints to be specified as a contract,
for example,
>     /**
>     * The entry's name.
>     * <p>
>     * Contract: is never null (and never empty?).
>     * Contains (only ASCII chars?  any Unicode chars?).
>     * Must be (<= 100 chars?  unlimited number of chars?).
>     * If {@link #file} is a directory, then must end in a '/' char.
>     * etc...
>     */
>     private StringBuffer name;

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message