Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Commit

Permalink
Merge pull request #867 from jonboulle/validate_unit_names
Browse files Browse the repository at this point in the history 
API should validate unit names
  • Loading branch information
@jonboulle
jonboulle committed Sep 9, 2014
2 parents 9fb8c2b + a64c9ca commit d0f98c6
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 45 deletions.
54 changes: 52 additions & 2 deletions api/units.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"path"
"strings"

"github.com/coreos/fleet/client"
"github.com/coreos/fleet/job"
Expand Down Expand Up @@ -103,12 +104,61 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str
ur.update(rw, su.Name, su.DesiredState)
}

const (
// These constants taken from systemd
unitNameMax = 256
digits = "0123456789"
lowercase = "abcdefghijklmnopqrstuvwxyz"
uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
alphanumerical = digits + lowercase + uppercase
validChars = alphanumerical + `:=+.\@`
)

var validUnitTypes = pkg.NewUnsafeSet(
"service",
"socket",
"busname",
"target",
"snapshot",
"device",
"mount",
"automount",
"swap",
"timer",
"path",
"slice",
"scope",
)

// validateName ensures that a given unit name is valid; if not, an error is
// returned detailing the issue encountered.
// returned describing the first issue encountered.
// systemd reference: `unit_name_is_valid` in `unit-name.c`
func validateName(name string) error {
if len(name) == 0 {
length := len(name)
if length == 0 {
return errors.New("unit name cannot be empty")
}
if length > unitNameMax {
return fmt.Errorf("unit name exceeds maximum length (%d)", unitNameMax)
}
dot := strings.Index(name, ".")
if dot == -1 {
return errors.New(`unit name must contain "."`)
}
if dot == length-1 {
return errors.New(`unit name cannot end in "."`)
}
if suffix := name[dot+1:]; !validUnitTypes.Contains(suffix) {
return fmt.Errorf("invalid unit type: %q", suffix)
}
for _, char := range name[:dot] {
if !strings.ContainsRune(validChars, char) {
return fmt.Errorf("invalid character %q in unit name", char)
}
}
if strings.HasPrefix(name, "@") {
return errors.New(`unit name cannot start in "@"`)
}
return nil
}

Expand Down
128 changes: 85 additions & 43 deletions api/units_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ func TestUnitsSubResourceNotFound(t *testing.T) {
func TestUnitsList(t *testing.T) {
fr := registry.NewFakeRegistry()
fr.SetJobs([]job.Job{
{Name: "XXX"},
{Name: "YYY"},
{Name: "XXX.service"},
{Name: "YYY.service"},
})
fAPI := &client.RegistryClient{fr}
resource := &unitsResource{fAPI, "/units"}
Expand Down Expand Up @@ -80,7 +80,7 @@ func TestUnitsList(t *testing.T) {
t.Fatalf("Received unparseable body: %v", err)
}

if len(page.Units) != 2 || page.Units[0].Name != "XXX" || page.Units[1].Name != "YYY" {
if len(page.Units) != 2 || page.Units[0].Name != "XXX.service" || page.Units[1].Name != "YYY.service" {
t.Errorf("Received incorrect UnitPage entity: %v", page)
}
}
Expand Down Expand Up @@ -160,14 +160,14 @@ func TestUnitGet(t *testing.T) {
item string
code int
}{
{item: "XXX", code: http.StatusOK},
{item: "XXX.service", code: http.StatusOK},
{item: "ZZZ", code: http.StatusNotFound},
}

fr := registry.NewFakeRegistry()
fr.SetJobs([]job.Job{
{Name: "XXX"},
{Name: "YYY"},
{Name: "XXX.service"},
{Name: "YYY.service"},
})
fAPI := &client.RegistryClient{fr}
resource := &unitsResource{fAPI, "/units"}
Expand Down Expand Up @@ -208,17 +208,17 @@ func TestUnitsDestroy(t *testing.T) {
}{
// Deletion of an existing unit should succeed
{
init: []job.Job{job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}},
arg: "XXX",
init: []job.Job{job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}},
arg: "XXX.service",
code: http.StatusNoContent,
remaining: []string{},
},
// Deletion of a nonexistent unit should fail
{
init: []job.Job{job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}},
arg: "YYY",
init: []job.Job{job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}},
arg: "YYY.service",
code: http.StatusNotFound,
remaining: []string{"XXX"},
remaining: []string{"XXX.service"},
},
}

