Fixes to URL functionality:

- Extend http.URLUnescape to convert '+' to space
- Add http.URLEscape
- Rename URL.Query to EncodedQuery (and stop decoding it, as decoding this field
  before separating key/value pairs loses important information)
- Report a clean error on incomplete hex escapes
- Update existing tests, add new ones

APPROVED=rsc
DELTA=293  (256 added, 3 deleted, 34 changed)
OCL=29685
CL=29759

1 files changed, 187 insertions, 13 deletions

diff --git a/src/lib/http/url_test.go b/src/lib/http/url_test.go
index f5a7069ae..8d8fabad5 100644
--- a/src/lib/http/url_test.go
+++ b/src/lib/http/url_test.go
@@ -20,6 +20,7 @@ import (
 type URLTest struct {
 	in string;
 	out *URL;
+	roundtrip string; // expected result of reserializing the URL; empty means same as "in".
 }
 
 var urltests = []URLTest {
@@ -31,7 +32,8 @@ var urltests = []URLTest {
 			"http", "//www.google.com",
 			"www.google.com", "", "www.google.com",
 			"", "", ""
-		}
+		},
+		""
 	},
 	// path
 	URLTest{
@@ -41,7 +43,19 @@ var urltests = []URLTest {
 			"http", "//www.google.com/",
 			"www.google.com", "", "www.google.com",
 			"/", "", ""
-		}
+		},
+		""
+	},
+	// path with hex escaping... note that space roundtrips to +
+	URLTest{
+		"http://www.google.com/file%20one%26two",
+		&URL{
+			"http://www.google.com/file%20one%26two",
+			"http", "//www.google.com/file%20one%26two",
+			"www.google.com", "", "www.google.com",
+			"/file one&two", "", ""
+		},
+		"http://www.google.com/file+one%26two"
 	},
 	// user
 	URLTest{
@@ -51,7 +65,19 @@ var urltests = []URLTest {
 			"ftp", "//webmaster@www.google.com/",
 			"webmaster@www.google.com", "webmaster", "www.google.com",
 			"/", "", ""
-		}
+		},
+		""
+	},
+	// escape sequence in username
+	URLTest{
+		"ftp://john%20doe@www.google.com/",
+		&URL{
+			"ftp://john%20doe@www.google.com/",
+			"ftp", "//john%20doe@www.google.com/",
+			"john doe@www.google.com", "john doe", "www.google.com",
+			"/", "", ""
+		},
+		"ftp://john+doe@www.google.com/"
 	},
 	// query
 	URLTest{
@@ -61,7 +87,19 @@ var urltests = []URLTest {
 			"http", "//www.google.com/?q=go+language",
 			"www.google.com", "", "www.google.com",
 			"/", "q=go+language", ""
-		}
+		},
+		""
+	},
+	// query with hex escaping: NOT parsed
+	URLTest{
+		"http://www.google.com/?q=go%20language",
+		&URL{
+			"http://www.google.com/?q=go%20language",
+			"http", "//www.google.com/?q=go%20language",
+			"www.google.com", "", "www.google.com",
+			"/", "q=go%20language", ""
+		},
+		""
 	},
 	// path without /, so no query parsing
 	URLTest{
@@ -70,8 +108,9 @@ var urltests = []URLTest {
 			"http:www.google.com/?q=go+language",
 			"http", "www.google.com/?q=go+language",
 			"", "", "",
-			"www.google.com/?q=go+language", "", ""
-		}
+			"www.google.com/?q=go language", "", ""
+		},
+		"http:www.google.com/%3fq%3dgo+language"
 	},
 	// non-authority
 	URLTest{
@@ -81,7 +120,8 @@ var urltests = []URLTest {
 			"mailto", "/webmaster@golang.org",
 			"", "", "",
 			"/webmaster@golang.org", "", ""
-		}
+		},
+		""
 	},
 	// non-authority
 	URLTest{
@@ -91,7 +131,8 @@ var urltests = []URLTest {
 			"mailto", "webmaster@golang.org",
 			"", "", "",
 			"webmaster@golang.org", "", ""
-		}
+		},
+		""
 	},
 }
 
@@ -103,7 +144,8 @@ var urlnofragtests = []URLTest {
 			"http", "//www.google.com/?q=go+language#foo",
 			"www.google.com", "", "www.google.com",
 			"/", "q=go+language#foo", ""
-		}
+		},
+		""
 	},
 }
 
