You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ni...@apache.org on 2007/10/13 01:36:15 UTC
svn commit: r584325 - in /commons/proper/io/trunk/src:
java/org/apache/commons/io/FilenameUtils.java
test/org/apache/commons/io/FilenameUtilsTestCase.java
Author: niallp
Date: Fri Oct 12 16:36:12 2007
New Revision: 584325
URL: http://svn.apache.org/viewvc?rev=584325&view=rev
Log:
IO-128 - currently file name "normalization" errors in the equals method causes a mis-leading NullPointerException. Adding a check for this and throwing an IllegalArgumentException with a better message should improve the user experience.
Modified:
commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java
commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java
Modified: commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java
URL: http://svn.apache.org/viewvc/commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java?rev=584325&r1=584324&r2=584325&view=diff
==============================================================================
--- commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java (original)
+++ commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java Fri Oct 12 16:36:12 2007
@@ -977,6 +977,10 @@
if (normalized) {
filename1 = normalize(filename1);
filename2 = normalize(filename2);
+ if (filename1 == null || filename2 == null) {
+ throw new IllegalArgumentException(
+ "Error normalizing one or both of the file names");
+ }
}
if (caseSensitivity == null) {
caseSensitivity = IOCase.SENSITIVE;
Modified: commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java
URL: http://svn.apache.org/viewvc/commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java?rev=584325&r1=584324&r2=584325&view=diff
==============================================================================
--- commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java (original)
+++ commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java Fri Oct 12 16:36:12 2007
@@ -784,6 +784,30 @@
assertEquals(false, FilenameUtils.equalsNormalizedOnSystem("a/b/", "a/b"));
}
+ /**
+ * Test for https://issues.apache.org/jira/browse/IO-128
+ */
+ public void testEqualsNormalizedError_IO_128() {
+ try {
+ FilenameUtils.equalsNormalizedOnSystem("//file.txt", "file.txt");
+ fail("Invalid normalized first file");
+ } catch(IllegalArgumentException e) {
+ // expected result
+ }
+ try {
+ FilenameUtils.equalsNormalizedOnSystem("file.txt", "//file.txt");
+ fail("Invalid normalized second file");
+ } catch(IllegalArgumentException e) {
+ // expected result
+ }
+ try {
+ FilenameUtils.equalsNormalizedOnSystem("//file.txt", "//file.txt");
+ fail("Invalid normalized both filse");
+ } catch(IllegalArgumentException e) {
+ // expected result
+ }
+ }
+
public void testEquals_fullControl() {
assertEquals(false, FilenameUtils.equals("file.txt", "FILE.TXT", true, IOCase.SENSITIVE));
assertEquals(true, FilenameUtils.equals("file.txt", "FILE.TXT", true, IOCase.INSENSITIVE));
Re: svn commit: r584325 - in /commons/proper/io/trunk/src: java/org/apache/commons/io/FilenameUtils.java test/org/apache/commons/io/FilenameUtilsTestCase.java
Posted by Niall Pemberton <ni...@gmail.com>.
Hi Antonio
I answered this on the Jira ticket - so that details of the change and
conversation is in the same place - I'm hoping someone else who knows
more about this will jump in and give you a better answer
https://issues.apache.org/jira/browse/IO-128
Niall
On 10/13/07, Antonio Gallardo <ag...@agssa.net> 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:
>
> //foo/.//bar --> /foo/bar
>
> Hence a user could assume that
>
> //file.txt --> /file.txt and not an IllegalArgumentException
>
> Is that correct?
>
> Many thanks in advance for your reply.
>
> Best Regards,
>
> Antonio Gallardo.
>
>
>
> niallp@apache.org escribió:
> > Author: niallp
> > Date: Fri Oct 12 16:36:12 2007
> > New Revision: 584325
> >
> > URL: http://svn.apache.org/viewvc?rev=584325&view=rev
> > Log:
> > IO-128 - currently file name "normalization" errors in the equals method causes a mis-leading NullPointerException. Adding a check for this and throwing an IllegalArgumentException with a better message should improve the user experience.
> >
> > Modified:
> > commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java
> > commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java
> >
> > Modified: commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java
> > URL: http://svn.apache.org/viewvc/commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java?rev=584325&r1=584324&r2=584325&view=diff
> > ==============================================================================
> > --- commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java (original)
> > +++ commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java Fri Oct 12 16:36:12 2007
> > @@ -977,6 +977,10 @@
> > if (normalized) {
> > filename1 = normalize(filename1);
> > filename2 = normalize(filename2);
> > + if (filename1 == null || filename2 == null) {
> > + throw new IllegalArgumentException(
> > + "Error normalizing one or both of the file names");
> > + }
> > }
> > if (caseSensitivity == null) {
> > caseSensitivity = IOCase.SENSITIVE;
> >
> > Modified: commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java
> > URL: http://svn.apache.org/viewvc/commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java?rev=584325&r1=584324&r2=584325&view=diff
> > ==============================================================================
> > --- commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java (original)
> > +++ commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java Fri Oct 12 16:36:12 2007
> > @@ -784,6 +784,30 @@
> > assertEquals(false, FilenameUtils.equalsNormalizedOnSystem("a/b/", "a/b"));
> > }
> >
> > + /**
> > + * Test for https://issues.apache.org/jira/browse/IO-128
> > + */
> > + public void testEqualsNormalizedError_IO_128() {
> > + try {
> > + FilenameUtils.equalsNormalizedOnSystem("//file.txt", "file.txt");
> > + fail("Invalid normalized first file");
> > + } catch(IllegalArgumentException e) {
> > + // expected result
> > + }
> > + try {
> > + FilenameUtils.equalsNormalizedOnSystem("file.txt", "//file.txt");
> > + fail("Invalid normalized second file");
> > + } catch(IllegalArgumentException e) {
> > + // expected result
> > + }
> > + try {
> > + FilenameUtils.equalsNormalizedOnSystem("//file.txt", "//file.txt");
> > + fail("Invalid normalized both filse");
> > + } catch(IllegalArgumentException e) {
> > + // expected result
> > + }
> > + }
> > +
> > public void testEquals_fullControl() {
> > assertEquals(false, FilenameUtils.equals("file.txt", "FILE.TXT", true, IOCase.SENSITIVE));
> > assertEquals(true, FilenameUtils.equals("file.txt", "FILE.TXT", true, IOCase.INSENSITIVE));
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r584325 - in /commons/proper/io/trunk/src: java/org/apache/commons/io/FilenameUtils.java
test/org/apache/commons/io/FilenameUtilsTestCase.java
Posted by Antonio Gallardo <ag...@agssa.net>.
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:
//foo/.//bar --> /foo/bar
Hence a user could assume that
//file.txt --> /file.txt and not an IllegalArgumentException
Is that correct?
Many thanks in advance for your reply.
Best Regards,
Antonio Gallardo.
niallp@apache.org escribió:
> Author: niallp
> Date: Fri Oct 12 16:36:12 2007
> New Revision: 584325
>
> URL: http://svn.apache.org/viewvc?rev=584325&view=rev
> Log:
> IO-128 - currently file name "normalization" errors in the equals method causes a mis-leading NullPointerException. Adding a check for this and throwing an IllegalArgumentException with a better message should improve the user experience.
>
> Modified:
> commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java
> commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java
>
> Modified: commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java
> URL: http://svn.apache.org/viewvc/commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java?rev=584325&r1=584324&r2=584325&view=diff
> ==============================================================================
> --- commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java (original)
> +++ commons/proper/io/trunk/src/java/org/apache/commons/io/FilenameUtils.java Fri Oct 12 16:36:12 2007
> @@ -977,6 +977,10 @@
> if (normalized) {
> filename1 = normalize(filename1);
> filename2 = normalize(filename2);
> + if (filename1 == null || filename2 == null) {
> + throw new IllegalArgumentException(
> + "Error normalizing one or both of the file names");
> + }
> }
> if (caseSensitivity == null) {
> caseSensitivity = IOCase.SENSITIVE;
>
> Modified: commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java
> URL: http://svn.apache.org/viewvc/commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java?rev=584325&r1=584324&r2=584325&view=diff
> ==============================================================================
> --- commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java (original)
> +++ commons/proper/io/trunk/src/test/org/apache/commons/io/FilenameUtilsTestCase.java Fri Oct 12 16:36:12 2007
> @@ -784,6 +784,30 @@
> assertEquals(false, FilenameUtils.equalsNormalizedOnSystem("a/b/", "a/b"));
> }
>
> + /**
> + * Test for https://issues.apache.org/jira/browse/IO-128
> + */
> + public void testEqualsNormalizedError_IO_128() {
> + try {
> + FilenameUtils.equalsNormalizedOnSystem("//file.txt", "file.txt");
> + fail("Invalid normalized first file");
> + } catch(IllegalArgumentException e) {
> + // expected result
> + }
> + try {
> + FilenameUtils.equalsNormalizedOnSystem("file.txt", "//file.txt");
> + fail("Invalid normalized second file");
> + } catch(IllegalArgumentException e) {
> + // expected result
> + }
> + try {
> + FilenameUtils.equalsNormalizedOnSystem("//file.txt", "//file.txt");
> + fail("Invalid normalized both filse");
> + } catch(IllegalArgumentException e) {
> + // expected result
> + }
> + }
> +
> public void testEquals_fullControl() {
> assertEquals(false, FilenameUtils.equals("file.txt", "FILE.TXT", true, IOCase.SENSITIVE));
> assertEquals(true, FilenameUtils.equals("file.txt", "FILE.TXT", true, IOCase.INSENSITIVE));
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org