Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ protected Logger getLogger() {
*/
private boolean ignorePermissions = false;

private boolean warnCannotHardlink = true;

public AbstractUnArchiver() {
// no op
}
Expand Down Expand Up @@ -278,8 +280,9 @@ protected void extractFile(
String entryName,
final Date entryDate,
final boolean isDirectory,
final boolean isSymlink,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is breaking change. I wonder if we can avoid it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how.

final Integer mode,
String symlinkDestination,
String linkDestination,
final FileMapper[] fileMappers)
throws IOException, ArchiverException {
if (fileMappers != null) {
Expand Down Expand Up @@ -312,11 +315,30 @@ protected void extractFile(
dirF.mkdirs();
}

if (!StringUtils.isEmpty(symlinkDestination)) {
SymlinkUtils.createSymbolicLink(targetFileName, new File(symlinkDestination));
boolean doCopy = true;
if (!StringUtils.isEmpty(linkDestination)) {
if (isSymlink) {
SymlinkUtils.createSymbolicLink(targetFileName, new File(linkDestination));
doCopy = false;
} else {
try {
Files.createLink(
targetFileName.toPath(),
FileUtils.resolveFile(dir, linkDestination).toPath());
doCopy = false;
} catch (final UnsupportedOperationException ex) {
if (warnCannotHardlink) {
getLogger().warn("Creating hardlinks is not supported");
warnCannotHardlink = false;
}
// We will do a copy instead.
}
Comment on lines +324 to +335
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardlink creation code only catches UnsupportedOperationException but Files.createLink() can throw IOException for various error conditions (e.g., file already exists, insufficient permissions, I/O errors). These IOExceptions will propagate up and may not be handled appropriately, potentially causing the extraction to fail when it could fall back to copying the file content instead. Consider also catching IOException and falling back to copying in that case.

Copilot uses AI. Check for mistakes.
}
} else if (isDirectory) {
targetFileName.mkdirs();
} else {
doCopy = false;
}
Comment on lines +318 to +340
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardlink creation logic has several issues:

  1. Only UnsupportedOperationException is caught, but Files.createLink() can throw IOException for various error conditions (e.g., file already exists, insufficient permissions, I/O errors), which should also be caught to fall back to copying.
  2. After creating links (hardlinks or symlinks), the code continues to execute outside this block and will call setLastModified() and chmod() on the link at lines 347 and 350. On most platforms, these operations on links affect the target file, not the link itself, which is likely not the intended behavior.

Consider catching IOException in addition to UnsupportedOperationException, and tracking whether a link was created to avoid calling setLastModified() and chmod() on links.

Copilot uses AI. Check for mistakes.
if (doCopy) {
try (OutputStream out = Files.newOutputStream(targetFileName.toPath())) {
IOUtil.copy(compressedInputStream, out);
}
Expand Down
52 changes: 48 additions & 4 deletions src/main/java/org/codehaus/plexus/archiver/tar/TarArchiver.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.LinkOption;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributeView;
import java.util.HashMap;
import java.util.Map;
import java.util.zip.GZIPOutputStream;

import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
Expand All @@ -39,6 +44,7 @@
import org.codehaus.plexus.archiver.util.Streams;
import org.codehaus.plexus.components.io.attributes.PlexusIoResourceAttributes;
import org.codehaus.plexus.components.io.functions.SymlinkDestinationSupplier;
import org.codehaus.plexus.components.io.resources.PlexusIoFileResource;
import org.codehaus.plexus.components.io.resources.PlexusIoResource;
import org.codehaus.plexus.util.IOUtil;
import org.codehaus.plexus.util.StringUtils;
Expand All @@ -65,6 +71,8 @@ public class TarArchiver extends AbstractArchiver {

private TarArchiveOutputStream tOut;

private final Map<Object, String> seenFiles = new HashMap<>(10);
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The seenFiles map is never cleared between archive creation operations. If a TarArchiver instance is reused to create multiple archives, the map will contain stale file keys from previous archives, potentially causing incorrect hardlink references across different archive creation operations. Consider clearing the map at the beginning of the execute() method or in the cleanUp() method.

Copilot uses AI. Check for mistakes.

/**
* Set how to handle long files, those with a path&gt;100 chars.
* Optional, default=warn.
Expand Down Expand Up @@ -194,7 +202,7 @@ protected void tarFile(ArchiveEntry entry, TarArchiveOutputStream tOut, String v
InputStream fIn = null;

try {
TarArchiveEntry te;
TarArchiveEntry te = null;
if (!longFileMode.isGnuMode()
&& pathLength >= org.apache.commons.compress.archivers.tar.TarConstants.NAMELEN) {
int maxPosixPathLen = org.apache.commons.compress.archivers.tar.TarConstants.NAMELEN
Expand Down Expand Up @@ -233,13 +241,39 @@ protected void tarFile(ArchiveEntry entry, TarArchiveOutputStream tOut, String v
}
}

boolean isLink = false;
if (entry.getType() == ArchiveEntry.SYMLINK) {
final SymlinkDestinationSupplier plexusIoSymlinkResource =
(SymlinkDestinationSupplier) entry.getResource();

te = new TarArchiveEntry(vPath, TarArchiveEntry.LF_SYMLINK);
te.setLinkName(plexusIoSymlinkResource.getSymlinkDestination());
} else {
isLink = true;
} else if (options.getPreserveHardLinks()
&& entry.getResource().isFile()
&& entry.getResource() instanceof PlexusIoFileResource) {
final PlexusIoFileResource fileResource = (PlexusIoFileResource) entry.getResource();
final Path file = fileResource.getFile().toPath();
if (Files.exists(file)) {
final BasicFileAttributeView fileAttributeView =
Files.getFileAttributeView(file, BasicFileAttributeView.class, LinkOption.NOFOLLOW_LINKS);
if (fileAttributeView != null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be best to extract this check to separate method to make this method easier to read.

final Object fileKey =
fileAttributeView.readAttributes().fileKey();
if (fileKey != null) {
final String seenFile = this.seenFiles.get(fileKey);
if (seenFile != null) {
te = new TarArchiveEntry(vPath, TarArchiveEntry.LF_LINK);
te.setLinkName(seenFile);
isLink = true;
} else {
this.seenFiles.put(fileKey, vPath);
}
}
}
}
}
if (te == null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I don't follow why this is added.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

te is assigned either from the symlink branch or the hardlink branch. But the hardlink branch can also not assign value at all. This handles that case plus all other cases of non-symlink&non-hardlink.

te = new TarArchiveEntry(vPath);
}

Expand All @@ -253,7 +287,7 @@ protected void tarFile(ArchiveEntry entry, TarArchiveOutputStream tOut, String v
te.setModTime(getLastModifiedTime().toMillis());
}

if (entry.getType() == ArchiveEntry.SYMLINK) {
if (isLink) {
te.setSize(0);

} else if (!entry.getResource().isDirectory()) {
Expand Down Expand Up @@ -289,7 +323,7 @@ protected void tarFile(ArchiveEntry entry, TarArchiveOutputStream tOut, String v
tOut.putArchiveEntry(te);

try {
if (entry.getResource().isFile() && !(entry.getType() == ArchiveEntry.SYMLINK)) {
if (entry.getResource().isFile() && !isLink) {
fIn = entry.getInputStream();

Streams.copyFullyDontCloseOutput(fIn, tOut, "xAR");
Expand Down Expand Up @@ -320,6 +354,8 @@ public class TarOptions {

private boolean preserveLeadingSlashes = false;

private boolean preserveHardLinks = true;

/**
* The username for the tar entry
* This is not the same as the UID.
Expand Down Expand Up @@ -405,6 +441,14 @@ public boolean getPreserveLeadingSlashes() {
public void setPreserveLeadingSlashes(boolean preserveLeadingSlashes) {
this.preserveLeadingSlashes = preserveLeadingSlashes;
}

public boolean getPreserveHardLinks() {
return preserveHardLinks;
}

public void setPreserveHardLinks(boolean preserveHardLinks) {
this.preserveHardLinks = preserveHardLinks;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,15 @@ protected void execute(File sourceFile, File destDirectory, FileMapper[] fileMap
while ((te = tis.getNextTarEntry()) != null) {
TarResource fileInfo = new TarResource(tarFile, te);
if (isSelected(te.getName(), fileInfo)) {
final String symlinkDestination = te.isSymbolicLink() ? te.getLinkName() : null;
final String symlinkDestination = te.isSymbolicLink() || te.isLink() ? te.getLinkName() : null;
extractFile(
sourceFile,
destDirectory,
tis,
te.getName(),
te.getModTime(),
te.isDirectory(),
te.isSymbolicLink(),
te.getMode() != 0 ? te.getMode() : null,
symlinkDestination,
fileMappers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ protected void execute(final String path, final File outputDirectory) throws Arc
ze.getName(),
new Date(ze.getTime()),
ze.isDirectory(),
ze.isUnixSymlink(),
ze.getUnixMode() != 0 ? ze.getUnixMode() : null,
resolveSymlink(zipFile, ze),
getFileMappers());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void shouldThrowExceptionBecauseRewrittenPathIsOutOfDirectory(@TempDir Fi
Exception exception = assertThrows(
ArchiverException.class,
() -> abstractUnArchiver.extractFile(
null, targetFolder, null, "ENTRYNAME", null, false, null, null, fileMappers));
null, targetFolder, null, "ENTRYNAME", null, false, false, null, null, fileMappers));
// then
// ArchiverException is thrown providing the rewritten path
assertEquals(
Expand Down
53 changes: 53 additions & 0 deletions src/test/java/org/codehaus/plexus/archiver/HardlinkTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.codehaus.plexus.archiver;

import java.io.File;
import java.nio.file.Files;

import org.codehaus.plexus.archiver.tar.TarArchiver;
import org.codehaus.plexus.archiver.tar.TarLongFileMode;
import org.codehaus.plexus.archiver.tar.TarUnArchiver;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;

import static org.junit.jupiter.api.Assertions.assertTrue;

/**
* @author Kristian Rosenvold
*/
public class HardlinkTest extends TestSupport {

@Test
@DisabledOnOs(OS.WINDOWS)
public void testHardlinkTar() throws Exception {
// Extract test files
final File archiveFile = getTestFile("src/test/resources/hardlinks/hardlinks.tar");
File output = getTestFile("target/output/untaredHardlinks");
output.mkdirs();
TarUnArchiver unarchiver = (TarUnArchiver) lookup(UnArchiver.class, "tar");
unarchiver.setSourceFile(archiveFile);
unarchiver.setDestFile(output);
unarchiver.extract();
// Check that we have hardlinks
assertTrue(Files.isSameFile(
output.toPath().resolve("fileR.txt"), output.toPath().resolve("hardlink")));

// Archive the extracted hardlinks to new archive
TarArchiver archiver = (TarArchiver) lookup(Archiver.class, "tar");
archiver.setLongfile(TarLongFileMode.posix);
archiver.addDirectory(output);
final File testFile = getTestFile("target/output/untaredHardlinks2.tar");
archiver.setDestFile(testFile);
archiver.createArchive();

// Check that our created archive actually contains hardlinks when extracted
unarchiver = (TarUnArchiver) lookup(UnArchiver.class, "tar");
output = getTestFile("target/output/untaredHardlinks2");
output.mkdirs();
unarchiver.setSourceFile(testFile);
unarchiver.setDestFile(output);
unarchiver.extract();
assertTrue(Files.isSameFile(
output.toPath().resolve("fileR.txt"), output.toPath().resolve("hardlink")));
}
}
10 changes: 5 additions & 5 deletions src/test/java/org/codehaus/plexus/archiver/tar/TarFileTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.Enumeration;

import org.apache.commons.compress.archivers.ArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.codehaus.plexus.archiver.Archiver;
import org.codehaus.plexus.archiver.TestSupport;
Expand All @@ -18,7 +18,7 @@
import org.junit.jupiter.api.Test;

import static org.codehaus.plexus.components.io.resources.ResourceFactory.createResource;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;

/**
* Test case for {@link TarFile}.
Expand Down Expand Up @@ -92,15 +92,15 @@ private void testTarFile(Compressor compressor, String extension, TarFileCreator
}
final TarFile tarFile = tarFileCreator.newTarFile(file);

for (Enumeration en = tarFile.getEntries(); en.hasMoreElements(); ) {
for (Enumeration<ArchiveEntry> en = tarFile.getEntries(); en.hasMoreElements(); ) {
final TarArchiveEntry te = (TarArchiveEntry) en.nextElement();
if (te.isDirectory() || te.isSymbolicLink()) {
if (te.isDirectory() || te.isSymbolicLink() || te.isLink()) {
continue;
}
final File teFile = new File("src", te.getName());
final InputStream teStream = tarFile.getInputStream(te);
final InputStream fileStream = Files.newInputStream(teFile.toPath());
assertTrue(Arrays.equals(IOUtil.toByteArray(teStream), IOUtil.toByteArray(fileStream)));
assertArrayEquals(IOUtil.toByteArray(teStream), IOUtil.toByteArray(fileStream));
teStream.close();
fileStream.close();
}
Expand Down
Binary file added src/test/resources/hardlinks/hardlinks.tar
Binary file not shown.
4 changes: 3 additions & 1 deletion src/test/resources/symlinks/regen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ rm symlinks.tar
cd src
zip --symlinks ../symlinks.zip file* targetDir sym*
tar -cvf ../symlinks.tar file* targetDir sym*

rm hardlink
ln fileR.txt hardlink
tar -cvf ../../hardlinks/hardlinks.tar fileR.txt hardlink