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

ZIP Files without subdirectories do not copy correctly #24

Open
dchoi-viant opened this issue Mar 13, 2025 · 3 comments
Open

ZIP Files without subdirectories do not copy correctly #24

dchoi-viant opened this issue Mar 13, 2025 · 3 comments
Assignees

Comments

@dchoi-viant
Copy link
Contributor

Expected Behavior

ZIP Files are extracted and mapped to the local file system as per their file table for all cases of directory support.

Actual Behavior

ZIP Files that do not start with a directory in their file table result in the last file in the file table being copied as a file as specified by the destination directory.

@dchoi-viant
Copy link
Contributor Author

dchoi-viant commented Mar 13, 2025

As per 21faa05, this sets up a situation of a ZIP file with just 2 files, a.txt and b.txt, which is then requested to be unzipped as per expected behavior using afs.(*Service).Copy().

The result is that the specified directory is a file.

Based on initial debugging, this occurs if the first entry in the ZIP is not a directory.

afs/copy.go

Lines 51 to 56 in 21faa05

mappedName := ""
if source.IsDir() || !isInternalWalker {
err = s.Create(ctx, destURL, source.Mode()|os.ModeDir, source.IsDir(), *destOptions...)
} else {
destURL, mappedName = url.Split(destURL, file.Scheme)
}

In the case the resulting source.IsDir() is false, then mappedName is set, which alters behavior, specifically, it replaces the resulting name of the copied file.

afs/copy.go

Lines 84 to 96 in 21faa05

err = walker.Walk(ctx, sourceURL, func(ctx context.Context, baseURL string, parent string, info os.FileInfo, reader io.Reader) (toContinue bool, err error) {
if mappedName != "" {
info = file.NewInfo(mappedName, info.Size(), info.Mode(), info.ModTime(), info.IsDir())
}
if modifier != nil && reader != nil {
info, reader, err = modifier(parent, info, ioutil.NopCloser(reader))
if err != nil {
return false, err
}
}
err = upload(ctx, parent, info, reader)
return err == nil, err
}, *srcOptions...)

@dchoi-viant
Copy link
Contributor Author

For branch https://github.com/viant/afs/tree/issue-24-zip-error
Converted manual run scripts to unit tests without file dependencies.

func TestCopy(t *testing.T) {

@dchoi-viant
Copy link
Contributor Author

dchoi-viant commented Mar 24, 2025

One of the crux issues is that the behavior for List() isn't particularly concrete depending on modifiers and implementation, particularly with the recursive option enabled.

  1. Should non-recursive List() include the directory itself (i.e. .)?
    • There is some behavior in the main Manager that handles this specially, and is why the ZIP behavior is weird.
  2. Should recursive List() include directories as items?
  3. Should ZIP file tables, which are not required to include directory entries, include directory entries?
  4. Should ZIP files, be recursive without the List() recursion enabled, due to how file table prefixes work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants