Merge pull request #845 from jonboulle/845

Prevent users from using "{}" in template names

api/units.go

@@ -71,7 +71,7 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str
 		sendError(rw, http.StatusBadRequest, fmt.Errorf("name in URL %q differs from unit name in request body %q", item, su.Name))
 		return
 	}
-	if err := validateName(su.Name); err != nil {
+	if err := ValidateName(su.Name); err != nil {
 		sendError(rw, http.StatusBadRequest, err)
 		return
 	}
@@ -130,10 +130,10 @@ var validUnitTypes = pkg.NewUnsafeSet(
 	"scope",
 )
 
-// validateName ensures that a given unit name is valid; if not, an error is
+// ValidateName ensures that a given unit name is valid; if not, an error is
 // returned describing the first issue encountered.
 // systemd reference: `unit_name_is_valid` in `unit-name.c`
-func validateName(name string) error {
+func ValidateName(name string) error {
 	length := len(name)
 	if length == 0 {
 		return errors.New("unit name cannot be empty")

api/units_test.go

@@ -657,7 +657,7 @@ func TestValidateName(t *testing.T) {
 		"@this.mount",
 	}
 	for _, name := range badTestCases {
-		if err := validateName(name); err == nil {
+		if err := ValidateName(name); err == nil {
 			t.Errorf("name %q: validation did not fail as expected!", name)
 		}
 	}
@@ -676,7 +676,7 @@ func TestValidateName(t *testing.T) {
 		fmt.Sprintf("%0"+strconv.Itoa(unitNameMax)+"s", ".service"),
 	}
 	for _, name := range goodTestCases {
-		if err := validateName(name); err != nil {
+		if err := ValidateName(name); err != nil {
 			t.Errorf("name %q: validation failed unexpectedly! err=%v", name, err)
 		}
 	}

fleetctl/fleetctl.go

@@ -424,6 +424,9 @@ func createUnit(name string, uf *unit.UnitFile) (*schema.Unit, error) {
 	// redundant with the check in api.unitsResource.set, but it is a
 	// workaround to implementing the same check in the RegistryClient. It
 	// will disappear once RegistryClient is deprecated.
+	if err := api.ValidateName(name); err != nil {
+		return nil, err
+	}
 	if err := api.ValidateOptions(u.Options); err != nil {
 		return nil, err
 	}

fleetctl/fleetctl_test.go

@@ -119,33 +119,72 @@ func TestCreateUnitFails(t *testing.T) {
 	}
 	cAPI = fakeAPI{}
 	var i int
+	var un string
 	var uf *unit.UnitFile
 	defer func() {
 		if r := recover(); r != nil {
 			t.Errorf("case %d: unexpectedly called API!", i)
+			t.Logf("unit name: %q", un)
 			t.Logf("unit file: %#v", uf)
 		}
 	}()
-	for i, uf = range []*unit.UnitFile{
-		nil,
-		newUnitFile(t, `[X-Fleet]
+	testCases := []struct {
+		name string
+		uf   *unit.UnitFile
+	}{
+		{
+			"foo@{1,3}.service",
+			newUnitFile(t, ``),
+		},
+		{
+			"foo@{1..3}.service",
+			newUnitFile(t, ``),
+		},
+		{
+			"foo.{1-3}.service",
+			newUnitFile(t, ``),
+		},
+		{
+			"foo.service",
+			nil,
+		},
+		{
+			"foo.service",
+			newUnitFile(t, `[X-Fleet]
+	MachineOf=abcd
+	Conflicts=abcd`),
+		},
+		{
+			"foo.service",
+			newUnitFile(t, `[X-Fleet]
 MachineOf=abcd
 Conflicts=abcd`),
-		newUnitFile(t, `[X-Fleet]
-MachineOf=abcd
-Conflicts=abcd`),
-		newUnitFile(t, `[X-Fleet]
+		},
+		{
+			"foo.service",
+			newUnitFile(t, `[X-Fleet]
 Global=true
 MachineOf=abcd`),
-		newUnitFile(t, `[X-Fleet]
+		},
+		{
+			"foo.service",
+			newUnitFile(t, `[X-Fleet]
 Global=true
 MachineOf=zxcvq`),
-		newUnitFile(t, `[X-Fleet]
+		},
+		{
+			"foo.service",
+			newUnitFile(t, `[X-Fleet]
 Global=true
 Conflicts=bar`),
-	} {
-		if _, err := createUnit("foo.service", uf); err == nil {
+		},
+	}
+	for i, tt := range testCases {
+		un = tt.name
+		uf = tt.uf
+		if _, err := createUnit(un, uf); err == nil {
 			t.Errorf("case %d did not return error as expected!", i)
+			t.Logf("unit name: %v", un)
 			t.Logf("unit file: %#v", uf)
 		}
 	}