fix: referencing symbolic links with tar archive

[jbrockopp]

Fixes #202

NOTE: Essentially this is a copy of #201

Intended to resolve the issue submitted in the original PR 👍

The only difference should be this PR also has tests as it was stated the original change won't be accepted without them:

#201 (comment)

Please let us know if anything else is required.

[ganeshrvel]

@mholt any updates on this? This has been a blocker for me

[coolaj86]

Hey @jbrockopp @ganeshrvel @technicianted @nishant-handa @kneal!

I've been given the almighty approver status and I'd like to help get this symlink fix pushed through in the next day or two. There's just a few things I'd like to get from you to understand this better:

1. Can you add a symlink to the testdata/corpus folder that demonstrates the problem?
2. Can you give me the output of tree or lsd --tree --icon=never on a directory structure that demonstrates the problem? I expect that should look something like this:

   testdata
   ├── gnu-hardlinks.tar
   ├── sample.rar
   └── corpus
     ├── already-compressed.jpg
     ├── quote1.txt
     └── proverbs
        ├── proverb1.txt
        ├── proverb2-symlink.txt ⇒ proverb2.txt
        ├── proverb2.txt
        └── extra
           └── proverb3.txt
   

3. Will you @jbrockopp git rebase master; git push --force to update the commit to the latest master and trigger the CI/CD ( which I just fixed in Cleanup for CI/CD Pipeline #234 ) to kick off again?

If you can do that for me, I should be able to merge this in right away and get it released this week.

[jbrockopp]

@coolaj86 apologies on getting back to you so late!

I actually completely missed your feedback and a colleague pointed it out today 😅

To start, you requested I rebase with master and as of today (12/30/2020), I believe all commits have been pulled in.

For the sake of transparency, here is a link to our forked repo where that change was submitted:

go-vela#2

The accompanying commit in this PR for that change can be found here:

ec598a9

You also asked if I could add a symlink in the testdata/corpus folder so here is the commit for that:

00a7841

It seems that after adding that symlink file, it has made some tests fail that appear outside my changes?

This makes me want to remove it from testdata/corpus but I'll wait for confirmation from you 👍

Here is what the output for tree looks like now:

$ tree testdata/corpus
testdata/corpus
├── already-compressed.jpg
├── proverbs
│   ├── extra
│   │   └── proverb3.txt
│   ├── proverb1.txt
│   ├── proverb2-symlink.txt -> proverb2.txt
│   └── proverb2.txt
└── quote1.txt

2 directories, 6 files

FWIW:

When I originally submitted this PR, I added a testdata/gnu-symlinks.tar file with symlinks in it for the tests:

• Unarchive
• Extract

Here is what that directory structure looks like:

$ mkdir testdata/gnu-symlinks

$ tar -xf testdata/gnu-symlinks.tar -C testdata/gnu-symlinks

$ tree testdata/gnu-symlinks
testdata/gnu-symlinks
└── dir-1
    └── dir-2
        ├── file-a
        ├── file-b
        ├── file-c -> file-a
        └── file-d -> file-b

2 directories, 4 files

This is setup based off the pre-existing testdata/gnu-hardlinks.tar file which has this directory structure:

$ mkdir testdata/gnu-hardlinks

$ tar -xf testdata/gnu-hardlinks.tar -C testdata/gnu-hardlinks

$ tree testdata/gnu-hardlinks
testdata/gnu-hardlinks
└── dir-1
    └── dir-2
        ├── file-a
        └── file-b

2 directories, 2 files

cc: @ganeshrvel @technicianted @nishant-handa @kneal

[xkisu]

Some progress needs to be made on this asap - it's severely affecting specific use cases in downstream repositories. In https://github.com/pterodactyl/wings/blob/develop/server/archiver.go it's causing server transfers to completely fail if a server has symlinks in its files - merging this patch into a fork solved the issue immediately.

[mholt]

If anyone would like to step up and pledge to help maintain this project in the long-term -- i.e. for more than just one or two PRs -- or commit to sponsor at a sufficiently high tier, that would help things move along faster. Right now my hands are full with Caddy development, unfortunately, and my current sponsors aren't really using this library, they're using Caddy.

[jbrockopp]

@coolaj86 @mholt

I never heard back on whether or not I should remove the symlink from the testdata/corpus folder so I did it anyway:

2f26972

As I suspected, now all status checks are passing 👍

Similar to what @xkisu stated, we were experiencing problems using symlinks with this library:

go-vela/community#30

So we decided to fix it ourselves and then contribute back:

go-vela/vela-s3-cache#20

We haven't experienced any issues since then so is there any reason this can't be merged now?

Tests have been added to verify the functionality as requested in the original PR:

• Fix issue when reading symlink during tar Archive #201 (comment)
• Fix issue when reading symlink during tar Archive #201 (comment)

cc: @ganeshrvel @technicianted @nishant-handa @kneal

[stingalleman]

Hey @mholt, can you look in to this PR? I understand that you are very busy and are searching for new maintainers, but this is such a big issue for us, and it would really help if this gets merged upstream.

Thank you so much for your work on this library, it is truly amazing.

[mholt]

Sure, sorry about that. Could someone fix the merge conflict first?

[mholt]

Probably fixed in #296.