Merge pull request #393 from docker/path-fix

use path instead of filepath to express TUF roles

client/helpers.go

@@ -4,7 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"path/filepath"
+	"path"
 	"time"
 
 	"github.com/Sirupsen/logrus"
@@ -134,7 +134,7 @@ func doWithRoleFallback(role string, doFunc func(string) error) error {
 		if _, ok := err.(data.ErrInvalidRole); !ok {
 			return err
 		}
-		role = filepath.Dir(role)
+		role = path.Dir(role)
 	}
 	return data.ErrInvalidRole{Role: role}
 }

server/handlers/validation.go

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"path/filepath"
+	"path"
 	"sort"
 	"strings"
 
@@ -145,7 +145,7 @@ func loadAndValidateTargets(gun string, repo *tuf.Repo, roles map[string]storage
 
 	updatesToApply := make([]storage.MetaUpdate, 0, len(targetsRoles))
 	for _, role := range targetsRoles {
-		parentRole := filepath.Dir(role)
+		parentRole := path.Dir(role)
 
 		// don't load parent if current role is "targets" or if the parent has
 		// already been loaded

tuf/client/client.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"io"
 	"path"
-	"path/filepath"
 	"strings"
 
 	"github.com/Sirupsen/logrus"
@@ -506,11 +505,12 @@ func (c Client) getTargetsFile(role string, keyIDs []string, snapshotMeta data.F
 	return s, nil
 }
 
-// RoleTargetsPath generates the appropriate filename for the targets file,
+// RoleTargetsPath generates the appropriate HTTP URL for the targets file,
 // based on whether the repo is marked as consistent.
 func (c Client) RoleTargetsPath(role string, hashSha256 string, consistent bool) (string, error) {
 	if consistent {
-		dir := filepath.Dir(role)
+		// Use path instead of filepath since we refer to the TUF role directly instead of its target files
+		dir := path.Dir(role)
 		if strings.Contains(role, "/") {
 			lastSlashIdx := strings.LastIndex(role, "/")
 			role = role[lastSlashIdx+1:]

tuf/data/roles.go

@@ -2,7 +2,7 @@ package data
 
 import (
 	"fmt"
-	"path/filepath"
+	"path"
 	"regexp"
 	"strings"
 )
@@ -124,7 +124,7 @@ func IsDelegation(role string) bool {
 	correctLength := len(role) < 256
 
 	// Removes ., .., extra slashes, and trailing slash
-	isClean := filepath.Clean(role) == role
+	isClean := path.Clean(role) == role
 	return strings.HasPrefix(role, targetsBase) &&
 		whitelistedChars &&
 		correctLength &&

tuf/data/roles_test.go

@@ -1,7 +1,7 @@
 package data
 
 import (
-	"path/filepath"
+	"path"
 	"strings"
 	"testing"
 
@@ -194,46 +194,44 @@ func TestErrInvalidRole(t *testing.T) {
 }
 
 func TestIsDelegation(t *testing.T) {
-	assert.True(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "level1")))
+	assert.True(t, IsDelegation(path.Join(CanonicalTargetsRole, "level1")))
 	assert.True(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole, "level1", "level2", "level3")))
-	assert.True(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "under_score")))
-	assert.True(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "hyphen-hyphen")))
+		path.Join(CanonicalTargetsRole, "level1", "level2", "level3")))
+	assert.True(t, IsDelegation(path.Join(CanonicalTargetsRole, "under_score")))
+	assert.True(t, IsDelegation(path.Join(CanonicalTargetsRole, "hyphen-hyphen")))
 	assert.False(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole, strings.Repeat("x", 255-len(CanonicalTargetsRole)))))
+		path.Join(CanonicalTargetsRole, strings.Repeat("x", 255-len(CanonicalTargetsRole)))))
 
 	assert.False(t, IsDelegation(""))
 	assert.False(t, IsDelegation(CanonicalRootRole))
-	assert.False(t, IsDelegation(filepath.Join(CanonicalRootRole, "level1")))
+	assert.False(t, IsDelegation(path.Join(CanonicalRootRole, "level1")))
 
 	assert.False(t, IsDelegation(CanonicalTargetsRole))
 	assert.False(t, IsDelegation(CanonicalTargetsRole+"/"))
