Repository: godbus/dbus
Pull request #302 (2022-02-01 21:51:47)

Introduce and use EscapeBusAddressValue
D-Bus specification [1] requires that the values in server address need
to be escaped in a special way, and other clients perform the needed
escaping (e.g. systemd [2] does that).

More to say, if dbus sees a character that should have been escaped, it
returns an error [3].

Add EscapeBusAddressValue function (with some tests and a benchmark),
and use it from tryDiscoverDbusSessionBusAddress().

The function is exported as it might be of use to other packages (in
particular, runc [4]).

NOTE that the function does not escape '*' symbol, because de-facto it
is the list of optionally-escaped characters since 2007, but that is not
yet documented in D-Bus spec (see [5] for more details).

[1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses
[2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318
[3] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330
[4] https://github.com/opencontainers/runc/pull/3356
[5] https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/248


kolyshkin: Well, it seems that `getKey` needs unescaping, as spec says only the key values should be escaped.

Patch:
diff --git a/conn.go b/conn.go
index 1a92b8b..045a2a4 100644
--- a/conn.go
+++ b/conn.go
@@ -768,7 +768,12 @@ func getKey(s, key string) string {
 	for _, keyEqualsValue := range strings.Split(s, ",") {
 		keyValue := strings.SplitN(keyEqualsValue, "=", 2)
 		if len(keyValue) == 2 && keyValue[0] == key {
-			return keyValue[1]
+			val, err := UnescapeBusAddressValue(keyValue[1])
+			if err != nil {
+				// No way to return an error.
+				return ""
+			}
+			return val
 		}
 	}
 	return ""
diff --git a/conn_other.go b/conn_other.go
index 89e81c1..90289ca 100644
--- a/conn_other.go
+++ b/conn_other.go
@@ -54,7 +54,7 @@ func tryDiscoverDbusSessionBusAddress() string {
 		if runUserBusFile := path.Join(runtimeDirectory, "bus"); fileExists(runUserBusFile) {
 			// if /run/user/<uid>/bus exists, that file itself
 			// *is* the unix socket, so return its path
-			return fmt.Sprintf("unix:path=%s", runUserBusFile)
+			return fmt.Sprintf("unix:path=%s", EscapeBusAddressValue(runUserBusFile))
 		}
 		if runUserSessionDbusFile := path.Join(runtimeDirectory, "dbus-session"); fileExists(runUserSessionDbusFile) {
 			// if /run/user/<uid>/dbus-session exists, it's a
diff --git a/escape.go b/escape.go
new file mode 100644
index 0000000..d1509d9
--- /dev/null
+++ b/escape.go
@@ -0,0 +1,84 @@
+package dbus
+
+import "net/url"
+
+// EscapeBusAddressValue implements a requirement to escape the values
+// in D-Bus server addresses, as defined by the D-Bus specification at
+// https://dbus.freedesktop.org/doc/dbus-specification.html#addresses.
+func EscapeBusAddressValue(val string) string {
+	toEsc := strNeedsEscape(val)
+	if toEsc == 0 {
+		// Avoid unneeded allocation/copying.
+		return val
+	}
+
+	// Avoid allocation for short paths.
+	var buf [64]byte
+	var out []byte
+	// Every to-be-escaped byte needs 2 extra bytes.
+	required := len(val) + 2*toEsc
+	if required <= len(buf) {
+		out = buf[:required]
+	} else {
+		out = make([]byte, required)
+	}
+
+	j := 0
+	for i := 0; i < len(val); i++ {
+		if ch := val[i]; needsEscape(ch) {
+			// Convert ch to %xx, where xx is hex value.
+			out[j] = '%'
+			out[j+1] = hexchar(ch >> 4)
+			out[j+2] = hexchar(ch & 0x0F)
+			j += 3
+		} else {
+			out[j] = ch
+			j++
+		}
+	}
+
+	return string(out)
+}
+
+// UnescapeBusAddressValue unescapes values in D-Bus server addresses,
+// as defined by the D-Bus specification at
+// https://dbus.freedesktop.org/doc/dbus-specification.html#addresses.
+func UnescapeBusAddressValue(val string) (string, error) {
+	// Looks like url.PathUnescape does exactly what is required.
+	return url.PathUnescape(val)
+}
+
+// hexchar returns an octal representation of a n, where n < 16.
+// For invalid values of n, the function panics.
+func hexchar(n byte) byte {
+	const hex = "0123456789abcdef"
+
+	// For n >= len(hex), runtime will panic.
+	return hex[n]
+}
+
+// needsEscape tells if a byte is NOT one of optionally-escaped bytes.
+func needsEscape(c byte) bool {
+	if 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' || '0' <= c && c <= '9' {
+		return false
+	}
+	switch c {
+	case '-', '_', '/', '\\', '.', '*':
+		return false
+	}
+
+	return true
+}
+
+// strNeedsEscape tells how many bytes in the string need escaping.
+func strNeedsEscape(val string) int {
+	count := 0
+
+	for i := 0; i < len(val); i++ {
+		if needsEscape(val[i]) {
+			count++
+		}
+	}
+
+	return count
+}


Test patch:
diff --git a/escape_test.go b/escape_test.go
new file mode 100644
index 0000000..a5e1fb5
--- /dev/null
+++ b/escape_test.go
@@ -0,0 +1,53 @@
+package dbus
+
+import (
+	"testing"
+)
+
+var escapeTestCases = []struct {
+	in, out string
+}{
+	{in: "", out: ""},
+	{in: "ABCDabcdZYXzyx01289", out: "ABCDabcdZYXzyx01289"},
+	{in: `_-/\*`, out: `_-/\*`},
+	{in: `=+:~!`, out: `%3d%2b%3a%7e%21`},
+	{in: `space here`, out: `space%20here`},
+	{in: `Привет`, out: `%d0%9f%d1%80%d0%b8%d0%b2%d0%b5%d1%82`},
+	{in: `ჰეი`, out: `%e1%83%b0%e1%83%94%e1%83%98`},
+	{in: `你好`, out: `%e4%bd%a0%e5%a5%bd`},
+	{in: `こんにちは`, out: `%e3%81%93%e3%82%93%e3%81%ab%e3%81%a1%e3%81%af`},
+}
+
+// More real world examples for more fair benchmark.
+var escapeBenchmarkCases = []struct {
+	in, out string
+}{
+	{in: "/run/user/1000/bus", out: "/run/user/1000/bus"},
+	{in: "/path/with/a/single space/bus", out: "/path/with/a/single%20space/bus"},
+}
+
+func TestEscapeBusAddressValue(t *testing.T) {
+	for _, tc := range escapeTestCases {
+		out := EscapeBusAddressValue(tc.in)
+		if out != tc.out {
+			t.Errorf("input: %q; want %q, got %q", tc.in, tc.out, out)
+		}
+		in, err := UnescapeBusAddressValue(out)
+		if err != nil {
+			t.Errorf("unescape error: %v", err)
+		} else if in != tc.in {
+			t.Errorf("unescape: want %q, got %q", tc.in, in)
+		}
+	}
+}
+
+func BenchmarkEscapeBusAddressValue(b *testing.B) {
+	var out string
+
+	for i := 0; i < b.N; i++ {
+		for _, tc := range escapeBenchmarkCases {
+			out = EscapeBusAddressValue(tc.in)
+		}
+	}
+	b.Log("out:", out)
+}