Merge pull request #49 from jonboulle/master

unit names with _{a-f}{a-f} in them cause dbus to crash

dbus/dbus.go

@@ -18,6 +18,7 @@ limitations under the License.
 package dbus
 
 import (
+	"fmt"
 	"os"
 	"strconv"
 	"strings"
@@ -26,16 +27,39 @@ import (
 	"github.com/godbus/dbus"
 )
 
-const signalBuffer = 100
+const (
+	alpha        = `abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ`
+	num          = `0123456789`
+	alphanum     = alpha + num
+	signalBuffer = 100
+)
 
-// ObjectPath creates a dbus.ObjectPath using the rules that systemd uses for
-// serializing special characters.
-func ObjectPath(path string) dbus.ObjectPath {
-	path = strings.Replace(path, ".", "_2e", -1)
-	path = strings.Replace(path, "-", "_2d", -1)
-	path = strings.Replace(path, "@", "_40", -1)
+// needsEscape checks whether a byte in a potential dbus ObjectPath needs to be escaped
+func needsEscape(i int, b byte) bool {
+	// Escape everything that is not a-z-A-Z-0-9
+	// Also escape 0-9 if it's the first character
+	return strings.IndexByte(alphanum, b) == -1 ||
+		(i == 0 && strings.IndexByte(num, b) != -1)
+}
 
-	return dbus.ObjectPath(path)
+// PathBusEscape sanitizes a constituent string of a dbus ObjectPath using the
+// rules that systemd uses for serializing special characters.
+func PathBusEscape(path string) string {
+	// Special case the empty string
+	if len(path) == 0 {
+		return "_"
+	}
+	n := []byte{}
+	for i := 0; i < len(path); i++ {
+		c := path[i]
+		if needsEscape(i, c) {
+			e := fmt.Sprintf("_%x", c)
+			n = append(n, []byte(e)...)
+		} else {
+			n = append(n, c)
+		}
+	}
+	return string(n)
 }
 
 // Conn is a connection to systemd's dbus endpoint.

dbus/dbus_test.go

@@ -20,15 +20,53 @@ import (
 	"testing"
 )
 
-// TestObjectPath ensures path encoding of the systemd rules works.
-func TestObjectPath(t *testing.T) {
-	input := "/silly-path/to@a/unit..service"
-	output := ObjectPath(input)
-	expected := "/silly_2dpath/to_40a/unit_2e_2eservice"
-
-	if string(output) != expected {
-		t.Fatalf("Output '%s' did not match expected '%s'", output, expected)
+func TestNeedsEscape(t *testing.T) {
+	// Anything not 0-9a-zA-Z should always be escaped
+	for want, vals := range map[bool][]byte{
+		false: []byte{'a', 'b', 'z', 'A', 'Q', '1', '4', '9'},
+		true:  []byte{'#', '%', '$', '!', '.', '_', '-', '%', '\\'},
+	} {
+		for i := 1; i < 10; i++ {
+			for _, b := range vals {
+				got := needsEscape(i, b)
+				if got != want {
+					t.Errorf("needsEscape(%d, %c) returned %t, want %t", i, b, got, want)
+				}
+			}
+		}
 	}
+
+	// 0-9 in position 0 should be escaped
+	for want, vals := range map[bool][]byte{
+		false: []byte{'A', 'a', 'e', 'x', 'Q', 'Z'},
+		true:  []byte{'0', '4', '5', '9'},
+	} {
+		for _, b := range vals {
+			got := needsEscape(0, b)
+			if got != want {
+				t.Errorf("needsEscape(0, %c) returned %t, want %t", b, got, want)
+			}
+		}
+	}
+
+}
+
+func TestPathBusEscape(t *testing.T) {
+	for in, want := range map[string]string{
+		"":                   "_",
+		"foo.service":        "foo_2eservice",
+		"foobar":             "foobar",
+		"woof@woof.service":  "woof_40woof_2eservice",
+		"0123456":            "_30123456",
+		"account_db.service": "account_5fdb_2eservice",
+		"got-dashes":         "got_2ddashes",
+	} {
+		got := PathBusEscape(in)
+		if got != want {
+			t.Errorf("bad result for PathBusEscape(%s): got %q, want %q", in, got, want)
+		}
+	}
+
 }
 
 // TestNew ensures that New() works without errors.

dbus/methods.go

@@ -149,7 +149,7 @@ func (c *Conn) getProperties(unit string, dbusInterface string) (map[string]inte
 	var err error
 	var props map[string]dbus.Variant
 
-	path := ObjectPath("/org/freedesktop/systemd1/unit/" + unit)
+	path := unitPath(unit)
 	if !path.IsValid() {
 		return nil, errors.New("invalid unit name: " + unit)
 	}
@@ -177,7 +177,7 @@ func (c *Conn) getProperty(unit string, dbusInterface string, propertyName strin
 	var err error
 	var prop dbus.Variant
 
-	path := ObjectPath("/org/freedesktop/systemd1/unit/" + unit)
+	path := unitPath(unit)
 	if !path.IsValid() {
 		return nil, errors.New("invalid unit name: " + unit)
 	}
@@ -400,3 +400,7 @@ type DisableUnitFileChange struct {
 func (c *Conn) Reload() error {
 	return c.sysobj.Call("org.freedesktop.systemd1.Manager.Reload", 0).Store()
 }
+
+func unitPath(name string) dbus.ObjectPath {
+	return dbus.ObjectPath("/org/freedesktop/systemd1/unit/" + PathBusEscape(name))
+}