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 a4daf8bcdfe53760d6dbb8c6630101ec24a99774
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Sat Apr 27 17:19:03 2024 -0400
ArArchiveOutputStream doesn't pad correctly when a file name length is
odd and greater than 16 (padding missing)
ArArchiveOutputStream should check that a file name length greater than
16 fits in a header
---
src/changes/changes.xml | 2 +
.../archivers/ar/ArArchiveOutputStream.java | 65 ++++++-----
.../compress/archivers/ar/Compress678Test.java | 126 +++++++++++++++++++++
3 files changed, 164 insertions(+), 29 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index f10980e24..26dc47e76 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -52,6 +52,8 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary Gregory">Avoid possible
NullPointerException in
org.apache.commons.compress.utils.Sets.newHashSet(E...).</action>
<action type="fix" issue="COMPRESS-677" dev="ggregory" due-to="Jeffrey
Adamson, Gary Gregory">ZipArchiveOutputStream.setEncoding(String) with a null
value throws IllegalArgumentException.</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">org.apache.commons.compress.harmony.unpack200.Archive.unpack() should
not close streams it does not own (when constructed from Archive(InputStream,
JarOutputStream)).</action>
+ <action type="fix" dev="ggregory" due-to="takaaki nakama, Gary
Gregory">ArArchiveOutputStream doesn't pad correctly when a file name length is
odd and greater than 16 (padding missing).</action>
+ <action type="fix" dev="ggregory" due-to="Gary
Gregory">ArArchiveOutputStream should check that a file name length greater
than 16 fits in a header.</action>
<!-- UPDATE -->
<action type="update" dev="ggregory" due-to="Dependabot, Gary
Gregory">Bump org.apache.commons:commons-parent from 66 to 69 #495,
#508.</action>
<action type="update" dev="ggregory" due-to="Dependabot, Gary
Gregory">Bump org.ow2.asm:asm from 9.6 to 9.7 #504.</action>
diff --git
a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
index 1a7e9f605..0753f6891 100644
---
a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
+++
b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveOutputStream.java
@@ -48,8 +48,9 @@ public class ArArchiveOutputStream extends
ArchiveOutputStream<ArArchiveEntry> {
private final OutputStream out;
private long entryOffset;
+ private int headerPlus;
private ArArchiveEntry prevEntry;
- private boolean haveUnclosedEntry;
+ private boolean prevEntryOpen;
private int longFileMode = LONGFILE_ERROR;
/** Indicates if this archive is finished */
@@ -93,13 +94,13 @@ public class ArArchiveOutputStream extends
ArchiveOutputStream<ArArchiveEntry> {
@Override
public void closeArchiveEntry() throws IOException {
checkFinished();
- if (prevEntry == null || !haveUnclosedEntry) {
+ if (prevEntry == null || !prevEntryOpen) {
throw new IOException("No current entry to close");
}
- if (entryOffset % 2 != 0) {
+ if ((headerPlus + entryOffset) % 2 != 0) {
out.write(PAD); // Pad byte
}
- haveUnclosedEntry = false;
+ prevEntryOpen = false;
}
@Override
@@ -119,25 +120,25 @@ public class ArArchiveOutputStream extends
ArchiveOutputStream<ArArchiveEntry> {
return new ArArchiveEntry(inputPath, entryName, options);
}
- private long fill(final long offset, final long newOffset, final char
fill) throws IOException {
- final long diff = newOffset - offset;
- if (diff > 0) {
- for (int i = 0; i < diff; i++) {
- write(fill);
- }
- }
- return newOffset;
- }
-
@Override
public void finish() throws IOException {
- if (haveUnclosedEntry) {
+ if (prevEntryOpen) {
throw new IOException("This archive contains unclosed entries.");
}
checkFinished();
finished = true;
}
+ private int pad(final int offset, final int newOffset, final char fill)
throws IOException {
+ final int diff = newOffset - offset;
+ if (diff > 0) {
+ for (int i = 0; i < diff; i++) {
+ write(fill);
+ }
+ }
+ return newOffset;
+ }
+
@Override
public void putArchiveEntry(final ArArchiveEntry entry) throws IOException
{
checkFinished();
@@ -147,14 +148,14 @@ public class ArArchiveOutputStream extends
ArchiveOutputStream<ArArchiveEntry> {
if (prevEntry.getLength() != entryOffset) {
throw new IOException("Length does not match entry (" +
prevEntry.getLength() + " != " + entryOffset);
}
- if (haveUnclosedEntry) {
+ if (prevEntryOpen) {
closeArchiveEntry();
}
}
prevEntry = entry;
- writeEntryHeader(entry);
+ headerPlus = writeEntryHeader(entry);
entryOffset = 0;
- haveUnclosedEntry = true;
+ prevEntryOpen = true;
}
/**
@@ -185,8 +186,8 @@ public class ArArchiveOutputStream extends
ArchiveOutputStream<ArArchiveEntry> {
out.write(ArchiveUtils.toAsciiBytes(ArArchiveEntry.HEADER));
}
- private void writeEntryHeader(final ArArchiveEntry entry) throws
IOException {
- long offset = 0;
+ private int writeEntryHeader(final ArArchiveEntry entry) throws
IOException {
+ int offset = 0;
boolean appendName = false;
final String eName = entry.getName();
final int nLength = eName.length();
@@ -195,31 +196,37 @@ public class ArArchiveOutputStream extends
ArchiveOutputStream<ArArchiveEntry> {
}
if (LONGFILE_BSD == longFileMode && (nLength > 16 ||
eName.indexOf(SPACE) > -1)) {
appendName = true;
- offset += write(ArArchiveInputStream.BSD_LONGNAME_PREFIX +
nLength);
+ final String fileNameLen =
ArArchiveInputStream.BSD_LONGNAME_PREFIX + nLength;
+ if (fileNameLen.length() > 16) {
+ throw new IOException("File length too long, > 16 chars: " +
eName);
+ }
+ offset += write(fileNameLen);
} else {
offset += write(eName);
}
+ offset = pad(offset, 16, SPACE);
// Last modified
- offset = fill(offset, 16, SPACE);
offset += write(checkLength(String.valueOf(entry.getLastModified()),
12, "Last modified"));
+ offset = pad(offset, 28, SPACE);
// User ID
- offset = fill(offset, 28, SPACE);
offset += write(checkLength(String.valueOf(entry.getUserId()), 6,
"User ID"));
+ offset = pad(offset, 34, SPACE);
// Group ID
- offset = fill(offset, 34, SPACE);
offset += write(checkLength(String.valueOf(entry.getGroupId()), 6,
"Group ID"));
+ offset = pad(offset, 40, SPACE);
// Mode
- offset = fill(offset, 40, SPACE);
offset +=
write(checkLength(String.valueOf(Integer.toString(entry.getMode(), 8)), 8,
"File mode"));
- // Length
- offset = fill(offset, 48, SPACE);
+ offset = pad(offset, 48, SPACE);
+ // Size
+ // On overflow, the file size is incremented by the length of the name.
offset += write(checkLength(String.valueOf(entry.getLength() +
(appendName ? nLength : 0)), 10, "Size"));
- // Trailer
- offset = fill(offset, 58, SPACE);
+ offset = pad(offset, 58, SPACE);
offset += write(ArArchiveEntry.TRAILER);
+ // Name
if (appendName) {
offset += write(eName);
}
+ return offset;
}
}
diff --git
a/src/test/java/org/apache/commons/compress/archivers/ar/Compress678Test.java
b/src/test/java/org/apache/commons/compress/archivers/ar/Compress678Test.java
new file mode 100644
index 000000000..0fb52a532
--- /dev/null
+++
b/src/test/java/org/apache/commons/compress/archivers/ar/Compress678Test.java
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.compress.archivers.ar;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+
+import org.apache.commons.lang3.StringUtils;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Tests COMPRESS-678.
+ */
+public class Compress678Test {
+
+ @ParameterizedTest
+ @ValueSource(ints = { 15, 16, 17, 18, 32, 64, 128 })
+ public void test_LONGFILE_BSD(final int fileNameLen) throws IOException {
+ test_LONGFILE_BSD(StringUtils.repeat('x', fileNameLen));
+ }
+
+ /**
+ * @param fileName
+ * @throws IOException
+ * @throws FileNotFoundException
+ */
+ private void test_LONGFILE_BSD(final String fileName) throws IOException,
FileNotFoundException {
+ final File file = new File("target/Compress678Test-b.ar");
+ Files.deleteIfExists(file.toPath());
+ // First entry's name length is longer than 16 bytes and odd
+ // data length is odd.
+ final byte[] data = { 1 };
+ try (ArArchiveOutputStream arOut = new ArArchiveOutputStream(new
FileOutputStream(file))) {
+ arOut.setLongFileMode(ArArchiveOutputStream.LONGFILE_BSD);
+ // entry 1
+ arOut.putArchiveEntry(new ArArchiveEntry(fileName, data.length));
+ arOut.write(data);
+ arOut.closeArchiveEntry();
+ // entry 2
+ arOut.putArchiveEntry(new ArArchiveEntry("a", data.length));
+ arOut.write(data);
+ arOut.closeArchiveEntry();
+ }
+ try (ArArchiveInputStream arIn = new ArArchiveInputStream(new
FileInputStream(file))) {
+ final ArArchiveEntry entry = arIn.getNextEntry();
+ assertEquals(fileName, entry.getName());
+ // Fix
+ // ar -tv Compress678Test-b.ar
+ // rw-r--r-- 0/0 1 Apr 27 16:10 2024 01234567891234567
+ // ar: Compress678Test-b.ar: Inappropriate file type or format
+ assertNotNull(arIn.getNextEntry());
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = { 15, 16, 17, 18, 32, 64, 128 })
+ public void test_LONGFILE_BSD_with_spaces(final int fileNameLen) throws
IOException {
+ test_LONGFILE_BSD(StringUtils.repeat("x y", fileNameLen / 3));
+ }
+
+ @Test
+ public void test_LONGFILE_ERROR() throws IOException {
+ final File file = new File("target/Compress678Test-a.ar");
+ Files.deleteIfExists(file.toPath());
+ // First entry's name length is longer than 16 bytes and odd
+ final String name1 = "01234567891234567";
+ // data length is odd.
+ final byte[] data = { 1 };
+ try (ArArchiveOutputStream arOut = new ArArchiveOutputStream(new
FileOutputStream(file))) {
+ arOut.setLongFileMode(ArArchiveOutputStream.LONGFILE_ERROR);
+ // java.io.IOException: File name too long, > 16 chars:
01234567891234567
+ assertThrows(IOException.class, () -> arOut.putArchiveEntry(new
ArArchiveEntry(name1, data.length)));
+ }
+ }
+
+ @Test
+ public void testShortName() throws IOException {
+ final File file = new File("target/Compress678Test-c.ar");
+ Files.deleteIfExists(file.toPath());
+ // First entry's name length is <= than 16 bytes and odd
+ final String name1 = "0123456789123456";
+ // data length is odd.
+ final byte[] data = { 1 };
+ try (ArArchiveOutputStream arOut = new ArArchiveOutputStream(new
FileOutputStream(file))) {
+ arOut.setLongFileMode(ArArchiveOutputStream.LONGFILE_BSD);
+ // entry 1
+ arOut.putArchiveEntry(new ArArchiveEntry(name1, data.length));
+ arOut.write(data);
+ arOut.closeArchiveEntry();
+ // entry 2
+ arOut.putArchiveEntry(new ArArchiveEntry("a", data.length));
+ arOut.write(data);
+ arOut.closeArchiveEntry();
+ }
+ try (ArArchiveInputStream arIn = new ArArchiveInputStream(new
FileInputStream(file))) {
+ final ArArchiveEntry entry = arIn.getNextEntry();
+ assertEquals(name1, entry.getName());
+ assertNotNull(arIn.getNextEntry());
+ }
+ }
+}
