archive/zip: AddFS with a symlink results in an empty file

[mauri870]

What version of Go are you using (go version)?

$ go version
tip

Does this issue reproduce with the latest release?

Yes, this is a tip (1.22) only issue.

What did you do?

Tried to call AddFS with a fs.FS with symlinks.

package main

import (
	"archive/zip"
	"log"
	"os"
)

func main() {
	fsys := os.DirFS("./dir") // ./dir contains symlinks inside
	f, err := os.OpenFile("file.zip", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755)
	if err != nil {
		log.Fatal(err)
	}
	defer f.Close()

	zw := zip.NewWriter(f)
	defer zw.Close()
	zw.AddFS(fsys)
}

What did you expect to see?

Either an error or the proper file contents.

What did you see instead?

Files that are symlinks result in an empty file inside the zip archive.

Had a discussion on this CL about symlink support in zip files.
We have the proposal for fs.ReadLinkFS that could somewhat make this possible to implement, but the proposal is currently on hold.

I see a couple options to be weighted:

• Keep it as it is, file gets added to the zip but is empty
• Detect files that are symlinks and then return an error upon calling AddFS
• Don't return an error and don't add the file to the zip. This seems to be the behavior with the zip command line tool. There is a logged message zip warning: name not matched: ./dir/iamasymlink but the zip gets created, skipping the symlinks.

[ianlancetaylor]

CC @dsnet

[dsnet]

Of the options, encoding an empty file seems to be the worst.

I'd argue for returning an error for the time being. It is generally more acceptable to convert an error into a properly handled case in the future, than the other way around. I'm not opposed to just ignoring non-regular files as well.

There is an seldom used extension to zip that can encode Unix-specific files like symlinks, devices, etc., but it's not clear to me that we should implement that.

I can imagine a community of helper fs.FS functions that would make handling this easier. For example, you could imagine a function that wraps fs.FS and returns one that only reports regular files. Such a helper seems broadly useful.

[mauri870]

Proposal for fs.ReadLinkFS seems promising. We could try to type assert it and then read from the source of the symlink, then fs.FS should work as expected.

As you said from the choices we have returning an error is the most correct way. In the future if we support this the transition will be seamless.

Iwill push a fix for the AddFS in archive/zip as well as fixing my open CL in archive/tar

[gopherbot]

Change https://go.dev/cl/517475 mentions this issue: archive/zip: AddFS can't process filesystem with symlinks