Expand Down Expand Up @@ -281,35 +281,35 @@ func TestUnitsSetDesiredState(t *testing.T) {
}{
// Modify the desired State of an existing Job
{
initJobs: []job.Job{job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")}},
initStates: map[string]job.JobState{"XXX": "inactive"},
item: "XXX",
arg: schema.Unit{Name: "XXX", DesiredState: "launched"},
initJobs: []job.Job{job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")}},
initStates: map[string]job.JobState{"XXX.service": "inactive"},
item: "XXX.service",
arg: schema.Unit{Name: "XXX.service", DesiredState: "launched"},
code: http.StatusNoContent,
finalStates: map[string]job.JobState{"XXX": "launched"},
finalStates: map[string]job.JobState{"XXX.service": "launched"},
},
// Create a new Unit
{
initJobs: []job.Job{},
initStates: map[string]job.JobState{},
item: "YYY",
item: "YYY.service",
arg: schema.Unit{
Name: "YYY",
Name: "YYY.service",
DesiredState: "loaded",
Options: []*schema.UnitOption{
&schema.UnitOption{Section: "Service", Name: "Foo", Value: "Baz"},
},
},
code: http.StatusNoContent,
finalStates: map[string]job.JobState{"YYY": "loaded"},
finalStates: map[string]job.JobState{"YYY.service": "loaded"},
},
// Creating a new Unit without Options fails
{
initJobs: []job.Job{},
initStates: map[string]job.JobState{},
item: "YYY",
item: "YYY.service",
arg: schema.Unit{
Name: "YYY",
Name: "YYY.service",
DesiredState: "loaded",
Options: []*schema.UnitOption{},
},
Expand All @@ -319,42 +319,42 @@ func TestUnitsSetDesiredState(t *testing.T) {
// Referencing a Unit where the name is inconsistent with the path should fail
{
initJobs: []job.Job{
job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")},
job.Job{Name: "YYY", Unit: newUnit(t, "[Service]\nFoo=Baz")},
job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")},
job.Job{Name: "YYY.service", Unit: newUnit(t, "[Service]\nFoo=Baz")},
},
initStates: map[string]job.JobState{
"XXX": "inactive",
"YYY": "inactive",
"XXX.service": "inactive",
"YYY.service": "inactive",
},
item: "XXX",
item: "XXX.service",
arg: schema.Unit{
Name: "YYY",
Name: "YYY.service",
DesiredState: "loaded",
},
code: http.StatusBadRequest,
finalStates: map[string]job.JobState{
"XXX": "inactive",
"YYY": "inactive",
"XXX.service": "inactive",
"YYY.service": "inactive",
},
},
// Referencing a Unit where the name is omitted should substitute the name from the path
{
initJobs: []job.Job{
job.Job{Name: "XXX", Unit: newUnit(t, "[Service]\nFoo=Bar")},
job.Job{Name: "YYY", Unit: newUnit(t, "[Service]\nFoo=Baz")},
job.Job{Name: "XXX.service", Unit: newUnit(t, "[Service]\nFoo=Bar")},
job.Job{Name: "YYY.service", Unit: newUnit(t, "[Service]\nFoo=Baz")},
},
initStates: map[string]job.JobState{
"XXX": "inactive",
"YYY": "inactive",
"XXX.service": "inactive",
"YYY.service": "inactive",
},
item: "XXX",
item: "XXX.service",
arg: schema.Unit{
DesiredState: "loaded",
},
code: http.StatusNoContent,
finalStates: map[string]job.JobState{
"XXX": "loaded",
"YYY": "inactive",
"XXX.service": "loaded",
"YYY.service": "inactive",
},
},
}
Expand Down Expand Up @@ -628,14 +628,56 @@ func TestValidateOptions(t *testing.T) {
}

func TestValidateName(t *testing.T) {
testCases := map[string]bool{
"asdf": true,
"": false,
badTestCases := []string{
// cannot be empty
"",
// cannot be longer than unitNameMax
fmt.Sprintf("%0"+strconv.Itoa(unitNameMax+1)+"s", ".service"),
fmt.Sprintf("%0"+strconv.Itoa(unitNameMax*2)+"s", ".mount"),
// must contain "."
"fooservice",
"barmount",
"bar@foo",
// cannot end in "."
"foo.",
"foo.service.",
"foo@foo.service.",
// must have valid unit suffix
"foo.bar",
"hello.cerveza",
"foo.servICE",
"yo.yo.service",
// cannot have invalid characters
"foo%.service",
"foo$asd.service",
"hello##.mount",
"yes/no.service",
// cannot start in "@"
"@foo.service",
"@this.mount",
}
for name, want := range testCases {
err := validateName(name)
if (err == nil) != want {
t.Errorf("name %q: bad validation: want %t, got err=%v", name, want, err)
for _, name := range badTestCases {
if err := validateName(name); err == nil {
t.Errorf("name %q: validation did not fail as expected!", name)
}
}

goodTestCases := []string{
"foo.service",
"hello.mount",
"foo@123.service",
"foo@.service",
"hello@world.path",
"hello:world.service",
"this+that.mount",
"yes@no\\.service",
"dog=woof@.mount",
// generate a name the exact length of unitNameMax
fmt.Sprintf("%0"+strconv.Itoa(unitNameMax)+"s", ".service"),
}
for _, name := range goodTestCases {
if err := validateName(name); err != nil {
t.Errorf("name %q: validation failed unexpectedly! err=%v", name, err)
}
}
}

0 comments on commit d0f98c6

Please sign in to comment.