From 3c46445a1423c5a8514a09a2223082df596a7540 Mon Sep 17 00:00:00 2001
From: Jonathan Boulle <jonathanboulle@gmail.com>
Date: Fri, 28 Nov 2014 11:28:02 -0800
Subject: [PATCH] actool/schema: ensure ACNames are not empty

---
 app-container/actool/build.go             | 28 +++++++++++++++--------
 app-container/schema/fileset.go           | 11 ++++++---
 app-container/schema/types/acname.go      |  3 +++
 app-container/schema/types/acname_test.go |  1 +
 4 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/app-container/actool/build.go b/app-container/actool/build.go
index dda8ae2..f23ae50 100644
--- a/app-container/actool/build.go
+++ b/app-container/actool/build.go
@@ -15,9 +15,9 @@ var (
 	buildOverwrite bool
 	cmdBuild       = &Command{
 		Name:        "build",
-		Description: "Build a fileset from the target directory",
-		Summary:     "Build a fileset from the target directory",
-		Usage:       "DIRECTORY",
+		Description: "Build a fileset ACI from the target directory",
+		Summary:     "Build a fileset ACI from the target directory",
+		Usage:       "[--overwrite] --name=NAME DIRECTORY OUTPUT_FILE",
 		Run:         runBuild,
 	}
 )
@@ -73,7 +73,11 @@ func buildWalker(root string, aw *fileset.ArchiveWriter) filepath.WalkFunc {
 
 func runBuild(args []string) (exit int) {
 	if len(args) != 2 {
-		stderr("build: Must provide directory and target fileset")
+		stderr("build: Must provide directory and output file")
+		return 1
+	}
+	if buildName == "" {
+		stderr("build: FileSet name cannot be empty")
 		return 1
 	}
 
@@ -81,12 +85,10 @@ func runBuild(args []string) (exit int) {
 	tgt := args[1]
 	ext := filepath.Ext(tgt)
 	if ext != schema.ACIExtension {
-		stderr("fileset: Extension must be %s (given %s)", schema.ACIExtension, ext)
+		stderr("build: Extension must be %s (given %s)", schema.ACIExtension, ext)
 		return 1
 	}
 
-	fsm := schema.NewFileSetManifest(buildName)
-
 	mode := os.O_CREATE | os.O_WRONLY
 	if !buildOverwrite {
 		mode |= os.O_EXCL
@@ -94,19 +96,25 @@ func runBuild(args []string) (exit int) {
 	afs, err := os.OpenFile(tgt, mode, 0655)
 	if err != nil {
 		if os.IsExist(err) {
-			stderr("Target file exists (try --overwrite)")
+			stderr("build: Target file exists (try --overwrite)")
 		} else {
-			stderr("fileset: Unable to open target %s: %v", tgt, err)
+			stderr("build: Unable to open target %s: %v", tgt, err)
 		}
 		return 1
 	}
+
+	fsm, err := schema.NewFileSetManifest(buildName)
+	if err != nil {
+		stderr("build: Unable to create FileSet Manifest: %v", err)
+		return 1
+	}
 	w := tar.NewWriter(afs)
 	aw := fileset.NewArchiveWriter(*fsm, w)
 	filepath.Walk(root, buildWalker(root, aw))
 
 	err = aw.Close()
 	if err != nil {
-		stderr("fileset: Unable to close fileset %s: %v", tgt, err)
+		stderr("build: Unable to close FileSet image %s: %v", tgt, err)
 		return 1
 	}
 
diff --git a/app-container/schema/fileset.go b/app-container/schema/fileset.go
index 0c840d8..884926d 100644
--- a/app-container/schema/fileset.go
+++ b/app-container/schema/fileset.go
@@ -23,14 +23,19 @@ type Dependency struct {
 	Root string       `json:"root"`
 }
 
-func NewFileSetManifest(name string) *FileSetManifest {
-	return &FileSetManifest{
+func NewFileSetManifest(name string) (*FileSetManifest, error) {
+	n, err := types.NewACName(name)
+	if err != nil {
+		return nil, err
+	}
+	fsm := FileSetManifest{
 		ACVersion: AppContainerVersion,
 		ACKind:    "FileSetManifest",
 		OS:        "linux",
 		Arch:      "amd64",
-		Name:      types.ACName(name),
+		Name:      *n,
 	}
+	return &fsm, nil
 }
 
 type fileSetManifest FileSetManifest
diff --git a/app-container/schema/types/acname.go b/app-container/schema/types/acname.go
index 8469c3d..32681f5 100644
--- a/app-container/schema/types/acname.go
+++ b/app-container/schema/types/acname.go
@@ -30,6 +30,9 @@ func (l ACName) Equals(o ACName) bool {
 // NewACName generates a new ACName from a string. If the given string is
 // not a valid ACName, nil and an error are returned.
 func NewACName(s string) (*ACName, error) {
+	if len(s) == 0 {
+		return nil, fmt.Errorf("ACName cannot be empty")
+	}
 	for _, c := range s {
 		if !strings.ContainsRune(valchars, c) {
 			msg := fmt.Sprintf("invalid char in ACName: %c", c)
diff --git a/app-container/schema/types/acname_test.go b/app-container/schema/types/acname_test.go
index 6f2e716..a702f04 100644
--- a/app-container/schema/types/acname_test.go
+++ b/app-container/schema/types/acname_test.go
@@ -24,6 +24,7 @@ func TestNewACName(t *testing.T) {
 
 func TestNewACNameBad(t *testing.T) {
 	tests := []string{
+		"",
 		"foo#",
 		"EXAMPLE.com",
 		"foo.com/BAR",