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.

Sign up for GitHub

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

Jump to bottom

Possible bug in "tar" Go package: Vagrant box is corrupted #1590

Closed
legal90 opened this issue Oct 15, 2014 · 3 comments
Closed

Possible bug in "tar" Go package: Vagrant box is corrupted #1590

legal90 opened this issue Oct 15, 2014 · 3 comments
Labels
bug post-processor/vagrant

Comments

@legal90
Copy link
Contributor

legal90 commented Oct 15, 2014

This issue relates to Vagrant post-processor and, probably caused by bug (or simple limitation) in "tar" Go package.

Description

Vagrant post-processor prepares tar archive (Vagrant box) and set header.Name as relative path name to the file: post-processor/vagrant/util.go#L107. It looks good and tar library is recommend to do so if we want to keep nested dir structure (see comment):

But on the other hand, tar library is limiting this Name to 100 characters and if this limit is reached, then Name will be split and truncated to base name: tar/writer.go#L207.

        if len(hdr.Name) > fileNameSize && isASCII(hdr.Name) {
            var err error
            prefix, suffix, err = tw.splitUSTARLongName(hdr.Name)
            if err == nil {
                // ok we can use a ustar long name instead of pax, now correct the fields

                // remove the path field from the pax header. this will suppress the pax header
                delete(paxHeaders, paxPath)

                // update the path fields
                tw.cString(pathHeaderBytes, suffix, false, paxNone, nil)
                tw.cString(prefixHeaderBytes, prefix, false, paxNone, nil)

                // Use the ustar magic if we used ustar long names.
                if len(prefix) > 0 && !tw.usedBinary {
                    copy(header[257:265], []byte("ustar\x00"))
                }
            }
        }

Limits are defined here: tar/common.go#L66

// File name constants from the tar spec.
const (
    fileNameSize       = 100 // Maximum number of bytes in a standard tar name.
    fileNamePrefixSize = 155 // Maximum number of ustar extension bytes.
)

I'm confused that Name is limited exactly to fileNameSize instead of fileNameSize + fileNamePrefixSize. Go masters, please, take a look at this and help me to understand - is it a library bug, or not?

I've noticed that the code above has been added by tar library revision 0c7e4c45acf8 as a fix for tar library issue 6056

How does it affect Packer

We have a long file names in Parallels Desktop VM image. Being concatenated with relative path, these names could be longer than 100 characters. In the result such files are placed to the root of tar archive without saving nested dir structure. So that, Vagrant box is corrupted but don't have any notification about that. See examples:

  1. Everything is OK. Length of header's packer-centos-6.5-x86_64.pvm/harddisk.hdd/harddisk.hdd.0.{5fbaabe3-6958-40ff-92a7-860e329aab41}.hds is 99 characters
### Part of verbose packer output
### ...
==> Builds finished. The artifacts of successful builds are:
2014/10/14 14:45:54 machine readable: parallels-iso,artifact-count []string{"2"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "builder-id", "packer.parallels"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "id", "VM"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "string", "VM files in directory: packer-centos-6.5-x86_64-parallels"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "files-count", "6"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "file", "0", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/VmInfo.pvi"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "file", "1", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/config.pvs"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "file", "2", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk.hdd/DiskDescriptor.xml"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "file", "3", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk.hdd/harddisk.hdd"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "file", "4", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk.hdd/harddisk.hdd.0.{5fbaabe3-6958-40ff-92a7-860e329aab41}.hds"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "file", "5", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk.hdd/harddisk.hdd.drh"}
2014/10/14 14:45:54 machine readable: parallels-iso,artifact []string{"0", "end"}
### ...