-	assert.False(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "level1")+"/"))
-	assert.False(t, IsDelegation(filepath.Join(CanonicalTargetsRole, "UpperCase")))
+	assert.False(t, IsDelegation(path.Join(CanonicalTargetsRole, "level1")+"/"))
+	assert.False(t, IsDelegation(path.Join(CanonicalTargetsRole, "UpperCase")))
 
 	assert.False(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole, "directory")+"/../../traversal"))
+		path.Join(CanonicalTargetsRole, "directory")+"/../../traversal"))
+
+	assert.False(t, IsDelegation(CanonicalTargetsRole+"///test/middle/slashes"))
+
+	assert.False(t, IsDelegation(CanonicalTargetsRole+"/./././"))
 
 	assert.False(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole)+"///test/middle/slashes"))
+		path.Join("  ", CanonicalTargetsRole, "level1")))
 
 	assert.False(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole)+"/./././"))
+		path.Join("  "+CanonicalTargetsRole, "level1")))
 
 	assert.False(t, IsDelegation(
-		filepath.Join("  ", CanonicalTargetsRole, "level1")))
+		path.Join(CanonicalTargetsRole, "level1"+"  ")))
 
 	assert.False(t, IsDelegation(
-		filepath.Join("  "+CanonicalTargetsRole, "level1")))
+		path.Join(CanonicalTargetsRole, "white   space"+"level2")))
 
 	assert.False(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole, "level1"+"  ")))
-
-	assert.False(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole, "white   space"+"level2")))
-
-	assert.False(t, IsDelegation(
-		filepath.Join(CanonicalTargetsRole, strings.Repeat("x", 256-len(CanonicalTargetsRole)))))
+		path.Join(CanonicalTargetsRole, strings.Repeat("x", 256-len(CanonicalTargetsRole)))))
 }
 
 func TestValidRoleFunction(t *testing.T) {
@@ -241,9 +239,9 @@ func TestValidRoleFunction(t *testing.T) {
 	assert.True(t, ValidRole(CanonicalTimestampRole))
 	assert.True(t, ValidRole(CanonicalSnapshotRole))
 	assert.True(t, ValidRole(CanonicalTargetsRole))
-	assert.True(t, ValidRole(filepath.Join(CanonicalTargetsRole, "level1")))
+	assert.True(t, ValidRole(path.Join(CanonicalTargetsRole, "level1")))
 	assert.True(t, ValidRole(
-		filepath.Join(CanonicalTargetsRole, "level1", "level2", "level3")))
+		path.Join(CanonicalTargetsRole, "level1", "level2", "level3")))
 
 	assert.False(t, ValidRole(""))
 	assert.False(t, ValidRole(CanonicalRootRole+"/"))
@@ -251,7 +249,7 @@ func TestValidRoleFunction(t *testing.T) {
 	assert.False(t, ValidRole(CanonicalSnapshotRole+"/"))
 	assert.False(t, ValidRole(CanonicalTargetsRole+"/"))
 
-	assert.False(t, ValidRole(filepath.Join(CanonicalRootRole, "level1")))
+	assert.False(t, ValidRole(path.Join(CanonicalRootRole, "level1")))
 
-	assert.False(t, ValidRole(filepath.Join("role")))
+	assert.False(t, ValidRole(path.Join("role")))
 }

tuf/tuf.go

@@ -7,7 +7,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
-	"path/filepath"
+	"path"
 	"strings"
 	"time"
 
@@ -181,7 +181,7 @@ func (tr *Repo) GetDelegation(role string) (*data.Role, error) {
 		return nil, data.ErrInvalidRole{Role: role, Reason: "not a valid delegated role"}
 	}
 
-	parent := filepath.Dir(role)
+	parent := path.Dir(role)
 
 	// check the parent role
 	if parentRole := tr.keysDB.GetRole(parent); parentRole == nil {
@@ -209,7 +209,7 @@ func (tr *Repo) UpdateDelegations(role *data.Role, keys []data.PublicKey) error
 	if !role.IsDelegation() || !role.IsValid() {
 		return data.ErrInvalidRole{Role: role.Name, Reason: "not a valid delegated role"}
 	}
-	parent := filepath.Dir(role.Name)
+	parent := path.Dir(role.Name)
 
 	if err := tr.VerifyCanSign(parent); err != nil {
 		return err
@@ -270,7 +270,7 @@ func (tr *Repo) DeleteDelegation(role data.Role) error {
 	// the role variable must not be used past this assignment for safety
 	name := role.Name
 
-	parent := filepath.Dir(name)
+	parent := path.Dir(name)
 	if err := tr.VerifyCanSign(parent); err != nil {
 		return err
 	}