[ 
https://issues.apache.org/jira/browse/IO-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12534452
 ] 

Niall Pemberton commented on IO-128:
------------------------------------

>From the dev list...

On 10/13/07, Antonio Gallardo <[EMAIL PROTECTED]> wrote:
> Hi Niall,
> 
> Thanks for taking care of the issue, however it is not clear to me from
> the javadocs that we should expect an IllegalArgumentException returning
> from equalsNormalizedOnSystem(). IMHO, it states it calls first the
> normalize() and based on the javadocs of normalize(), it should silently
> fix, the link. On the javados, there is:

Unfortunately until today I had never looked at or used the normalize code - so 
I don't know what the original intention was.  The javadocs for the normalize() 
method do say "the normalized filename, or null if invalid" for the return 
value (see http://tinyurl.com/2tczc8). So it seems clear that errors return 
null (and there are tests for error conditions in the test case (see 
http://tinyurl.com/3bh7ml).

On that basis I believe that we must cater for errors in the equals method. As 
it stands I think my change to throw an IllegalArgumentException with a 
relevant message and in the right place is better than a misleading 
NullPointerException elsewhere. However I am happy for anyone else to come up 
with better suggestions.

Whether a value like "//file.txt" should be causing a normalize error is 
another question though...

> //foo/.//bar --> /foo/bar
> 
> Hence a user could assume that
> 
> //file.txt --> /file.txt and not an IllegalArgumentException
> 
> Is that correct?

Tracking through your issue one of the first things the doNormalize() method 
does is call  getPrefixLength() and if it returns a negative value, then it 
returns null - indicating invalid.

In getPrefixLength() there is code that if the first two characters are file 
separators (which they are in your case) then it returns -1 if there are no 
other separator characters - so thats triggering the error here.

Therefore seems that someone has specifically coded to cause an error in your 
scenario. Hopefully someone with better knowledge will jump in and talk more 
sense than I can. Sorry :(


> NPE on FilenameUtils.equalsNormalizedOnSystem()
> -----------------------------------------------
>
>                 Key: IO-128
>                 URL: https://issues.apache.org/jira/browse/IO-128
>             Project: Commons IO
>          Issue Type: Bug
>          Components: Utilities
>    Affects Versions: 1.2, 1.3, 1.3.1, 1.3.2
>            Reporter: Antonio Gallardo
>            Assignee: Niall Pemberton
>             Fix For: 1.4
>
>
> The following code in commons-io (1.3.2) throws an NPE exception:
> org.apache.commons.io.FilenameUtils
>     .equalsNormalizedOnSystem(
>             "//a.html",
>             "//ab.html");
> And here is the exception:
> java.lang.NullPointerException: The strings must not be null
>    at org.apache.commons.io.IOCase.checkEquals(IOCase.java:141)
>    at org.apache.commons.io.FilenameUtils.equals(FilenameUtils.java:984)
>    at 
> org.apache.commons.io.FilenameUtils.equalsNormalizedOnSystem(FilenameUtils.java:956)
>    at CodeSnippet_32.run(CodeSnippet_32.java:4)
>    at 
> org.eclipse.jdt.internal.debug.ui.snippeteditor.ScrapbookMain1.eval(ScrapbookMain1.java:20)
>    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>    at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>    at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>    at java.lang.reflect.Method.invoke(Method.java:585)
>    at 
> org.eclipse.jdt.internal.debug.ui.snippeteditor.ScrapbookMain.evalLoop(ScrapbookMain.java:54)
>    at 
> org.eclipse.jdt.internal.debug.ui.snippeteditor.ScrapbookMain.main(ScrapbookMain.java:35)
> I think it is wrong a message "The strings must not be null", since there is 
> not a null string involved in the call.
> Interesting is if both or 1 of the strings is null, it did not throws an 
> exception.
> Additional comment from Niall Pemberton (on the dev mail list):
> The problem is that the FilenameUtils's normalize(String) method
> returns "null" if it thinks the file names are invalid - which in your
> case it seems to be doing so for both file names.
> So I guess theres two issues here - you're right the error is
> misleading and FilenameUtils should check the names again after
> calling normalize() for nulls and throw a more appropriate message.

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

Reply via email to