### We're trying to check the result. Everything is OK
$ tar tzvf ../builds/parallels/parallels_centos-6.5_chef-provisionerless.box
-rw-r--r--  0 487752610  1077286973 220 Oct 14 14:44 Vagrantfile
-rw-r--r--  0 487752610  1077286973  25 Oct 14 14:44 metadata.json
-rw-r--r--  0 487752610  1077286973 6672 Oct 14 14:44 packer-centos-6.5-x86_64.pvm/VmInfo.pvi
-rw-r--r--  0 487752610  1077286973 23430 Oct 14 14:44 packer-centos-6.5-x86_64.pvm/config.pvs
-rw-r--r--  0 487752610  1077286973  1680 Oct 14 14:44 packer-centos-6.5-x86_64.pvm/harddisk.hdd/DiskDescriptor.xml
-rw-r--r--  0 487752610  1077286973     0 Oct 14 14:44 packer-centos-6.5-x86_64.pvm/harddisk.hdd/harddisk.hdd
-rw-r--r--  0 487752610  1077286973 1531969536 Oct 14 14:44 packer-centos-6.5-x86_64.pvm/harddisk.hdd/harddisk.hdd.0.{5fbaabe3-6958-40ff-92a7-860e329aab41}.hds
-rw-r--r--  0 487752610  1077286973       8992 Oct 14 14:44 packer-centos-6.5-x86_64.pvm/harddisk.hdd/harddisk.hdd.drh
  1. Failed. Length of header's Name packer-centos-6.5-x86_64.pvm/harddisk1.hdd/harddisk1.hdd.0.{5fbaabe3-6958-40ff-92a7-860e329aab41}.hds is 101 characters. In the result it is truncated to base name harddisk1.hdd.0.{5fbaabe3-6958-40ff-92a7-860e329aab41}.hds, so that it is placed to the root dir
### Part of verbose packer output
### ...
==> Builds finished. The artifacts of successful builds are:
2014/10/14 15:23:03 machine readable: parallels-iso,artifact-count []string{"2"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "builder-id", "packer.parallels"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "id", "VM"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "string", "VM files in directory: packer-centos-6.5-x86_64-parallels"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "files-count", "6"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "file", "0", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/VmInfo.pvi"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "file", "1", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/config.pvs"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "file", "2", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk1.hdd/DiskDescriptor.xml"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "file", "3", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk1.hdd/harddisk1.hdd"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "file", "4", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk1.hdd/harddisk1.hdd.0.{5fbaabe3-6958-40ff-92a7-860e329aab41}.hds"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "file", "5", "packer-centos-6.5-x86_64-parallels/packer-centos-6.5-x86_64.pvm/harddisk1.hdd/harddisk1.hdd.drh"}
2014/10/14 15:23:03 machine readable: parallels-iso,artifact []string{"0", "end"}
### ...

### We're trying to check the result. See that big file is placed in the root
$ tar tzvf ../builds/parallels/parallels_centos-6.5_chef-provisionerless.box
-rw-r--r--  0 487752610  1077286973 220 Oct 14 15:20 Vagrantfile
-rw-r--r--  0 487752610  1077286973  25 Oct 14 15:20 metadata.json
-rw-r--r--  0 487752610  1077286973 6672 Oct 14 15:20 packer-centos-6.5-x86_64.pvm/VmInfo.pvi
-rw-r--r--  0 487752610  1077286973 23433 Oct 14 15:20 packer-centos-6.5-x86_64.pvm/config.pvs
-rw-r--r--  0 487752610  1077286973  1486 Oct 14 15:20 packer-centos-6.5-x86_64.pvm/harddisk1.hdd/DiskDescriptor.xml
-rw-r--r--  0 487752610  1077286973     0 Oct 14 15:20 packer-centos-6.5-x86_64.pvm/harddisk1.hdd/harddisk1.hdd
-rw-r--r--  0 487752610  1077286973 2261778432 Oct 14 15:20 harddisk1.hdd.0.{5fbaabe3-6958-40ff-92a7-860e329aab41}.hds
-rw-r--r--  0 487752610  1077286973       9088 Oct 14 15:20 packer-centos-6.5-x86_64.pvm/harddisk1.hdd/harddisk1.hdd.drh

@mitchellh, What do you think about that? Is it possible to workaround this issue in Packer?
/cc: @rickard-von-essen
@legal90
Copy link
Contributor Author

legal90 commented Oct 23, 2014

To contributors: Could you please check it out before Packer 0.7.2 release?

@mitchellh
Copy link
Contributor

This looks correct. Tagging as a bug.

@mitchellh
Copy link
Contributor

Hm, you're right, this does look like a Go bug. Can you ask them? I took a closer look and can't see anything obviously wrong with our code regarding lengths. I could be wrong though.

@legal90 legal90 mentioned this issue Jun 12, 2015
Closed
@cheeseplus cheeseplus mentioned this issue Jul 11, 2015
Merged
gildas pushed a commit to gildas/packer-windows that referenced this issue May 18, 2016
use short vm_names with Parallels Desktop (hashicorp/packer#1590)
6dc0a87
@lawrence-c lawrence-c mentioned this issue Mar 23, 2018
Closed
@ghost ghost locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug post-processor/vagrant
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mitchellh @legal90