Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TAR: Implement extraction and archiving of hardlinks. #286

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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,
Copy link
Member

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.
}
}
} else if (isDirectory) {
targetFileName.mkdirs();
} else {
doCopy = false;
}
if (doCopy) {
try (OutputStream out = Files.newOutputStream(targetFileName.toPath())) {
IOUtil.copy(compressedInputStream, out);
}
Expand Down
63 changes: 53 additions & 10 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,10 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Files;
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 +43,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 +70,8 @@ public class TarArchiver extends AbstractArchiver {

private TarArchiveOutputStream tOut;

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

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

if (entry.getResource().isDirectory() && !vPath.endsWith("/")) {
final PlexusIoResource ioResource = entry.getResource();
if (ioResource.isDirectory() && !vPath.endsWith("/")) {
vPath += "/";
}

Expand All @@ -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,18 +241,43 @@ protected void tarFile(ArchiveEntry entry, TarArchiveOutputStream tOut, String v
}
}

boolean doCopy = true;
Copy link
Member

Choose a reason for hiding this comment

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

I found this name confusing. isLink, or something else, probably would be more appropriate as it seems that the flag indicates whether the entry is link (symbolic or hard) to some other entry.

if (entry.getType() == ArchiveEntry.SYMLINK) {
final SymlinkDestinationSupplier plexusIoSymlinkResource =
(SymlinkDestinationSupplier) entry.getResource();
final SymlinkDestinationSupplier plexusIoSymlinkResource = (SymlinkDestinationSupplier) ioResource;
Copy link
Member

Choose a reason for hiding this comment

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

Is this change related to hard links? It seems that it is just refactoring to avoid calling entry.getResource() multiple times. If this is the case I would rather have this in separate PR or at least separate commit as the change is already complex enough to understand on its own.


te = new TarArchiveEntry(vPath, TarArchiveEntry.LF_SYMLINK);
te.setLinkName(plexusIoSymlinkResource.getSymlinkDestination());
} else {
doCopy = false;
} else if (options.getPreserveHardLinks()
&& ioResource.isFile()
&& ioResource instanceof PlexusIoFileResource) {
final PlexusIoFileResource fileResource = (PlexusIoFileResource) ioResource;
final Path file = fileResource.getFile().toPath();
if (Files.exists(file)) {
final BasicFileAttributeView fileAttributeView =
Files.getFileAttributeView(file, BasicFileAttributeView.class);
Copy link
Member

Choose a reason for hiding this comment

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

I would rather use LinkOption.NOFOLLOW_LINKS to be on the safe side, although it seems that symbolic links are already handled (haven't tested it).

if (fileAttributeView != null) {
Copy link
Member

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);
doCopy = false;
} else {
this.seenFiles.put(fileKey, vPath);
}
}
}
}
}
if (te == null) {
Copy link
Member

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);
}

if (getLastModifiedTime() == null) {
long teLastModified = entry.getResource().getLastModified();
long teLastModified = ioResource.getLastModified();
te.setModTime(
teLastModified == PlexusIoResource.UNKNOWN_MODIFICATION_DATE
? System.currentTimeMillis()
Expand All @@ -253,11 +286,11 @@ protected void tarFile(ArchiveEntry entry, TarArchiveOutputStream tOut, String v
te.setModTime(getLastModifiedTime().toMillis());
}

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

} else if (!entry.getResource().isDirectory()) {
final long size = entry.getResource().getSize();
} else if (!ioResource.isDirectory()) {
final long size = ioResource.getSize();
te.setSize(size == PlexusIoResource.UNKNOWN_RESOURCE_SIZE ? 0 : size);
}
te.setMode(entry.getMode());
Expand Down Expand Up @@ -289,7 +322,7 @@ protected void tarFile(ArchiveEntry entry, TarArchiveOutputStream tOut, String v
tOut.putArchiveEntry(te);

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

Streams.copyFullyDontCloseOutput(fIn, tOut, "xAR");
Expand Down Expand Up @@ -320,6 +353,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 +440,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