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 <gardgreg...@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">