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-compress.git
commit 28cd44bc4fe05bc4c487268c789ced88b76e1c0b
Author: Gary D. Gregory <garydgreg...@gmail.com>
AuthorDate: Mon Mar 17 09:12:40 2025 -0400
Some ZIP operations won't read all data from a non-blocking file channel
---
src/changes/changes.xml | 1 +
.../apache/commons/compress/archivers/zip/ZipIoUtil.java | 16 ++++++++++++----
.../archivers/zip/FileRandomAccessOutputStreamTest.java | 6 +++---
.../zip/SeekableChannelRandomAccessOutputStreamTest.java | 4 ++--
.../commons/compress/archivers/zip/ZipIoUtilTest.java | 8 ++++----
5 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 000ab3dec..1ef938995 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -69,6 +69,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary Gregory">Package-private
and private classes can be final.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate
ArjArchiveEntry.HostOs.HostOs().</action>
<action type="fix" dev="sebb">Drop coveralls reference (no longer
needed)</action>
+ <action type="fix" dev="ggregory" due-to="Gary Gregory">Some ZIP
operations won't read all data from a non-blocking file channel.</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary Gregory">Add
GzipParameters.getModificationInstant().</action>
<action type="add" dev="ggregory" due-to="Gary Gregory">Add
GzipParameters.setModificationInstant(Instant).</action>
diff --git
a/src/main/java/org/apache/commons/compress/archivers/zip/ZipIoUtil.java
b/src/main/java/org/apache/commons/compress/archivers/zip/ZipIoUtil.java
index 4a2fe0bb0..d09416f71 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipIoUtil.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipIoUtil.java
@@ -42,7 +42,11 @@ static void writeAll(final FileChannel channel, final
ByteBuffer buffer, final l
for (long currentPos = position; buffer.hasRemaining();) {
final int remaining = buffer.remaining();
final int written = channel.write(buffer, currentPos);
- if (written <= 0) {
+ if (written == 0) {
+ // A non-blocking channel
+ Thread.yield();
+ continue;
+ } else if (written < 0) {
throw new IOException("Failed to write all bytes in the buffer
for channel=" + channel + ", length=" + remaining + ", written=" + written);
}
currentPos += written;
@@ -52,15 +56,19 @@ static void writeAll(final FileChannel channel, final
ByteBuffer buffer, final l
/**
* Writes all bytes in a buffer to a channel.
*
- * @param channel The target channel.
- * @param buffer The source bytes.
+ * @param channel The target channel.
+ * @param buffer The source bytes.
* @throws IOException If some I/O error occurs or fails or fails to write
all bytes.
*/
static void writeAll(final WritableByteChannel channel, final ByteBuffer
buffer) throws IOException {
while (buffer.hasRemaining()) {
final int remaining = buffer.remaining();
final int written = channel.write(buffer);
- if (written <= 0) {
+ if (written == 0) {
+ // A non-blocking channel
+ Thread.yield();
+ continue;
+ } else if (written < 0) {
throw new IOException("Failed to write all bytes in the buffer
for channel=" + channel + ", length=" + remaining + ", written=" + written);
}
}
diff --git
a/src/test/java/org/apache/commons/compress/archivers/zip/FileRandomAccessOutputStreamTest.java
b/src/test/java/org/apache/commons/compress/archivers/zip/FileRandomAccessOutputStreamTest.java
index 92cc97d54..871122956 100644
---
a/src/test/java/org/apache/commons/compress/archivers/zip/FileRandomAccessOutputStreamTest.java
+++
b/src/test/java/org/apache/commons/compress/archivers/zip/FileRandomAccessOutputStreamTest.java
@@ -128,14 +128,14 @@ public void
testWriteFullyAt_whenFullButPartial_thenSucceed() throws IOException
public void testWriteFullyAt_whenPartial_thenFail() throws IOException {
final FileChannel channel = mock(FileChannel.class);
final FileRandomAccessOutputStream stream = new
FileRandomAccessOutputStream(channel);
- when(channel.write((ByteBuffer) any(), eq(20L))).thenAnswer(answer -> {
+ when(channel.write((ByteBuffer) any(), eq(20L))).thenAnswer(answer ->
0).thenAnswer(answer -> {
((ByteBuffer) answer.getArgument(0)).position(3);
return 3;
});
- when(channel.write((ByteBuffer) any(), eq(23L))).thenAnswer(answer ->
0);
+ when(channel.write((ByteBuffer) any(), eq(23L))).thenAnswer(answer ->
-1);
assertThrows(IOException.class, () ->
stream.writeAll("hello".getBytes(StandardCharsets.UTF_8), 20));
- verify(channel, times(1)).write((ByteBuffer) any(), eq(20L));
+ verify(channel, times(2)).write((ByteBuffer) any(), eq(20L));
verify(channel, times(1)).write((ByteBuffer) any(), eq(23L));
verify(channel, times(0)).write((ByteBuffer) any(), eq(25L));
diff --git
a/src/test/java/org/apache/commons/compress/archivers/zip/SeekableChannelRandomAccessOutputStreamTest.java
b/src/test/java/org/apache/commons/compress/archivers/zip/SeekableChannelRandomAccessOutputStreamTest.java
index 6767483b1..9ca006765 100644
---
a/src/test/java/org/apache/commons/compress/archivers/zip/SeekableChannelRandomAccessOutputStreamTest.java
+++
b/src/test/java/org/apache/commons/compress/archivers/zip/SeekableChannelRandomAccessOutputStreamTest.java
@@ -135,11 +135,11 @@ public void testWriteFullyAt_whenPartial_thenFail()
throws IOException {
when(channel.write((ByteBuffer) any())).thenAnswer(answer -> {
((ByteBuffer) answer.getArgument(0)).position(3);
return 3;
- }).thenAnswer(answer -> 0);
+ }).thenAnswer(answer -> 0).thenAnswer(answer -> -1);
assertThrows(IOException.class, () ->
stream.writeAll("hello".getBytes(StandardCharsets.UTF_8), 20));
- verify(channel, times(2)).write((ByteBuffer) any());
+ verify(channel, times(3)).write((ByteBuffer) any());
assertEquals(50L, stream.position());
}
diff --git
a/src/test/java/org/apache/commons/compress/archivers/zip/ZipIoUtilTest.java
b/src/test/java/org/apache/commons/compress/archivers/zip/ZipIoUtilTest.java
index c485826d3..509bb8f46 100644
--- a/src/test/java/org/apache/commons/compress/archivers/zip/ZipIoUtilTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipIoUtilTest.java
@@ -93,9 +93,9 @@ public void testWriteFully_whenPartial_thenFail() throws
IOException {
when(channel.write((ByteBuffer) any())).thenAnswer(answer -> {
((ByteBuffer) answer.getArgument(0)).position(3);
return 3;
- }).thenAnswer(answer -> 0);
+ }).thenAnswer(answer -> 0).thenAnswer(answer -> -1);
assertThrows(IOException.class, () -> ZipIoUtil.writeAll(channel,
ByteBuffer.wrap("hello".getBytes(StandardCharsets.UTF_8))));
- verify(channel, times(2)).write((ByteBuffer) any());
+ verify(channel, times(3)).write((ByteBuffer) any());
}
}
@@ -147,10 +147,10 @@ public void testWriteFullyAt_whenPartial_thenFail()
throws IOException {
((ByteBuffer) answer.getArgument(0)).position(3);
return 3;
});
- when(channel.write((ByteBuffer) any(), eq(23L))).thenAnswer(answer
-> 0);
+ when(channel.write((ByteBuffer) any(), eq(23L))).thenAnswer(answer
-> 0).thenAnswer(answer -> -1);
assertThrows(IOException.class, () -> ZipIoUtil.writeAll(channel,
ByteBuffer.wrap("hello".getBytes(StandardCharsets.UTF_8)), 20));
verify(channel, times(1)).write((ByteBuffer) any(), eq(20L));
- verify(channel, times(1)).write((ByteBuffer) any(), eq(23L));
+ verify(channel, times(2)).write((ByteBuffer) any(), eq(23L));
verify(channel, times(0)).write((ByteBuffer) any(), eq(25L));
}
}
