From c34d2cea011b236d90babea53fc12a1d033cd0f4 Mon Sep 17 00:00:00 2001 From: Dominic Della Valle Date: Tue, 8 May 2018 12:05:54 -0400 Subject: [PATCH] Simplify / refactor --- extractor.go | 66 +++++++++++++++++++-------------------------- sanitize.go | 8 +++--- sanitize_windows.go | 8 +++--- 3 files changed, 36 insertions(+), 46 deletions(-) diff --git a/extractor.go b/extractor.go index e41d923..fa13d07 100644 --- a/extractor.go +++ b/extractor.go @@ -14,17 +14,12 @@ type Extractor struct { Path string Progress func(int64) int64 - // SanitizePathFunc can be provided if you wish to modify the source path - // The source path is divided by "/" into elements. A legal path base is expected to be returned, it will be joined with the extraction root. - // "extractionRoot/a/b" -> SanitizePathFunc(["a", "b"]) - SanitizePathFunc func(pathComponents []string) (joinedComponents string) - - // SanitizePathCallback is called with the original input path and the sanitized output path from SanitizePathFunc - // This is useful if you wish to log changes between the source and destination, or fail when encountering an incompatible source->dest - // returning an error will abort the extraction - SanitizePathCallback func(input, output string) (userDefined error) + // SanitizePathFunc can be provided if you wish to inspect and/or modify the source path + // returning an error from this function will abort extraction + SanitizePathFunc func(path string) (saferPath string, userDefined error) // LinkFunc can be provided for user specified handling of filesystem links + // returning an error from this function aborts extraction LinkFunc func(Link) error } @@ -73,7 +68,7 @@ func (te *Extractor) Extract(reader io.Reader) error { return err } case tar.TypeSymlink: - if err := te.extractSymlink(header, i); err != nil { + if err := te.extractSymlink(header); err != nil { return err } default: @@ -83,7 +78,7 @@ func (te *Extractor) Extract(reader io.Reader) error { return nil } -// Sanitize toggles output sanitation, using default sanitation functions +// Sanitize toggles output sanitation, using built in sanitation functions // (Modify paths to be platform legal, symlinks may not escape extraction root) func (te *Extractor) Sanitize(toggle bool) { te.SanitizePathFunc = sanitizePath @@ -91,7 +86,7 @@ func (te *Extractor) Sanitize(toggle bool) { if err := childrenOnly(inLink); err != nil { return err } - if err := platformAnalyzeLink(inLink); err != nil { + if err := platformLink(inLink); err != nil { return err } return os.Symlink(inLink.Target, inLink.Name) @@ -99,25 +94,24 @@ func (te *Extractor) Sanitize(toggle bool) { } // outputPath returns the path at which to place tarPath -func (te *Extractor) outputPath(tarPath string) string { - var outPath string - elems := strings.Split(tarPath, "/") // break into elems - elems = elems[1:] // remove original root +func (te *Extractor) outputPath(tarPath string) (outPath string, err error) { + elems := strings.Split(tarPath, "/") // break into elems + elems = elems[1:] // remove original root + outPath = strings.Join(elems, "/") // join elems + outPath = gopath.Join(te.Path, outPath) // rebase on to extraction target root + // sanitize path to be platform legal if te.SanitizePathFunc != nil { - outPath = te.SanitizePathFunc(elems) // sanitize base path elements to be platform legal + outPath, err = te.SanitizePathFunc(outPath) } else { - outPath = fp.Join(elems...) // join elems + outPath = fp.FromSlash(outPath) } - outPath = fp.Join(te.Path, outPath) // rebase on to extraction target root - return outPath + return } func (te *Extractor) extractDir(h *tar.Header, depth int) error { - path := te.outputPath(h.Name) - if te.SanitizePathCallback != nil && depth != 0 { - if err := te.SanitizePathCallback(gopath.Join(te.Path, h.Name), path); err != nil { - return err - } + path, err := te.outputPath(h.Name) + if err != nil { + return err } if depth == 0 { @@ -128,12 +122,10 @@ func (te *Extractor) extractDir(h *tar.Header, depth int) error { return os.MkdirAll(path, 0755) } -func (te *Extractor) extractSymlink(h *tar.Header, depth int) error { - path := te.outputPath(h.Name) - if te.SanitizePathCallback != nil && depth != 0 { - if err := te.SanitizePathCallback(h.Name, path); err != nil { - return err - } +func (te *Extractor) extractSymlink(h *tar.Header) error { + path, err := te.outputPath(h.Name) + if err != nil { + return err } if te.LinkFunc != nil { @@ -144,11 +136,9 @@ func (te *Extractor) extractSymlink(h *tar.Header, depth int) error { } func (te *Extractor) extractFile(h *tar.Header, r *tar.Reader, depth int, rootExists bool, rootIsDir bool) error { - path := te.outputPath(h.Name) - if te.SanitizePathCallback != nil && depth != 0 { - if err := te.SanitizePathCallback(h.Name, path); err != nil { - return err - } + path, err := te.outputPath(h.Name) + if err != nil { + return err } if depth == 0 { // if depth is 0, this is the only file (we aren't extracting a directory) @@ -194,10 +184,10 @@ func copyWithProgress(to io.Writer, from io.Reader, cb func(int64) int64) error } } -// childrenOnly is a link-filter base, called before platform specific filters +// childrenOnly will return an error if link targets escape their root func childrenOnly(inLink Link) error { if fp.IsAbs(inLink.Target) { - return fmt.Errorf("Link target %q is an absolute path (forbidden)", inLink.Target) //TODO: discuss + return fmt.Errorf("Link target %q is an absolute path (forbidden)", inLink.Target) } resolvedTarget := fp.Join(inLink.Name, inLink.Target) diff --git a/sanitize.go b/sanitize.go index 55b1900..bd28631 100644 --- a/sanitize.go +++ b/sanitize.go @@ -4,18 +4,16 @@ package tar import ( "os" - "path/filepath" ) func isNullDevice(path string) bool { return path == os.DevNull } -func sanitizePath(pathElements []string) (string, error) { - return filepath.Join(pathElements...), nil +func sanitizePath(path string) (string, error) { + return path, nil } -//func sanitizeLink(targetRoot, link Link) (Link, error) { -func platformAnalyzeLink(inLink Link) error { +func platformLink(inLink Link) error { return nil } diff --git a/sanitize_windows.go b/sanitize_windows.go index 071d7c3..c8c8fdd 100644 --- a/sanitize_windows.go +++ b/sanitize_windows.go @@ -36,7 +36,9 @@ func isNullDevice(path string) bool { return true } -func sanitizePath(pathElements []string) string { +func sanitizePath(path string) (string, error) { + pathElements := strings.Split(path, "/") + //first pass: strip illegal tail & prefix reserved names `CON .` -> `_CON` for pi := range pathElements { pathElements[pi] = strings.TrimRight(pathElements[pi], ". ") //MSDN: Do not end a file or directory name with a space or a period @@ -69,10 +71,10 @@ func sanitizePath(pathElements []string) string { res = builder.String() } - return filepath.FromSlash(res) + return filepath.FromSlash(res), nil } -func platformAnalyzeLink(inLink Link) error { +func platformLink(inLink Link) error { if strings.HasPrefix(inLink.Target, string(os.PathSeparator)) || strings.HasPrefix(inLink.Target, "/") { return fmt.Errorf("Link target %q is relative to drive root (forbidden)", inLink.Target) //TODO: discuss }