From 773763fb87c40176e9cb54cf599587fbc3964ee6 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 20 Jul 2016 21:18:27 -0400 Subject: [PATCH] vis: refactored code to reflect using vis/unvis for file names Added some more test cases for `vis`ing and `unvis`ing strings, and a test case that walks/checks a directory with filenames that require encoding. Had to change Path() to account for possible errors Unvis() could return. Refactored Vis()/Unvis() into go-mtree tar functionality as well. Signed-off-by: Stephen Chung --- check.go | 30 +++++++++++++++++++++--------- check_test.go | 35 +++++++++++++++++++++++++++++++++++ cmd/gomtree/main.go | 16 ++++++++++++++-- entry.go | 21 ++++++++++++++++----- hierarchy.go | 3 ++- tar.go | 23 ++++++++++++++++++++--- tar_test.go | 12 ++++++++++-- testdata/test.tar | Bin 20480 -> 20480 bytes unvis.go | 23 +++++++++++++++-------- vis.go | 26 +++++++++++++++++++------- vis_test.go | 16 +++++++++++++++- walk.go | 13 ++++++++++--- 12 files changed, 177 insertions(+), 41 deletions(-) diff --git a/check.go b/check.go index a565251..edc730a 100644 --- a/check.go +++ b/check.go @@ -53,8 +53,11 @@ func Check(root string, dh *DirectoryHierarchy, keywords []string) (*Result, err creator.curSet = nil } case RelativeType, FullType: - filename := e.Path() - info, err := os.Lstat(filename) + pathname, err := e.Path() + if err != nil { + return nil, err + } + info, err := os.Lstat(pathname) if err != nil { return nil, err } @@ -77,23 +80,23 @@ func Check(root string, dh *DirectoryHierarchy, keywords []string) (*Result, err keywordFunc, ok := KeywordFuncs[kw] if !ok { - return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), e.Path()) + return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), pathname) } if keywords != nil && !inSlice(kv.Keyword(), keywords) { continue } - fh, err := os.Open(filename) + fh, err := os.Open(pathname) if err != nil { return nil, err } - curKeyVal, err := keywordFunc(filename, info, fh) + curKeyVal, err := keywordFunc(pathname, info, fh) if err != nil { fh.Close() return nil, err } fh.Close() if string(kv) != curKeyVal { - failure := Failure{Path: e.Path(), Keyword: kv.Keyword(), Expected: kv.Value(), Got: KeyVal(curKeyVal).Value()} + failure := Failure{Path: pathname, Keyword: kv.Keyword(), Expected: kv.Value(), Got: KeyVal(curKeyVal).Value()} result.Failures = append(result.Failures, failure) } } @@ -133,8 +136,12 @@ func TarCheck(tarDH, dh *DirectoryHierarchy, keywords []string) (*Result, error) creator.curSet = nil } case RelativeType, FullType: + pathname, err := e.Path() + if err != nil { + return nil, err + } if outOfTree { - return &result, fmt.Errorf("No parent node from %s", e.Path()) + return &result, fmt.Errorf("No parent node from %s", pathname) } // TODO: handle the case where "." is not the first Entry to be found tarEntry := curDir.Descend(e.Name) @@ -165,15 +172,20 @@ func TarCheck(tarDH, dh *DirectoryHierarchy, keywords []string) (*Result, error) } for _, kv := range kvs { + if _, ok := KeywordFuncs[kv.Keyword()]; !ok { - return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), e.Path()) + return nil, fmt.Errorf("Unknown keyword %q for file %q", kv.Keyword(), pathname) } if keywords != nil && !inSlice(kv.Keyword(), keywords) { continue } + tarpath, err := tarEntry.Path() + if err != nil { + return nil, err + } if tarkv := tarkvs.Has(kv.Keyword()); tarkv != emptyKV { if string(tarkv) != string(kv) { - failure := Failure{Path: tarEntry.Path(), Keyword: kv.Keyword(), Expected: kv.Value(), Got: tarkv.Value()} + failure := Failure{Path: tarpath, Keyword: kv.Keyword(), Expected: kv.Value(), Got: tarkv.Value()} result.Failures = append(result.Failures, failure) } } diff --git a/check_test.go b/check_test.go index fa0368a..035c2b3 100644 --- a/check_test.go +++ b/check_test.go @@ -260,3 +260,38 @@ func TestIgnoreComments(t *testing.T) { t.Fatal(res.Failures) } } + +func TestCheckNeedsEncoding(t *testing.T) { + dir, err := ioutil.TempDir("", "test-needs-encoding") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + fh, err := os.Create(filepath.Join(dir, "file[ ")) + if err != nil { + t.Fatal(err) + } + if err := fh.Close(); err != nil { + t.Error(err) + } + fh, err = os.Create(filepath.Join(dir, " , should work")) + if err != nil { + t.Fatal(err) + } + if err := fh.Close(); err != nil { + t.Error(err) + } + + dh, err := Walk(dir, nil, DefaultKeywords) + if err != nil { + t.Fatal(err) + } + res, err := Check(dir, dh, nil) + if err != nil { + t.Fatal(err) + } + if len(res.Failures) > 0 { + t.Fatal(res.Failures) + } +} diff --git a/cmd/gomtree/main.go b/cmd/gomtree/main.go index c7f90d8..705c41c 100644 --- a/cmd/gomtree/main.go +++ b/cmd/gomtree/main.go @@ -202,13 +202,25 @@ func main() { if len(res.Extra) > 0 { defer os.Exit(1) for _, extra := range res.Extra { - fmt.Printf("%s extra\n", extra.Path()) + extrapath, err := extra.Path() + if err != nil { + log.Println(err) + isErr = true + return + } + fmt.Printf("%s extra\n", extrapath) } } if len(res.Missing) > 0 { defer os.Exit(1) for _, missing := range res.Missing { - fmt.Printf("%s missing\n", missing.Path()) + missingpath, err := missing.Path() + if err != nil { + log.Println(err) + isErr = true + return + } + fmt.Printf("%s missing\n", missingpath) } } } else { diff --git a/entry.go b/entry.go index 8fe5027..c6f5bec 100644 --- a/entry.go +++ b/entry.go @@ -48,14 +48,25 @@ func (e Entry) Ascend() *Entry { return e.Parent } -// Path provides the full path of the file, despite RelativeType or FullType -func (e Entry) Path() string { - if e.Parent == nil || e.Type == FullType { - return filepath.Clean(e.Name) +// Path provides the full path of the file, despite RelativeType or FullType. It +// will be in Unvis'd form. +func (e Entry) Path() (string, error) { + decodedName, err := Unvis(e.Name) + if err != nil { + return "", err } - return filepath.Clean(filepath.Join(e.Parent.Path(), e.Name)) + if e.Parent == nil || e.Type == FullType { + return filepath.Clean(decodedName), nil + } + parentName, err := e.Parent.Path() + if err != nil { + return "", err + } + return filepath.Clean(filepath.Join(parentName, decodedName)), nil } +// String joins a file with its associated keywords. The file name will be the +// Vis'd encoded version so that it can be parsed appropriately when Check'd. func (e Entry) String() string { if e.Raw != "" { return e.Raw diff --git a/hierarchy.go b/hierarchy.go index 9f66056..28d7fdc 100644 --- a/hierarchy.go +++ b/hierarchy.go @@ -16,7 +16,8 @@ func (dh DirectoryHierarchy) WriteTo(w io.Writer) (n int64, err error) { sort.Sort(byPos(dh.Entries)) var sum int64 for _, e := range dh.Entries { - i, err := io.WriteString(w, e.String()+"\n") + str := e.String() + i, err := io.WriteString(w, str+"\n") if err != nil { return sum, err } diff --git a/tar.go b/tar.go index b221e24..9455853 100644 --- a/tar.go +++ b/tar.go @@ -97,8 +97,15 @@ func (ts *tarStream) readHeaders() { defer os.Remove(tmpFile.Name()) // Alright, it's either file or directory + encodedName, err := Vis(filepath.Base(hdr.Name)) + if err != nil { + tmpFile.Close() + os.Remove(tmpFile.Name()) + ts.pipeReader.CloseWithError(err) + return + } e := Entry{ - Name: filepath.Base(hdr.Name), + Name: encodedName, Type: RelativeType, } @@ -213,8 +220,13 @@ func populateTree(root, e *Entry, hdr *tar.Header, ts *tarStream) { if isDir { newEntry = e } else { + encodedName, err := Vis(name) + if err != nil { + ts.setErr(err) + return + } newEntry = &Entry{ - Name: name, + Name: encodedName, Type: RelativeType, } } @@ -230,8 +242,13 @@ func populateTree(root, e *Entry, hdr *tar.Header, ts *tarStream) { parent.Children = append([]*Entry{e}, parent.Children...) e.Parent = parent } else { + commentpath, err := e.Path() + if err != nil { + ts.setErr(err) + return + } commentEntry := Entry{ - Raw: "# " + e.Path(), + Raw: "# " + commentpath, Type: CommentType, } e.Prev = &commentEntry diff --git a/tar_test.go b/tar_test.go index 38b8dd5..9f5ce87 100644 --- a/tar_test.go +++ b/tar_test.go @@ -119,12 +119,20 @@ func TestTar(t *testing.T) { errors += "Keyword validation errors\n" case len(res.Missing) > 0: for _, m := range res.Missing { - t.Errorf("Missing file: %s\n", m.Path()) + missingpath, err := m.Path() + if err != nil { + t.Fatal(err) + } + t.Errorf("Missing file: %s\n", missingpath) } errors += "Missing files not expected for this test\n" case len(res.Extra) > 0: for _, e := range res.Extra { - t.Errorf("Extra file: %s\n", e.Path()) + extrapath, err := e.Path() + if err != nil { + t.Fatal(err) + } + t.Errorf("Extra file: %s\n", extrapath) } errors += "Extra files not expected for this test\n" } diff --git a/testdata/test.tar b/testdata/test.tar index 6ae9b223f25c2896e8565b3112f69361460b760c..da66be3a712a824fbcb85d15126018affae32191 100644 GIT binary patch delta 215 zcmYL@O$vfg6h<`-nuU#;1h+xatoN(5h*r=*!u!kzqT&;pIWaYB!viHV81nTZ*LfuW&^ks*VE>10FZl+BEc-OOMqb7KRbl(B)4Ib4dB mMRqf*N;{LFiIK5^Ay5KH7?~Kr<<(jEH?vy&7GLaO$pQe?aTjI) diff --git a/unvis.go b/unvis.go index 74d5ad1..70b8342 100644 --- a/unvis.go +++ b/unvis.go @@ -1,15 +1,22 @@ package mtree // #include "vis.h" +// #include import "C" -import "fmt" +import ( + "fmt" + "unsafe" +) -func Unvis(str string) (string, error) { - dst := new(C.char) - ret := C.strunvis(dst, C.CString(str)) - if ret == 0 { - return "", fmt.Errorf("failed to encode string") +// Unvis is a wrapper for the C implementation of unvis, which decodes a string +// that potentially has characters that are encoded with Vis +func Unvis(src string) (string, error) { + cDst, cSrc := C.CString(string(make([]byte, len(src)+1))), C.CString(src) + defer C.free(unsafe.Pointer(cDst)) + defer C.free(unsafe.Pointer(cSrc)) + ret := C.strunvis(cDst, cSrc) + if ret == -1 { + return "", fmt.Errorf("failed to decode: %q", src) } - - return C.GoString(dst), nil + return C.GoString(cDst), nil } diff --git a/vis.go b/vis.go index 9da8545..75b9d79 100644 --- a/vis.go +++ b/vis.go @@ -1,14 +1,26 @@ package mtree // #include "vis.h" +// #include import "C" -import "fmt" +import ( + "fmt" + "math" + "unsafe" +) -func Vis(str string) (string, error) { - dst := new(C.char) - ret := C.strvis(dst, C.CString(str), C.VIS_WHITE|C.VIS_OCTAL|C.VIS_GLOB) - if ret == 0 { - return "", fmt.Errorf("failed to encode string") +// Vis is a wrapper of the C implementation of the function vis, which encodes +// a character with a particular format/style +func Vis(src string) (string, error) { + // dst needs to be 4 times the length of str, must check appropriate size + if uint32(len(src)*4+1) >= math.MaxUint32/4 { + return "", fmt.Errorf("failed to encode: %q", src) } - return C.GoString(dst), nil + dst := string(make([]byte, 4*len(src)+1)) + cDst, cSrc := C.CString(dst), C.CString(src) + defer C.free(unsafe.Pointer(cDst)) + defer C.free(unsafe.Pointer(cSrc)) + C.strvis(cDst, cSrc, C.VIS_WHITE|C.VIS_OCTAL|C.VIS_GLOB) + + return C.GoString(cDst), nil } diff --git a/vis_test.go b/vis_test.go index bdcd1b3..eb78f33 100644 --- a/vis_test.go +++ b/vis_test.go @@ -9,13 +9,17 @@ func TestVis(t *testing.T) { {"[", "\\133"}, {" ", "\\040"}, {" ", "\\011"}, + {"dir with space", "dir\\040with\\040space"}, + {"consec spaces", "consec\\040\\040\\040spaces"}, + {"trailingsymbol[", "trailingsymbol\\133"}, + {" [ leadingsymbols", "\\040\\133\\040leadingsymbols"}, + {"no_need_for_encoding", "no_need_for_encoding"}, } for i := range testset { got, err := Vis(testset[i].Src) if err != nil { t.Errorf("working with %q: %s", testset[i].Src, err) - continue } if got != testset[i].Dest { t.Errorf("expected %#v; got %#v", testset[i].Dest, got) @@ -33,3 +37,13 @@ func TestVis(t *testing.T) { } } } + +// The resulting string of Vis output could potentially be four times longer than +// the original. Vis must handle this possibility. +func TestVisLength(t *testing.T) { + testString := "All work and no play makes Jack a dull boy\n" + for i := 0; i < 20; i++ { + Vis(testString) + testString = testString + testString + } +} diff --git a/walk.go b/walk.go index ca1a706..8e0763f 100644 --- a/walk.go +++ b/walk.go @@ -47,9 +47,13 @@ func Walk(root string, exlcudes []ExcludeFunc, keywords []string) (*DirectoryHie // Insert a comment of the full path of the directory's name if creator.curDir != nil { + dirname, err := creator.curDir.Path() + if err != nil { + return err + } creator.DH.Entries = append(creator.DH.Entries, Entry{ Pos: len(creator.DH.Entries), - Raw: "# " + filepath.Join(creator.curDir.Path(), entryPathName), + Raw: "# " + filepath.Join(dirname, entryPathName), Type: CommentType, }) } else { @@ -147,9 +151,12 @@ func Walk(root string, exlcudes []ExcludeFunc, keywords []string) (*DirectoryHie } } } - + encodedEntryName, err := Vis(entryPathName) + if err != nil { + return err + } e := Entry{ - Name: entryPathName, + Name: encodedEntryName, Pos: len(creator.DH.Entries), Type: RelativeType, Set: creator.curSet,