Skip to content

Commit

Permalink
Use the implementation of golang's http.Dir for queue.createAbs()
Browse files Browse the repository at this point in the history
  • Loading branch information
tetsuharuohzeki committed Jan 26, 2017
1 parent 073387a commit 65108ec
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 68 deletions.
8 changes: 8 additions & 0 deletions LICENSE.MIT
Expand Up @@ -20,3 +20,11 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.

------------------------------------------------

Basically, we license our codes as applying the above license.
But some codes are licensed by other licenses.

* The base logic of `queue.createAbs()` is licensed as
[https://github.com/golang/go](https://github.com/golang/go/blob/98842cabb6133ab7b8f2b323754a48085eed82f3/LICENSE)
27 changes: 2 additions & 25 deletions queue/file_repo.go
Expand Up @@ -8,7 +8,6 @@ import (
"os"
"path"
"path/filepath"
"strings"
"sync"
)

Expand Down Expand Up @@ -149,13 +148,8 @@ func exists(filename string) bool {
}

func createQueueJSONPath(root, owner, name string) (ok bool, path string) {
dir, err := createAbs(root, owner)
if err != nil {
log.Printf("error: %v\n", err)
return false, ""
}

file, err := createAbs(dir, name+".json")
reponame := owner + "/" + name + ".json"
file, err := createAbs(root, reponame)
if err != nil {
log.Printf("error: %v\n", err)
return false, ""
Expand All @@ -164,23 +158,6 @@ func createQueueJSONPath(root, owner, name string) (ok bool, path string) {
return true, file
}

func createAbs(root, subpath string) (path string, err error) {
if !validPathFragment(subpath) {
return "", fmt.Errorf("subpath `%v` is not valid input. It may cause directory traversal", subpath)
}

abs, err := filepath.Abs(root + "/" + subpath)
if err != nil {
return "", fmt.Errorf("error: cannot get the path to `%v` + `%v`", root, subpath)
}

if !strings.HasPrefix(abs, root) {
return "", fmt.Errorf("abs `%v` is not under `%v. It may cause directory traversal", abs, root)
}

return abs, nil
}

// Check `p` is insecure string as a path.
// If `p` is `../`, it can access to security path (e.g. `~/.ssh/`).
func validPathFragment(p string) bool {
Expand Down
28 changes: 28 additions & 0 deletions queue/path.go
@@ -0,0 +1,28 @@
package queue

import (
"errors"
"path"
"path/filepath"
"strings"
)

// Based on https://github.com/golang/go/blob/98842cabb6133ab7b8f2b323754a48085eed82f3/src/net/http/fs.go#L26-L50
// This code's license is https://github.com/golang/go/blob/98842cabb6133ab7b8f2b323754a48085eed82f3/LICENSE.
func createAbs(root, name string) (p string, err error) {
if root == "" || name == "" {
return "", errors.New("all arguments must not be empty")
}

if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) ||
strings.Contains(name, "\x00") {
return "", errors.New("invalid character in file path")
}

f := filepath.Join(filepath.FromSlash(path.Clean("/"+root)), filepath.FromSlash(path.Clean("/"+name)))
if f == "/" {
return "", errors.New("the result is `/`")
}

return f, nil
}
109 changes: 66 additions & 43 deletions queue/file_repo_test.go → queue/path_test.go
Expand Up @@ -2,40 +2,6 @@ package queue

import "testing"

func Test_validPathValidCase(t *testing.T) {
list := []string{
"a",
".a",
"..a",
"a.",
"a..",
}

for _, testcase := range list {
if ok := validPathFragment(testcase); !ok {
t.Errorf("%+v should be valid", testcase)
}
}
}

func Test_validPathInValidCase(t *testing.T) {
list := []string{
"~/a",
"/a",
"./a",
"../a",
"a/",
"a/.",
"a/..",
}

for _, testcase := range list {
if ok := validPathFragment(testcase); ok {
t.Errorf("%+v should be invalid", testcase)
}
}
}

func Test_createAbs_ValidCase(t *testing.T) {
type Testcase struct {
root string
Expand All @@ -48,11 +14,56 @@ func Test_createAbs_ValidCase(t *testing.T) {
path: "b",
expected: "/a/b",
},
Testcase{
root: "/a",
path: "/b",
expected: "/a/b",
},
Testcase{
root: "/a",
path: ".b",
expected: "/a/.b",
},
Testcase{
root: "/a",
path: "../~/b",
expected: "/a/~/b",
},
Testcase{
root: "/a",
path: "./b",
expected: "/a/b",
},
Testcase{
root: "/a",
path: "../../b",
expected: "/a/b",
},
Testcase{
root: "/a",
path: "..",
expected: "/a",
},
Testcase{
root: "..",
path: "/a",
expected: "/a",
},
Testcase{
root: "a",
path: "/b",
expected: "/a/b",
},
Testcase{
root: "a",
path: "../b",
expected: "/a/b",
},
Testcase{
root: "a",
path: "../~/b",
expected: "/a/~/b",
},
}

for _, test := range list {
Expand All @@ -78,23 +89,35 @@ func Test_createAbs_InvalidCase(t *testing.T) {
},
Testcase{
root: "/a",
path: "/b",
path: "",
},
Testcase{
root: "/a",
path: "./b",
root: "/",
path: "",
},
Testcase{
root: "/a",
path: "../b",
root: "",
path: "/",
},
Testcase{
root: "/a",
path: "../~/b",
root: "/",
path: "/",
},
Testcase{
root: "/a",
path: "../../b",
root: ".",
path: ".",
},
Testcase{
root: ".",
path: "..",
},
Testcase{
root: "..",
path: ".",
},
Testcase{
root: "..",
path: "..",
},
}

Expand Down

0 comments on commit 65108ec

Please sign in to comment.