commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bode...@apache.org
Subject [commons-compress] branch master updated: COMPRESS-499 properly set the position when truncate is called
Date Sat, 04 Jan 2020 14:03:43 GMT
This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/master by this push:
     new af63d69  COMPRESS-499 properly set the position when truncate is called
af63d69 is described below

commit af63d69043348dcb24d76adcde0e6db61e6657b3
Author: Stefan Bodewig <bodewig@apache.org>
AuthorDate: Sat Jan 4 15:02:57 2020 +0100

    COMPRESS-499 properly set the position when truncate is called
    
    closes #88
---
 src/changes/changes.xml                            |   4 +
 .../utils/SeekableInMemoryByteChannel.java         |  40 +++-
 .../utils/SeekableInMemoryByteChannelTest.java     | 222 ++++++++++++++++++++-
 3 files changed, 251 insertions(+), 15 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 01cbada..2a6c038 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -82,6 +82,10 @@ The <action> type attribute can be add,update,fix,remove.
               due-to="Peter Alfred Lee">
         Added support for reading sparse entries to the TAR package.
       </action>
+      <action issue="COMPRESS-499" type="fix" date="2020-01-04">
+        SeekableInMemoryByteChannel's truncate didn't set position
+        according to the spec in an edge case.
+      </action>
     </release>
     <release version="1.19" date="2019-08-27"
              description="Release 1.19
