commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Christian Grobmeier (JIRA)" <j...@apache.org>
Subject [jira] Updated: (SANDBOX-284) TarArchiveEntry(File) now crashes on file system roots
Date Tue, 17 Mar 2009 07:42:50 GMT

     [ https://issues.apache.org/jira/browse/SANDBOX-284?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Christian Grobmeier updated SANDBOX-284:
----------------------------------------

    Attachment: patch-filerootcrash.txt

Attached patch should fix the problem. Before:
this.name.charAt(this.name.length() - 1) != '/'

name's length will be asked if it is zero. If so, the root directory is meant.

I couldn't write a working test since I am on OSX. I had to copy out the necessary code. If
somebody has an Win-Box, please test.

I think this file needs some refactoring to make it more testable.

> TarArchiveEntry(File) now crashes on file system roots
> ------------------------------------------------------
>
>                 Key: SANDBOX-284
>                 URL: https://issues.apache.org/jira/browse/SANDBOX-284
>             Project: Commons Sandbox
>          Issue Type: Bug
>          Components: Compress
>    Affects Versions: Nightly Builds
>         Environment: win xp sp3, but this is probably irrelevant
>            Reporter: Sam Smith
>            Priority: Blocker
>         Attachments: patch-filerootcrash.txt
>
>
> The TarArchiveEntry(File) constructor now crashes if the File argument is a file system
root.
> 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(StringBuffer.java:162)
> 		at org.apache.commons.compress.archivers.tar.TarArchiveEntry.<init>(TarArchiveEntry.java:245)
> Looking at the code (I downloaded revision 743098 yesterday), it is easy to see why this
occured:
> 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 (this.name.charAt(this.name.length() - 1) != '/') {
> must crash, because it falsely assumes that fileName has content.
> THIS IS A SHOW STOPPER BUG FOR ME.
> 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
fine.
> 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.


Mime
View raw message