From commits-return-71607-archive-asf-public=cust-asf.ponee.io@commons.apache.org Sat Jan 4 14:03:45 2020 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id CF11018065E for ; Sat, 4 Jan 2020 15:03:44 +0100 (CET) Received: (qmail 96651 invoked by uid 500); 4 Jan 2020 14:03:43 -0000 Mailing-List: contact commits-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@commons.apache.org Delivered-To: mailing list commits@commons.apache.org Received: (qmail 96641 invoked by uid 99); 4 Jan 2020 14:03:43 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 04 Jan 2020 14:03:43 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 62A5C8D80D; Sat, 4 Jan 2020 14:03:43 +0000 (UTC) Date: Sat, 04 Jan 2020 14:03:43 +0000 To: "commits@commons.apache.org" Subject: [commons-compress] branch master updated: COMPRESS-499 properly set the position when truncate is called MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <157814662333.22239.13909860900235560443@gitbox.apache.org> From: bodewig@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: commons-compress X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 3449298549c1f59bebb262b476fe42170010cb48 X-Git-Newrev: af63d69043348dcb24d76adcde0e6db61e6657b3 X-Git-Rev: af63d69043348dcb24d76adcde0e6db61e6657b3 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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 type attribute can be add,update,fix,remove. due-to="Peter Alfred Lee"> Added support for reading sparse entries to the TAR package. + + SeekableInMemoryByteChannel's truncate didn't set position + according to the spec in an edge case. + 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() + + /* + * If the stream is already closed then invoking this method has no effect. + */ + @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() + + /* + * ClosedChannelException - If this channel is closed + */ + @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() + + /* + * ClosedChannelException - If this channel is closed + */ + @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) + + /* + * ClosedChannelException - If this channel is closed + */ + @Test(expected = ClosedChannelException.class) + public void throwsClosedChannelExceptionWhenPositionIsSetOnClosedChannel() throws Exception { + try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + c.close(); + c.position(0); + } + } + + /* + * 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 + */ + @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)); + } + } + + /* + * 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. + */ + 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)); + } + } + + /* + * IllegalArgumentException - If the new position is negative + */ + @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) + + /* + * If the given size is greater than or equal to the current size then the entity is not modified. + */ + @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)); + } + } + + /* + * If the given size is greater than or equal to the current size then the entity is not modified. + */ + @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)); + } + } + + /* + * In either case, if the current position is greater than the given size then it is set to that size. + */ + @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()); + } + } + + /* + * In either case, if the current position is greater than the given size then it is set to that size. + */ + @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()); + } + } + + /* + * In either case, if the current position is greater than the given size then it is set to that size. + */ + @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()); + } + } + + /* + * In either case, if the current position is greater than the given size then it is set to that size. + */ + @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()); + } + } + + /* + * IllegalArgumentException - If the new position is negative + */ + @Test(expected = IllegalArgumentException.class) + public void throwsIllegalArgumentExceptionWhenTruncatingToANegativeSize() throws Exception { + try (SeekableInMemoryByteChannel c = new SeekableInMemoryByteChannel()) { + c.truncate(-1); + } + } + + /* + * ClosedChannelException - If this channel is closed + */ + @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); + } } }