diff --git a/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java
b/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java
index eece7f5..fd083c4 100644
--- a/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java
+++ b/src/main/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannel.java
@@ -28,9 +28,10 @@ import java.util.concurrent.atomic.AtomicBoolean;
 /**
  * A {@link SeekableByteChannel} implementation that wraps a byte[].
  *
- * <p>When this channel is used for writing an internal buffer grows to accommodate
- * incoming data. A natural size limit is the value of {@link Integer#MAX_VALUE}.
- * Internal buffer can be accessed via {@link SeekableInMemoryByteChannel#array()}.</p>
+ * <p>When this channel is used for writing an internal buffer grows to accommodate
incoming data. The natural size
+ * limit is the value of {@link Integer#MAX_VALUE} and it is not possible to {@link #position(long)
set the position} or
+ * {@link #truncate truncate} to a value bigger than that.  Internal buffer can be accessed
via {@link
+ * SeekableInMemoryByteChannel#array()}.</p>
  *
  * @since 1.13
  * @NotThreadSafe
@@ -74,6 +75,13 @@ public class SeekableInMemoryByteChannel implements SeekableByteChannel
{
         this(new byte[size]);
     }
 
+    /**
+     * Returns this channel's position.
+     *
+     * <p>This method violates the contract of {@link SeekableByteChannel#position()}
as it will not throw any exception
+     * when invoked on a closed channel. Instead it will return the position the channel
had when close has been
+     * called.</p>
+     */
     @Override
     public long position() {
         return position;
@@ -89,24 +97,40 @@ public class SeekableInMemoryByteChannel implements SeekableByteChannel
{
         return this;
     }
 
+    /**
+     * Returns the current size of entity to which this channel is connected.
+     *
+     * <p>This method violates the contract of {@link SeekableByteChannel#size} as
it will not throw any exception when
+     * invoked on a closed channel. Instead it will return the size the channel had when
close has been called.</p>
+     */
     @Override
     public long size() {
         return size;
     }
 
+    /**
+     * Truncates the entity, to which this channel is connected, to the given size.
+     *
+     * <p>This method violates the contract of {@link SeekableByteChannel#truncate}
as it will not throw any exception when
+     * invoked on a closed channel.</p>
+     */
     @Override
     public SeekableByteChannel truncate(long newSize) {
+        if (newSize < 0L || newSize > Integer.MAX_VALUE) {
+            throw new IllegalArgumentException("Size has to be in range 0.. " + Integer.MAX_VALUE);
+        }
         if (size > newSize) {
             size = (int) newSize;
         }
-        repositionIfNecessary();
+        if (position > newSize) {
+            position = (int) newSize;
+        }
         return this;
     }
 
     @Override
     public int read(ByteBuffer buf) throws IOException {
         ensureOpen();
-        repositionIfNecessary();
         int wanted = buf.remaining();
         int possible = size - position;
         if (possible <= 0) {
@@ -186,10 +210,4 @@ public class SeekableInMemoryByteChannel implements SeekableByteChannel
{
         }
     }
 
-    private void repositionIfNecessary() {
-        if (position > size) {
-            position = size;
-        }
-    }
-
 }
diff --git a/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java
b/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java
index 32af308..df2fc83 100644
--- a/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java
+++ b/src/test/java/org/apache/commons/compress/utils/SeekableInMemoryByteChannelTest.java
@@ -18,17 +18,20 @@
  */
 package org.apache.commons.compress.utils;
 
+import org.junit.Ignore;
 import org.junit.Test;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.nio.channels.ClosedChannelException;
+import java.nio.channels.SeekableByteChannel;
 import java.nio.charset.Charset;
 import java.util.Arrays;
 
 import static org.apache.commons.compress.utils.CharsetNames.UTF_8;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 
 public class SeekableInMemoryByteChannelTest {
 
@@ -88,6 +91,7 @@ public class SeekableInMemoryByteChannelTest {
         //then
         assertEquals(0L, readBuffer.position());
         assertEquals(-1, readCount);
+        assertEquals(-1, c.read(readBuffer));
         c.close();
     }
 
@@ -177,7 +181,7 @@ public class SeekableInMemoryByteChannelTest {
         //then
         assertEquals(4L, posAtFour);
         assertEquals(c.size(), posAtTheEnd);
-        assertEquals(posPastTheEnd, posPastTheEnd);
+        assertEquals(testData.length + 1L, posPastTheEnd);
         c.close();
     }
 
@@ -190,13 +194,223 @@ public class SeekableInMemoryByteChannelTest {
         c.close();
     }
 
-    @Test(expected = ClosedChannelException.class)
-    public void shouldThrowExceptionWhenSettingPositionOnClosedChannel() throws IOException
{
+    @Test(expected = IllegalArgumentException.class)
+    public void shouldThrowExceptionWhenTruncatingToIncorrectSize() throws IOException {
         //given
         SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel();
         //when
+        c.truncate(Integer.MAX_VALUE + 1L);
         c.close();
-        c.position(1L);
+    }
+
+    // Contract Tests added in response to https://issues.apache.org/jira/browse/COMPRESS-499
+
+    // https://docs.oracle.com/javase/7/docs/api/java/io/Closeable.html#close()
+
+    /*
+     * <q>If the stream is already closed then invoking this method has no effect.</q>
+     */
+    @Test
+    public void closeIsIdempotent() {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.close();
+            assertFalse(c.isOpen());
+            c.close();
+            assertFalse(c.isOpen());
+        }
+    }
+
+    // https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SeekableByteChannel.html#position()
+
+    /*
+     * <q>ClosedChannelException - If this channel is closed</q>
+     */
+    @Test(expected = ClosedChannelException.class)
+    @Ignore("we deliberately violate the spec")
+    public void throwsClosedChannelExceptionWhenPositionIsReadOnClosedChannel() throws Exception
{
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.close();
+            c.position();
+        }
+    }
+
+    // https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SeekableByteChannel.html#size()
+
+    /*
+     * <q>ClosedChannelException - If this channel is closed</q>
+     */
+    @Test(expected = ClosedChannelException.class)
+    @Ignore("we deliberately violate the spec")
+    public void throwsClosedChannelExceptionWhenSizeIsReadOnClosedChannel() throws Exception
{
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.close();
+            c.size();
+        }
+    }
+
+    // https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SeekableByteChannel.html#position(long)
+
+    /*
+     * <q>ClosedChannelException - If this channel is closed</q>
+     */
+    @Test(expected = ClosedChannelException.class)
+    public void throwsClosedChannelExceptionWhenPositionIsSetOnClosedChannel() throws Exception
{
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.close();
+            c.position(0);
+        }
+    }
+
+    /*
+     * <q>Setting the position to a value that is greater than the current size is
legal but does not change the size of
+     * the entity. A later attempt to read bytes at such a position will immediately return
an end-of-file
+     * indication</q>
+     */
+    @Test
+    public void readingFromAPositionAfterEndReturnsEOF() throws Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.position(2);
+            assertEquals(2, c.position());
+            ByteBuffer readBuffer = ByteBuffer.allocate(5);
+            assertEquals(-1, c.read(readBuffer));
+        }
+    }
+
+    /*
+     * <q>Setting the position to a value that is greater than the current size is
legal but does not change the size of
+     * the entity. A later attempt to write bytes at such a position will cause the entity
to grow to accommodate the
+     * new bytes; the values of any bytes between the previous end-of-file and the newly-written
bytes are
+     * unspecified.</q>
+     */
+    public void writingToAPositionAfterEndGrowsChannel() throws Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.position(2);
+            assertEquals(2, c.position());
+            ByteBuffer inData = ByteBuffer.wrap(testData);
+            assertEquals(testData.length, c.write(inData));
+            assertEquals(testData.length + 2, c.size());
+
+            c.position(2);
+            ByteBuffer readBuffer = ByteBuffer.allocate(testData.length);
+            c.read(readBuffer);
+            assertArrayEquals(testData, Arrays.copyOf(readBuffer.array(), testData.length));
+        }
+    }
+
+    /*
+     * <q>IllegalArgumentException - If the new position is negative</q>
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void throwsIllegalArgumentExceptionWhenPositionIsSetToANegativeValue() throws
Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.position(-1);
+        }
+    }
+
+    // https://docs.oracle.com/javase/7/docs/api/java/nio/channels/SeekableByteChannel.html#truncate(long)
+
+    /*
+     * <q>If the given size is greater than or equal to the current size then the entity
is not modified.</q>
+     */
+    @Test
+    public void truncateToCurrentSizeDoesntChangeAnything() throws Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) {
+            assertEquals(testData.length, c.size());
+            c.truncate(testData.length);
+            assertEquals(testData.length, c.size());
+            ByteBuffer readBuffer = ByteBuffer.allocate(testData.length);
+            assertEquals(testData.length, c.read(readBuffer));
+            assertArrayEquals(testData, Arrays.copyOf(readBuffer.array(), testData.length));
+        }
+    }
+
+    /*
+     * <q>If the given size is greater than or equal to the current size then the entity
is not modified.</q>
+     */
+    @Test
+    public void truncateToBiggerSizeDoesntChangeAnything() throws Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) {
+            assertEquals(testData.length, c.size());
+            c.truncate(testData.length + 1);
+            assertEquals(testData.length, c.size());
+            ByteBuffer readBuffer = ByteBuffer.allocate(testData.length);
+            assertEquals(testData.length, c.read(readBuffer));
+            assertArrayEquals(testData, Arrays.copyOf(readBuffer.array(), testData.length));
+        }
+    }
+
+    /*
+     * <q> In either case, if the current position is greater than the given size then
it is set to that size.</q>
+     */
+    @Test
+    public void truncateDoesntChangeSmallPosition() throws Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) {
+            c.position(1);
+            c.truncate(testData.length - 1);
+            assertEquals(testData.length - 1, c.size());
+            assertEquals(1, c.position());
+        }
+    }
+
+    /*
+     * <q> In either case, if the current position is greater than the given size then
it is set to that size.</q>
+     */
+    @Test
+    public void truncateMovesPositionWhenShrinkingBeyondPosition() throws Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) {
+            c.position(4);
+            c.truncate(3);
+            assertEquals(3, c.size());
+            assertEquals(3, c.position());
+        }
+    }
+
+    /*
+     * <q> In either case, if the current position is greater than the given size then
it is set to that size.</q>
+     */
+    @Test
+    public void truncateMovesPositionWhenNotResizingButPositionBiggerThanSize() throws Exception
{
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) {
+            c.position(2 * testData.length);
+            c.truncate(testData.length);
+            assertEquals(testData.length, c.size());
+            assertEquals(testData.length, c.position());
+        }
+    }
+
+    /*
+     * <q> In either case, if the current position is greater than the given size then
it is set to that size.</q>
+     */
+    @Test
+    public void truncateMovesPositionWhenNewSizeIsBiggerThanSizeAndPositionIsEvenBigger()
throws Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel(testData)) {
+            c.position(2 * testData.length);
+            c.truncate(testData.length + 1);
+            assertEquals(testData.length, c.size());
+            assertEquals(testData.length + 1, c.position());
+        }
+    }
+
+    /*
+     * <q>IllegalArgumentException - If the new position is negative</q>
+     */
+    @Test(expected = IllegalArgumentException.class)
+    public void throwsIllegalArgumentExceptionWhenTruncatingToANegativeSize() throws Exception
{
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.truncate(-1);
+        }
+    }
+
+    /*
+     * <q>ClosedChannelException - If this channel is closed</q>
+     */
+    @Test(expected = ClosedChannelException.class)
+    @Ignore("we deliberately violate the spec")
+    public void throwsClosedChannelExceptionWhenTruncateIsCalledOnClosedChannel() throws
Exception {
+        try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) {
+            c.close();
+            c.truncate(0);
+        }
     }
 
 }


Mime
View raw message