@@ -115,7 +157,18 @@ var urlfragtests = []URLTest {
 			"http", "//www.google.com/?q=go+language",
 			"www.google.com", "", "www.google.com",
 			"/", "q=go+language", "foo"
-		}
+		},
+		""
+	},
+	URLTest{
+		"http://www.google.com/?q=go+language#foo%26bar",
+		&URL{
+			"http://www.google.com/?q=go+language",
+			"http", "//www.google.com/?q=go+language",
+			"www.google.com", "", "www.google.com",
+			"/", "q=go+language", "foo&bar"
+		},
+		""
 	},
 }
 
@@ -123,7 +176,7 @@ var urlfragtests = []URLTest {
 func ufmt(u *URL) string {
 	return fmt.Sprintf("%q, %q, %q, %q, %q, %q, %q, %q, %q",
 		u.Raw, u.Scheme, u.RawPath, u.Authority, u.Userinfo,
-		u.Host, u.Path, u.Query, u.Fragment);
+		u.Host, u.Path, u.RawQuery, u.Fragment);
 }
 
 func DoTest(t *testing.T, parse func(string) (*URL, os.Error), name string, tests []URLTest) {
@@ -158,8 +211,12 @@ func DoTestString(t *testing.T, parse func(string) (*URL, os.Error), name string
 			continue;
 		}
 		s := u.String();
-		if s != tt.in {
-			t.Errorf("%s(%q).String() == %q", tt.in, s);
+		expected := tt.in;
+		if len(tt.roundtrip) > 0 {
+			expected = tt.roundtrip;
+		}
+		if s != expected {
+			t.Errorf("%s(%q).String() == %q (expected %q)", name, tt.in, s, expected);
 		}
 	}
 }
@@ -172,3 +229,120 @@ func TestURLString(t *testing.T) {
 	DoTestString(t, ParseURLReference, "ParseURLReference", urlfragtests);
 	DoTestString(t, ParseURLReference, "ParseURLReference", urlnofragtests);
 }
+
+type URLEscapeTest struct {
+	in string;
+	out string;
+	err os.Error;
+}
+
+var unescapeTests = []URLEscapeTest {
+	URLEscapeTest{
+		"",
+		"",
+		nil
+	},
+	URLEscapeTest{
+		"abc",
+		"abc",
+		nil
+	},
+	URLEscapeTest{
+		"1%41",
+		"1A",
+		nil
+	},
+	URLEscapeTest{
+		"1%41%42%43",
+		"1ABC",
+		nil
+	},
+	URLEscapeTest{
+		"%4a",
+		"J",
+		nil
+	},
+	URLEscapeTest{
+		"%6F",
+		"o",
+		nil
+	},
+	URLEscapeTest{
+		"%", // not enough characters after %
+		"",
+		BadURL{"invalid hexadecimal escape"}
+	},
+	URLEscapeTest{
+		"%a", // not enough characters after %
+		"",
+		BadURL{"invalid hexadecimal escape"}
+	},
+	URLEscapeTest{
+		"%1", // not enough characters after %
+		"",
+		BadURL{"invalid hexadecimal escape"}
+	},
+	URLEscapeTest{
+		"123%45%6", // not enough characters after %
+		"",
+		BadURL{"invalid hexadecimal escape"}
+	},
+	URLEscapeTest{
+		"%zz", // invalid hex digits
+		"",
+		BadURL{"invalid hexadecimal escape"}
+	},
+}
+
+func TestURLUnescape(t *testing.T) {
+	for i, tt := range unescapeTests {
+		actual, err := URLUnescape(tt.in);
+		if actual != tt.out || (err != nil) != (tt.err != nil) {
+			t.Errorf("URLUnescape(%q) = %q, %s; want %q, %s", tt.in, actual, err, tt.out, tt.err);
+		}
+	}
+}
+
+var escapeTests = []URLEscapeTest {
+	URLEscapeTest{
+		"",
+		"",
+		nil
+	},
+	URLEscapeTest{
+		"abc",
+		"abc",
+		nil
+	},
+	URLEscapeTest{
+		"one two",
+		"one+two",
+		nil
+	},
+	URLEscapeTest{
+		"10%",
+		"10%25",
+		nil
+	},
+	URLEscapeTest{
+		" ?&=#+%!",
+		"+%3f%26%3d%23%2b%25!",
+		nil
+	},
+}
+
+func TestURLEscape(t *testing.T) {
+	for i, tt := range escapeTests {
+		actual := URLEscape(tt.in);
+		if tt.out != actual {
+			t.Errorf("URLEscape(%q) = %q, want %q", tt.in, actual, tt.out);
+		}
+
+		// for bonus points, verify that escape:unescape is an identity.
+		roundtrip, err := URLUnescape(actual);
+		if roundtrip != tt.in || err != nil {
+			t.Errorf("URLUnescape(%q) = %q, %s; want %q, %s", actual, roundtrip, err, tt.in, "[no error]");
+		}
+	}
+}
+