commit 5791268c391c9388d664c654e2278a50463c2c74
Author: David Fifield <da...@bamsoftware.com>
Date: Wed Oct 8 15:39:45 2014 -0700
Don't escape, just panic in formatline.
Backslash escaping in formatline was conflicting with pt-spec.txt's
specified backslash escaping for SMETHOD ARGS.
https://trac.torproject.org/projects/tor/ticket/13370
Arguably we could just ignore any illegal bytes, but I made it panic in
order to make any application bugs more obvious.
---
pt.go | 56 +++++++++++++++++++++++++++++--------------
pt_test.go | 77 +++++++++++++++++++++++++++++++++++++-----------------------
2 files changed, 86 insertions(+), 47 deletions(-)
diff --git a/pt.go b/pt.go
index 62dfc38..b02b9c2 100644
--- a/pt.go
+++ b/pt.go
@@ -201,35 +201,57 @@ func getenvRequired(key string) (string, error) {
return value, nil
}
-// Escape a string so it contains no byte values over 127 and doesn't contain
-// any of the characters '\x00' or '\n'.
-func escape(s string) string {
- var buf bytes.Buffer
- for _, b := range []byte(s) {
- if b == '\n' {
- buf.WriteString("\\n")
- } else if b == '\\' {
- buf.WriteString("\\\\")
- } else if 0 < b && b < 128 {
- buf.WriteByte(b)
- } else {
- fmt.Fprintf(&buf, "\\x%02x", b)
+// Returns true iff keyword contains only bytes allowed in a PTâTor output
line
+// keyword.
+// <KeywordChar> ::= <any US-ASCII alphanumeric, dash, and underscore>
+func keywordIsSafe(keyword string) bool {
+ for _, b := range []byte(keyword) {
+ if b >= '0' && b <= '9' {
+ continue
+ }
+ if b >= 'A' && b <= 'Z' {
+ continue
+ }
+ if b >= 'a' && b <= 'z' {
+ continue
}
+ if b == '-' || b == '_' {
+ continue
+ }
+ return false
}
- return buf.String()
+ return true
+}
+
+// Returns true iff arg contains only bytes allowed in a PTâTor output line
arg.
+// <ArgChar> ::= <any US-ASCII character but NUL or NL>
+func argIsSafe(arg string) bool {
+ for _, b := range []byte(arg) {
+ if b >= '\x80' || b == '\x00' || b == '\n' {
+ return false
+ }
+ }
+ return true
}
func formatline(keyword string, v ...string) string {
var buf bytes.Buffer
+ if !keywordIsSafe(keyword) {
+ panic(fmt.Sprintf("keyword %q contains forbidden bytes",
keyword))
+ }
buf.WriteString(keyword)
for _, x := range v {
- buf.WriteString(" " + escape(x))
+ if !argIsSafe(x) {
+ panic(fmt.Sprintf("arg %q contains forbidden bytes", x))
+ }
+ buf.WriteString(" " + x)
}
return buf.String()
}
-// Print a pluggable transports protocol line to Stdout. The line consists of
an
-// unescaped keyword, followed by any number of escaped strings.
+// Print a pluggable transports protocol line to Stdout. The line consists of a
+// keyword followed by any number of space-separated arg strings. Panics if
+// there are forbidden bytes in the keyword or the args (pt-spec.txt 2.2.1).
func line(keyword string, v ...string) {
fmt.Fprintln(Stdout, formatline(keyword, v...))
}
diff --git a/pt_test.go b/pt_test.go
index 456313a..983bf55 100644
--- a/pt_test.go
+++ b/pt_test.go
@@ -13,43 +13,60 @@ import (
"testing"
)
-func stringIsSafe(s string) bool {
- for _, c := range []byte(s) {
- if c == '\x00' || c == '\n' || c > 127 {
- return false
+func TestKeywordIsSafe(t *testing.T) {
+ tests := [...]struct {
+ keyword string
+ expected bool
+ }{
+ {"", true},
+
{"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ-_", true},
+ {"CMETHOD", true},
+ {"CMETHOD:", false},
+ {"a b c", false},
+ {"CMETHOD\x7f", false},
+ {"CMETHOD\x80", false},
+ {"CMETHOD\x81", false},
+ {"CMETHOD\xff", false},
+ {"CMÃTHOD", false},
+ }
+
+ for _, input := range tests {
+ isSafe := keywordIsSafe(input.keyword)
+ if isSafe != input.expected {
+ t.Errorf("keywordIsSafe(%q) â %v (expected %v)",
+ input.keyword, isSafe, input.expected)
}
}
- return true
}
-func TestEscape(t *testing.T) {
- tests := [...]string{
- "",
- "abc",
- "a\nb",
- "a\\b",
- "ab\\",
- "ab\\\n",
- "ab\n\\",
+func TestArgIsSafe(t *testing.T) {
+ tests := [...]struct {
+ arg string
+ expected bool
+ }{
+ {"", true},
+ {"abc", true},
+ {"127.0.0.1:8000", true},
+ {"étude", false},
+ {"a\nb", false},
+ {"a\\b", true},
+ {"ab\\", true},
+ {"ab\\\n", false},
+ {"ab\n\\", false},
+ {"abc\x7f", true},
+ {"abc\x80", false},
+ {"abc\x81", false},
+ {"abc\xff", false},
+ {"abc\xff", false},
+ {"var=GVsbG8\\=", true},
}
- check := func(input string) {
- output := escape(input)
- if stringIsSafe(input) && input != output {
- t.Errorf("escape(%q) â %q despite being safe", input,
output)
- }
- if !stringIsSafe(output) {
- t.Errorf("escape(%q) â %q is not safe", input, output)
- }
- }
for _, input := range tests {
- check(input)
- }
- for b := 0; b < 256; b++ {
- // check one-byte string with each byte value 0â255
- check(string([]byte{byte(b)}))
- // check UTF-8 encoding of each character 0â255
- check(string(b))
+ isSafe := argIsSafe(input.arg)
+ if isSafe != input.expected {
+ t.Errorf("argIsSafe(%q) â %v (expected %v)",
+ input.arg, isSafe, input.expected)
+ }
}
}
_______________________________________________
tor-commits mailing list
tor-commits@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits
