Merge pull request #14 from nlnwa/fix-application-x-www-form-serializing

Query parameter with empty value should be serialized with '=' according to spec
nlnwa · May 22, 2023 · 5fe754e · 5fe754e
2 parents f9c8322 + e6991e3
commit 5fe754e
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 10 deletions.
diff --git a/canonicalizer/profiles.go b/canonicalizer/profiles.go
@@ -47,6 +47,7 @@ var GoogleSafeBrowsing = New(
 		host = re.ReplaceAllString(host, ".")
 		return host
 	}),
+	url.WithSkipEqualsForEmptySearchParamsValue(),
 	WithRemovePort(),
 	WithRemoveFragment(),
 	WithRepeatedPercentDecoding(),

diff --git a/url/parseroptions.go b/url/parseroptions.go
@@ -48,6 +48,7 @@ type parserOptions struct {
 	queryPercentEncodeSet               *PercentEncodeSet
 	specialFragmentPercentEncodeSet     *PercentEncodeSet
 	fragmentPercentEncodeSet            *PercentEncodeSet
+	skipEqualsForEmptySearchParamsValue bool
 }
 
 // ParserOption configures how we parse a URL.
@@ -181,15 +182,16 @@ func WithSkipWindowsDriveLetterNormalization() ParserOption {
 // special is a map of 'scheme' => 'default port'
 //
 // WhatWg standard removed gopher from special schemes. This is how you add it back:
-// special := map[string]string{
-//                        "ftp":    "21",
-//                        "file":   "",
-//                        "http":   "80",
-//                        "https":  "443",
-//                        "ws":     "80",
-//                        "wss":    "443",
-//                        "gopher": "70",
-//                }
+//
+//	special := map[string]string{
+//	                       "ftp":    "21",
+//	                       "file":   "",
+//	                       "http":   "80",
+//	                       "https":  "443",
+//	                       "ws":     "80",
+//	                       "wss":    "443",
+//	                       "gopher": "70",
+//	               }
 //
 // This API is EXPERIMENTAL.
 func WithSpecialSchemes(special map[string]string) ParserOption {
@@ -264,3 +266,14 @@ func WithSkipTrailingSlashNormalization() ParserOption {
 		o.skipTrailingSlashNormalization = true
 	})
 }
+
+// WithSkipEqualsForEmptySearchParamsValue skips writing '=' when setting an empty value for a search parameter.
+//
+// e.g. url.SearchParams().Set("name", "") gives 'http://...?name' instead of 'http://...?name='
+//
+// This API is EXPERIMENTAL.
+func WithSkipEqualsForEmptySearchParamsValue() ParserOption {
+	return newFuncParserOption(func(o *parserOptions) {
+		o.skipEqualsForEmptySearchParamsValue = true
+	})
+}
diff --git a/url/searchparams.go b/url/searchparams.go
@@ -151,8 +151,10 @@ func (s *searchParams) String() string {
 		}
 
 		s.QueryEscape(nvp.Name, &output)
-		if nvp.Value != "" {
+		if !s.url.parser.opts.skipEqualsForEmptySearchParamsValue || nvp.Value != "" {
 			output.WriteRune('=')
+		}
+		if nvp.Value != "" {
 			s.QueryEscape(nvp.Value, &output)
 		}
 	}

diff --git a/url/searchparams_test.go b/url/searchparams_test.go
@@ -152,6 +152,8 @@ func TestUrlSearchParams_Set(t *testing.T) {
 		{"5", "http://example.com?xyz=aaa&foo=bar2&xyz=aaa&foo=bar", "foo", "xyz", "xyz=aaa&foo=xyz&xyz=aaa"},
 		{"6", "http://example.com?xyz=aaa&foo=bar2&xyz=aaa&foo=bar", "foo2", "xyz", "xyz=aaa&foo=bar2&xyz=aaa&foo=bar&foo2=xyz"},
 		{"7", "http://example.com?foo=bar&foo=fuzz&foo=barfuzz", "foo", "xyz", "foo=xyz"},
+		{"8", "http://example.com?foo=bar&foo=fuzz&foo=barfuzz", "foo", "", "foo="},
+		{"9", "http://example.com", "foo", "", "foo="},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -164,3 +166,28 @@ func TestUrlSearchParams_Set(t *testing.T) {
 		})
 	}
 }
+
+func TestUrlSearchParams_String(t *testing.T) {
+	tests := []struct {
+		name           string
+		url            string
+		wantSerialized string
+	}{
+		{"1", "http://example.com?foo=bar", "foo=bar"},
+		{"2", "http://example.com?foo=bar&foo 2=bar+2", "foo=bar&foo+2=bar+2"},
+		{"3", "http://example.com?foo2=bar2&foo=bar", "foo2=bar2&foo=bar"},
+		{"4", "http://example.com?foo=bar&foo=bar2", "foo=bar&foo=bar2"},
+		{"5", "http://example.com?xyz=aaa&foo=bar2&xyz=aaa&foo=bar", "xyz=aaa&foo=bar2&xyz=aaa&foo=bar"},
+		{"6", "http://example.com?foo=bar&foo=fuzz&foo=barfuzz", "foo=bar&foo=fuzz&foo=barfuzz"},
+		{"7", "http://example.com?foo", "foo="},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			url, _ := Parse(tt.url)
+			s := url.SearchParams()
+			if got := s.String(); got != tt.wantSerialized {
+				t.Errorf("got %v, want %v", got, tt.wantSerialized)
+			}
+		})
+	}
+}