From e5580974c07fbb14ac8dc5c2b683103b0394cc17 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 30 Aug 2017 17:40:21 -0700 Subject: [PATCH] Improve soong_zip filename collisions Allow filename collisions for directories, which may happen if multiple globs of resources include the same directory names. Continue to report errors if collisions occur between files, and also add checks for collisions between files and directories. Test: manual Change-Id: Iac19fbd325c53fbb41552ea230d813c8dddf9439 --- cmd/soong_zip/soong_zip.go | 133 ++++++++++++++++++++----------------- 1 file changed, 71 insertions(+), 62 deletions(-) diff --git a/cmd/soong_zip/soong_zip.go b/cmd/soong_zip/soong_zip.go index 78a77b831..ae176dff3 100644 --- a/cmd/soong_zip/soong_zip.go +++ b/cmd/soong_zip/soong_zip.go @@ -190,9 +190,10 @@ func usage() { } type zipWriter struct { - time time.Time - createdDirs map[string]bool - directories bool + time time.Time + createdFiles map[string]string + createdDirs map[string]string + directories bool errors chan error writeOps chan chan *zipEntry @@ -254,14 +255,14 @@ func main() { } w := &zipWriter{ - time: time.Date(2009, 1, 1, 0, 0, 0, 0, time.UTC), - createdDirs: make(map[string]bool), - directories: *directories, - compLevel: *compLevel, + time: time.Date(2009, 1, 1, 0, 0, 0, 0, time.UTC), + createdDirs: make(map[string]string), + createdFiles: make(map[string]string), + directories: *directories, + compLevel: *compLevel, } pathMappings := []pathMapping{} - set := make(map[string]string) for _, fa := range fArgs { srcs := fa.sourceFiles @@ -270,7 +271,7 @@ func main() { } for _, src := range srcs { if err := fillPathPairs(fa.pathPrefixInZip, - fa.sourcePrefixToStrip, src, set, &pathMappings); err != nil { + fa.sourcePrefixToStrip, src, &pathMappings); err != nil { log.Fatal(err) } } @@ -283,7 +284,7 @@ func main() { } } -func fillPathPairs(prefix, rel, src string, set map[string]string, pathMappings *[]pathMapping) error { +func fillPathPairs(prefix, rel, src string, pathMappings *[]pathMapping) error { src = strings.TrimSpace(src) if src == "" { return nil @@ -295,14 +296,6 @@ func fillPathPairs(prefix, rel, src string, set map[string]string, pathMappings } dest = filepath.Join(prefix, dest) - if _, found := set[dest]; found { - return fmt.Errorf("found two file paths to be copied into dest path: %q,"+ - " both [%q]%q and [%q]%q!", - dest, dest, src, dest, set[dest]) - } else { - set[dest] = src - } - zipMethod := zip.Deflate if _, found := nonDeflatedFiles[dest]; found { zipMethod = zip.Store @@ -488,14 +481,29 @@ func (z *zipWriter) addFile(dest, src string, method uint16) error { return err } else if s.IsDir() { if z.directories { - return z.writeDirectory(dest) + return z.writeDirectory(dest, src) } return nil - } else if s.Mode()&os.ModeSymlink != 0 { - return z.writeSymlink(dest, src) - } else if !s.Mode().IsRegular() { - return fmt.Errorf("%s is not a file, directory, or symlink", src) } else { + if err := z.writeDirectory(filepath.Dir(dest), src); err != nil { + return err + } + + if prev, exists := z.createdDirs[dest]; exists { + return fmt.Errorf("destination %q is both a directory %q and a file %q", dest, prev, src) + } + if prev, exists := z.createdFiles[dest]; exists { + return fmt.Errorf("destination %q has two files %q and %q", dest, prev, src) + } + + z.createdFiles[dest] = src + + if s.Mode()&os.ModeSymlink != 0 { + return z.writeSymlink(dest, src) + } else if !s.Mode().IsRegular() { + return fmt.Errorf("%s is not a file, directory, or symlink", src) + } + fileSize = s.Size() executable = s.Mode()&0100 != 0 } @@ -518,13 +526,19 @@ func (z *zipWriter) addFile(dest, src string, method uint16) error { return z.writeFileContents(header, r) } -// writes the contents of r according to the specifications in header func (z *zipWriter) addManifest(dest string, src string, method uint16) error { givenBytes, err := ioutil.ReadFile(src) if err != nil { return err } + if prev, exists := z.createdDirs[dest]; exists { + return fmt.Errorf("destination %q is both a directory %q and a file %q", dest, prev, src) + } + if prev, exists := z.createdFiles[dest]; exists { + return fmt.Errorf("destination %q has two files %q and %q", dest, prev, src) + } + manifestMarker := []byte("Manifest-Version:") header := append(manifestMarker, []byte(" 1.0\nCreated-By: soong_zip\n")...) @@ -552,15 +566,6 @@ func (z *zipWriter) writeFileContents(header *zip.FileHeader, r readerSeekerClos header.SetModTime(z.time) - if z.directories { - dest := header.Name - dir, _ := filepath.Split(dest) - err := z.writeDirectory(dir) - if err != nil { - return err - } - } - compressChan := make(chan *zipEntry, 1) z.writeOps <- compressChan @@ -781,53 +786,57 @@ func (z *zipWriter) addExtraField(zipHeader *zip.FileHeader, fieldHeader [2]byte zipHeader.Extra = append(zipHeader.Extra, data...) } -func (z *zipWriter) writeDirectory(dir string) error { +// writeDirectory annotates that dir is a directory created for the src file or directory, and adds +// the directory entry to the zip file if directories are enabled. +func (z *zipWriter) writeDirectory(dir, src string) error { // clean the input - cleanDir := filepath.Clean(dir) + dir = filepath.Clean(dir) // discover any uncreated directories in the path zipDirs := []string{} - for cleanDir != "" && cleanDir != "." && !z.createdDirs[cleanDir] { + for dir != "" && dir != "." { + if _, exists := z.createdDirs[dir]; exists { + break + } - z.createdDirs[cleanDir] = true + if prev, exists := z.createdFiles[dir]; exists { + return fmt.Errorf("destination %q is both a directory %q and a file %q", dir, src, prev) + } + + z.createdDirs[dir] = src // parent directories precede their children - zipDirs = append([]string{cleanDir}, zipDirs...) + zipDirs = append([]string{dir}, zipDirs...) - cleanDir = filepath.Dir(cleanDir) + dir = filepath.Dir(dir) } - // make a directory entry for each uncreated directory - for _, cleanDir := range zipDirs { - dirHeader := &zip.FileHeader{ - Name: cleanDir + "/", - } - dirHeader.SetMode(0700 | os.ModeDir) - dirHeader.SetModTime(z.time) + if z.directories { + // make a directory entry for each uncreated directory + for _, cleanDir := range zipDirs { + dirHeader := &zip.FileHeader{ + Name: cleanDir + "/", + } + dirHeader.SetMode(0700 | os.ModeDir) + dirHeader.SetModTime(z.time) - if *emulateJar && dir == "META-INF/" { - // Jar files have a 0-length extra field with header "CAFE" - z.addExtraField(dirHeader, [2]byte{0xca, 0xfe}, []byte{}) - } + if *emulateJar && dir == "META-INF/" { + // Jar files have a 0-length extra field with header "CAFE" + z.addExtraField(dirHeader, [2]byte{0xca, 0xfe}, []byte{}) + } - ze := make(chan *zipEntry, 1) - ze <- &zipEntry{ - fh: dirHeader, + ze := make(chan *zipEntry, 1) + ze <- &zipEntry{ + fh: dirHeader, + } + close(ze) + z.writeOps <- ze } - close(ze) - z.writeOps <- ze } return nil } func (z *zipWriter) writeSymlink(rel, file string) error { - if z.directories { - dir, _ := filepath.Split(rel) - if err := z.writeDirectory(dir); err != nil { - return err - } - } - fileHeader := &zip.FileHeader{ Name: rel, }