You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2019/08/10 13:28:00 UTC

[commons-vfs] branch master updated: [VFS-724] FileContent#getByteArray() throws IllegalArgumentException: Buffer size <= 0 when file size is 0.

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-vfs.git


The following commit(s) were added to refs/heads/master by this push:
     new a8cd5b8  [VFS-724] FileContent#getByteArray() throws IllegalArgumentException: Buffer size <= 0 when file size is 0.
a8cd5b8 is described below

commit a8cd5b889fa79abd3a4faa1de9dc74ebd7278054
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sat Aug 10 09:27:54 2019 -0400

    [VFS-724] FileContent#getByteArray() throws IllegalArgumentException:
    Buffer size <= 0 when file size is 0.
    
    Different implementation and tests for this ticket. Closes #68.
---
 commons-vfs2/pom.xml                               |  5 ++
 .../commons/vfs2/provider/DefaultFileContent.java  | 12 ++--
 .../vfs2/provider/DefaultFileContentTest.java      | 84 +++++++++++++++++-----
 .../src/test/resources/test-data/size-0-file.bin   |  0
 pom.xml                                            |  6 ++
 src/changes/changes.xml                            |  3 +
 6 files changed, 85 insertions(+), 25 deletions(-)

diff --git a/commons-vfs2/pom.xml b/commons-vfs2/pom.xml
index 586c89f..d20d65e 100644
--- a/commons-vfs2/pom.xml
+++ b/commons-vfs2/pom.xml
@@ -96,6 +96,11 @@
       <scope>test</scope>
     </dependency>
     <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
       <groupId>org.apache.commons</groupId>
       <artifactId>commons-lang3</artifactId>
       <scope>test</scope>
diff --git a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java
index 5fa6242..870f4ea 100644
--- a/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java
+++ b/commons-vfs2/src/main/java/org/apache/commons/vfs2/provider/DefaultFileContent.java
@@ -340,7 +340,7 @@ public final class DefaultFileContent implements FileContent {
      */
     @Override
     public InputStream getInputStream() throws FileSystemException {
-        return buildInputStream(null);
+        return buildInputStream(0);
     }
 
     /**
@@ -401,7 +401,7 @@ public final class DefaultFileContent implements FileContent {
      */
     @Override
     public OutputStream getOutputStream(final boolean bAppend) throws FileSystemException {
-        return buildOutputStream(bAppend, null);
+        return buildOutputStream(bAppend, 0);
     }
 
     /**
@@ -485,7 +485,7 @@ public final class DefaultFileContent implements FileContent {
         }
     }
 
-    private InputStream buildInputStream(final Integer bufferSize) throws FileSystemException {
+    private InputStream buildInputStream(final int bufferSize) throws FileSystemException {
         /*
          * if (getThreadData().getState() == STATE_WRITING || getThreadData().getState() == STATE_RANDOM_ACCESS) { throw
          * new FileSystemException("vfs.provider/read-in-use.error", file); }
@@ -494,7 +494,7 @@ public final class DefaultFileContent implements FileContent {
         // Get the raw input stream
         final InputStream inputStream = fileObject.getInputStream();
 
-        final InputStream wrappedInputStream = bufferSize == null ?
+        final InputStream wrappedInputStream = bufferSize == 0 ?
             new FileContentInputStream(fileObject, inputStream) :
             new FileContentInputStream(fileObject, inputStream, bufferSize);
 
@@ -504,7 +504,7 @@ public final class DefaultFileContent implements FileContent {
         return wrappedInputStream;
     }
 
-    private OutputStream buildOutputStream(final boolean bAppend, final Integer bufferSize) throws FileSystemException {
+    private OutputStream buildOutputStream(final boolean bAppend, final int bufferSize) throws FileSystemException {
         /*
          * if (getThreadData().getState() != STATE_NONE)
          */
@@ -518,7 +518,7 @@ public final class DefaultFileContent implements FileContent {
         final OutputStream outstr = fileObject.getOutputStream(bAppend);
 
         // Create and set wrapper
-        final FileContentOutputStream wrapped = bufferSize == null ?
+        final FileContentOutputStream wrapped = bufferSize == 0 ?
             new FileContentOutputStream(fileObject, outstr) :
             new FileContentOutputStream(fileObject, outstr, bufferSize);
         streams.setOutstr(wrapped);
diff --git a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/DefaultFileContentTest.java b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/DefaultFileContentTest.java
index 2192f9e..e818474 100644
--- a/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/DefaultFileContentTest.java
+++ b/commons-vfs2/src/test/java/org/apache/commons/vfs2/provider/DefaultFileContentTest.java
@@ -16,6 +16,9 @@
  */
 package org.apache.commons.vfs2.provider;
 
+import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.vfs2.FileContent;
 import org.apache.commons.vfs2.FileObject;
 import org.apache.commons.vfs2.FileSystemManager;
 import org.apache.commons.vfs2.VFS;
@@ -23,8 +26,10 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import java.io.File;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
 
 /**
  * {@code DefaultFileContentTest} tests for bug-VFS-614. This bug involves the stream implementation closing the stream
@@ -33,8 +38,46 @@ import java.io.OutputStream;
 public class DefaultFileContentTest {
     private static final String expected = "testing";
 
+    /**
+     * Test VFS-724 should be done on a website which render a page with no content size. Note the getSize() is
+     * currently the value sent back by the server then zero usually means no content length attached.
+     */
     @Test
-    public void testMarkingWorks() throws Exception {
+    public void testGetZeroContents() throws IOException {
+        final FileSystemManager fsManager = VFS.getManager();
+        try (final FileObject fo = fsManager.resolveFile(new File("."), "src/test/resources/test-data/size-0-file.bin");
+                final FileContent content = fo.getContent()) {
+            Assert.assertEquals(0, content.getSize());
+            Assert.assertEquals(StringUtils.EMPTY, content.getString(StandardCharsets.UTF_8));
+            Assert.assertEquals(StringUtils.EMPTY, content.getString(StandardCharsets.UTF_8.name()));
+            Assert.assertArrayEquals(ArrayUtils.EMPTY_BYTE_ARRAY, content.getByteArray());
+        }
+    }
+
+    private void testInputStreamBufferSize(final int bufferSize) throws Exception {
+        final File temp = File.createTempFile("temp-file-name", ".tmp");
+        final FileSystemManager fileSystemManager = VFS.getManager();
+
+        try (FileObject file = fileSystemManager.resolveFile(temp.getAbsolutePath())) {
+            file.getContent().getInputStream(bufferSize);
+        }
+    }
+
+    public void testInputStreamBufferSize0() throws Exception {
+        testInputStreamBufferSize(0);
+    }
+
+    public void testInputStreamBufferSize1() throws Exception {
+        testInputStreamBufferSize(1);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testInputStreamBufferSizeNegative() throws Exception {
+        testInputStreamBufferSize(-2);
+    }
+
+    @Test
+    public void testMarkingWhenReadingEOS() throws Exception {
         final File temp = File.createTempFile("temp-file-name", ".tmp");
         final FileSystemManager fileSystemManager = VFS.getManager();
 
@@ -44,13 +87,17 @@ public class DefaultFileContentTest {
                 outputStream.flush();
             }
             try (InputStream stream = file.getContent().getInputStream()) {
+                int readCount = 0;
                 if (stream.markSupported()) {
                     for (int i = 0; i < 10; i++) {
                         stream.mark(0);
                         final byte[] data = new byte[100];
-                        stream.read(data, 0, 7);
+                        readCount = stream.read(data, 0, 7);
                         stream.read();
+                        Assert.assertEquals(readCount, 7);
                         Assert.assertEquals(expected, new String(data).trim());
+                        readCount = stream.read(data, 8, 10);
+                        Assert.assertEquals(readCount, -1);
                         stream.reset();
                     }
                 }
@@ -59,7 +106,7 @@ public class DefaultFileContentTest {
     }
 
     @Test
-    public void testMarkingWhenReadingEOS() throws Exception {
+    public void testMarkingWorks() throws Exception {
         final File temp = File.createTempFile("temp-file-name", ".tmp");
         final FileSystemManager fileSystemManager = VFS.getManager();
 
@@ -69,17 +116,13 @@ public class DefaultFileContentTest {
                 outputStream.flush();
             }
             try (InputStream stream = file.getContent().getInputStream()) {
-                int readCount = 0;
                 if (stream.markSupported()) {
                     for (int i = 0; i < 10; i++) {
                         stream.mark(0);
                         final byte[] data = new byte[100];
-                        readCount = stream.read(data, 0, 7);
+                        stream.read(data, 0, 7);
                         stream.read();
-                        Assert.assertEquals(readCount, 7);
                         Assert.assertEquals(expected, new String(data).trim());
-                        readCount = stream.read(data, 8, 10);
-                        Assert.assertEquals(readCount, -1);
                         stream.reset();
                     }
                 }
@@ -87,28 +130,30 @@ public class DefaultFileContentTest {
         }
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testPassingIllegalBufferSizeToInputStream() throws Exception {
+    private void testOutputStreamBufferSize(final int bufferSize) throws Exception {
         final File temp = File.createTempFile("temp-file-name", ".tmp");
         final FileSystemManager fileSystemManager = VFS.getManager();
 
         try (FileObject file = fileSystemManager.resolveFile(temp.getAbsolutePath())) {
-            file.getContent().getInputStream(-2);
+            file.getContent().getOutputStream(bufferSize).close();
         }
     }
 
-    @Test(expected = IllegalArgumentException.class)
-    public void testPassingIllegalBufferSizeToOutputStream() throws Exception {
-        final File temp = File.createTempFile("temp-file-name", ".tmp");
-        final FileSystemManager fileSystemManager = VFS.getManager();
+    public void testOutputStreamBufferSize0() throws Exception {
+        testOutputStreamBufferSize(0);
+    }
 
-        try (FileObject file = fileSystemManager.resolveFile(temp.getAbsolutePath())) {
-            file.getContent().getOutputStream(0);
-        }
+    public void testOutputStreamBufferSize1() throws Exception {
+        testOutputStreamBufferSize(1);
     }
 
     @Test(expected = IllegalArgumentException.class)
-    public void testPassingIllegalBufferSizeToOutputStreamWithAppendFlag() throws Exception {
+    public void testOutputStreamBufferSizeNegative() throws Exception {
+        testOutputStreamBufferSize(-1);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testOutputStreamBufferSizeNegativeWithAppendFlag() throws Exception {
         final File temp = File.createTempFile("temp-file-name", ".tmp");
         final FileSystemManager fileSystemManager = VFS.getManager();
 
@@ -116,4 +161,5 @@ public class DefaultFileContentTest {
             file.getContent().getOutputStream(true, -1);
         }
     }
+
 }
diff --git a/commons-vfs2/src/test/resources/test-data/size-0-file.bin b/commons-vfs2/src/test/resources/test-data/size-0-file.bin
new file mode 100644
index 0000000..e69de29
diff --git a/pom.xml b/pom.xml
index 42c3c3f..87d27f2 100644
--- a/pom.xml
+++ b/pom.xml
@@ -276,6 +276,7 @@
         <configuration>
           <excludes combine.children="append">
             <!--  trivial test data text files -->
+            <exclude>src/test/resources/test-data/**/*.bin</exclude>
             <exclude>src/test/resources/test-data/**/*.txt</exclude>
             <exclude>src/test/resources/test-data/**/*.tgz</exclude>
             <exclude>src/test/resources/test-data/**/*.tbz2</exclude>
@@ -521,6 +522,11 @@
         <version>4.12</version>
       </dependency>
       <dependency>
+        <groupId>org.mockito</groupId>
+        <artifactId>mockito-core</artifactId>
+        <version>3.0.0</version>
+      </dependency>
+      <dependency>
         <groupId>org.apache.commons</groupId>
         <artifactId>commons-lang3</artifactId>
         <version>3.9</version>
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index f2cee23..e227c91 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -53,6 +53,9 @@ The <action> type attribute can be add,update,fix,remove.
       <action issue="VFS-725" dev="ggregory" type="fix" due-to="Gary Gregory">
         [Local] org.apache.commons.vfs2.FileContent.getLastModifiedTime() is losing milliseconds (always ends in 000).
       </action>
+      <action issue="VFS-724" dev="ggregory" type="fix" due-to="William R, Gary Gregory">
+        FileContent#getByteArray() throws IllegalArgumentException: Buffer size <= 0 when file size is 0.
+      </action>
     </release>
     <release version="2.4" date="2019-07-12" description="New features and bug fix release.">
       <action issue="VFS-690" dev="ggregory" type="add">