Repository: valyala/fasthttp
Pull request #502 (2018-12-26 11:39:56)

fix args empty string be changed to boolen
args is a=&b=1,  when use args.String() ,  it be changed to a&b=1

erikdubbelboer: Do you have an example of a program or library that interprets those 2 as different values for `a`? I thought they mean the same?
xuecai: maybe this is nignx+lua's question.
nignx+lua code like this
```
local get = ngx.req.get_uri_args()

ngx.say("a1 is " .. type(get['a']))
ngx.say(get['a'])
ngx.say("b1 is " .. type(get['b']))
ngx.say(get['b'])
```
request is http://127.0.0.1/a.lua?a&b=
result is 
```
a1 is boolean
true
b1 is string
```

in nginx + php-fpm is ok
```
<?php
var_dump($_GET);exit;
```
request is http://127.0.0.1/a.lua?a=&b=1&c&d=1
result is 
```
array(4) { ["a"]=> string(0) "" ["b"]=> string(1) "1" ["c"]=> string(0) "" ["d"]=> string(1) "1" }
```

btw, maybe nochange is better? 
source is a&b=, args.String()  return a&b= is better than a&b ?
xuecai: i complite it in new pull  request.
please review, tks !
erikdubbelboer: Can you also look into the tests that are failing now.
erikdubbelboer: Looks good so far. I think it would also be very useful to add a function to `Args` called `SetNoValue` that just adds the key and sets `noValue` to true. Can you add that (and a proper comment explaining what the function does).
erikdubbelboer: Looks good. What do you think @kirillDanshin ?

Patch:
diff --git a/args.go b/args.go
index 4d9b35e..8b67f90 100644
--- a/args.go
+++ b/args.go
@@ -9,6 +9,11 @@ import (
 	"github.com/valyala/bytebufferpool"
 )
 
+const (
+	argsNoValue = true
+	argsHasValue = false
+)
+
 // AcquireArgs returns an empty Args object from the pool.
 //
 // The returned Args may be returned to the pool with ReleaseArgs
@@ -45,8 +50,9 @@ type Args struct {
 }
 
 type argsKV struct {
-	key   []byte
-	value []byte
+	key     []byte
+	value   []byte
+	noValue bool
 }
 
 // Reset clears query args.
@@ -114,9 +120,11 @@ func (a *Args) AppendBytes(dst []byte) []byte {
 	for i, n := 0, len(a.args); i < n; i++ {
 		kv := &a.args[i]
 		dst = AppendQuotedArg(dst, kv.key)
-		if len(kv.value) > 0 {
+		if kv.noValue == argsHasValue {
 			dst = append(dst, '=')
-			dst = AppendQuotedArg(dst, kv.value)
+			if len(kv.value) > 0 {
+				dst = AppendQuotedArg(dst, kv.value)
+			}
 		}
 		if i+1 < n {
 			dst = append(dst, '&')
@@ -147,48 +155,74 @@ func (a *Args) DelBytes(key []byte) {
 //
 // Multiple values for the same key may be added.
 func (a *Args) Add(key, value string) {
-	a.args = appendArg(a.args, key, value)
+	a.args = appendArg(a.args, key, value, argsHasValue)
 }
 
 // AddBytesK adds 'key=value' argument.
 //
 // Multiple values for the same key may be added.
 func (a *Args) AddBytesK(key []byte, value string) {
-	a.args = appendArg(a.args, b2s(key), value)
+	a.args = appendArg(a.args, b2s(key), value, argsHasValue)
 }
 
 // AddBytesV adds 'key=value' argument.
 //
 // Multiple values for the same key may be added.
 func (a *Args) AddBytesV(key string, value []byte) {
-	a.args = appendArg(a.args, key, b2s(value))
+	a.args = appendArg(a.args, key, b2s(value), argsHasValue)
 }
 
 // AddBytesKV adds 'key=value' argument.
 //
 // Multiple values for the same key may be added.
 func (a *Args) AddBytesKV(key, value []byte) {
-	a.args = appendArg(a.args, b2s(key), b2s(value))
+	a.args = appendArg(a.args, b2s(key), b2s(value), argsHasValue)
+}
+
+// AddNoValue adds only 'key' as argument without the '='.
+//
+// Multiple values for the same key may be added.
+func (a *Args) AddNoValue(key string) {
+	a.args = appendArg(a.args, key, "", argsNoValue)
+}
+
+// AddBytesKNoValue adds only 'key' as argument without the '='.
+//
+// Multiple values for the same key may be added.
+func (a *Args) AddBytesKNoValue(key []byte) {
+	a.args = appendArg(a.args, b2s(key), "", argsNoValue)
 }
 
 // Set sets 'key=value' argument.
 func (a *Args) Set(key, value string) {
-	a.args = setArg(a.args, key, value)
+	a.args = setArg(a.args, key, value, argsHasValue)
 }
 
 // SetBytesK sets 'key=value' argument.
 func (a *Args) SetBytesK(key []byte, value string) {
-	a.args = setArg(a.args, b2s(key), value)
+	a.args = setArg(a.args, b2s(key), value, argsHasValue)
 }
 
 // SetBytesV sets 'key=value' argument.
 func (a *Args) SetBytesV(key string, value []byte) {
-	a.args = setArg(a.args, key, b2s(value))
+	a.args = setArg(a.args, key, b2s(value), argsHasValue)
 }
 
 // SetBytesKV sets 'key=value' argument.
 func (a *Args) SetBytesKV(key, value []byte) {
-	a.args = setArgBytes(a.args, key, value)
+	a.args = setArgBytes(a.args, key, value, argsHasValue)
+}
+
+// SetNoValue sets only 'key' as argument without the '='.
+//
+// Only key in argumemt, like key1&key2
+func (a *Args) SetNoValue(key string) {
+	a.args = setArg(a.args, key, "", argsNoValue)
+}
+
+// SetBytesKNoValue sets 'key' argument.
+func (a *Args) SetBytesKNoValue(key []byte) {
+	a.args = setArg(a.args, b2s(key), "", argsNoValue)
 }
 
 // Peek returns query arg value for the given key.
@@ -322,7 +356,12 @@ func copyArgs(dst, src []argsKV) []argsKV {
 		dstKV := &dst[i]
 		srcKV := &src[i]
 		dstKV.key = append(dstKV.key[:0], srcKV.key...)
-		dstKV.value = append(dstKV.value[:0], srcKV.value...)
+		if srcKV.noValue == argsNoValue {
+			dstKV.value = dstKV.value[:0]
+		} else {
+			dstKV.value = append(dstKV.value[:0], srcKV.value...)
+		}
+		dstKV.noValue = srcKV.noValue
 	}
 	return dst
 }
@@ -345,31 +384,41 @@ func delAllArgs(args []argsKV, key string) []argsKV {
 	return args
 }
 
-func setArgBytes(h []argsKV, key, value []byte) []argsKV {
-	return setArg(h, b2s(key), b2s(value))
+func setArgBytes(h []argsKV, key, value []byte, noValue bool) []argsKV {
+	return setArg(h, b2s(key), b2s(value), noValue)
 }
 
-func setArg(h []argsKV, key, value string) []argsKV {
+func setArg(h []argsKV, key, value string, noValue bool) []argsKV {
 	n := len(h)
 	for i := 0; i < n; i++ {
 		kv := &h[i]
 		if key == string(kv.key) {
-			kv.value = append(kv.value[:0], value...)
+			if noValue == argsNoValue {
+				kv.value = kv.value[:0]
+			} else {
+				kv.value = append(kv.value[:0], value...)
+			}
+			kv.noValue = noValue
 			return h
 		}
 	}
-	return appendArg(h, key, value)
+	return appendArg(h, key, value, noValue)
 }
 
-func appendArgBytes(h []argsKV, key, value []byte) []argsKV {
-	return appendArg(h, b2s(key), b2s(value))
+func appendArgBytes(h []argsKV, key, value []byte, noValue bool) []argsKV {
+	return appendArg(h, b2s(key), b2s(value), noValue)
 }
 
-func appendArg(args []argsKV, key, value string) []argsKV {
+func appendArg(args []argsKV, key, value string, noValue bool) []argsKV {
 	var kv *argsKV
 	args, kv = allocArg(args)
 	kv.key = append(kv.key[:0], key...)
-	kv.value = append(kv.value[:0], value...)
+	if noValue == argsNoValue {
+		kv.value = kv.value[:0]
+	} else {
+		kv.value = append(kv.value[:0], value...)
+	}
+	kv.noValue = noValue
 	return args
 }
 
@@ -425,6 +474,7 @@ func (s *argsScanner) next(kv *argsKV) bool {
 	if len(s.b) == 0 {
 		return false
 	}
+	kv.noValue = argsHasValue
 
 	isKey := true
 	k := 0
@@ -440,6 +490,7 @@ func (s *argsScanner) next(kv *argsKV) bool {
 			if isKey {
 				kv.key = decodeArgAppend(kv.key[:0], s.b[:i])
 				kv.value = kv.value[:0]
+				kv.noValue = argsNoValue
 			} else {
 				kv.value = decodeArgAppend(kv.value[:0], s.b[k:i])
 			}
@@ -451,6 +502,7 @@ func (s *argsScanner) next(kv *argsKV) bool {
 	if isKey {
 		kv.key = decodeArgAppend(kv.key[:0], s.b)
 		kv.value = kv.value[:0]
+		kv.noValue = argsNoValue
 	} else {
 		kv.value = decodeArgAppend(kv.value[:0], s.b[k:])
 	}
diff --git a/header.go b/header.go
index 3dee640..3485f94 100644
--- a/header.go
+++ b/header.go
@@ -220,7 +220,7 @@ func (h *ResponseHeader) SetContentLength(contentLength int) {
 			h.SetConnectionClose()
 			value = strIdentity
 		}
-		h.h = setArgBytes(h.h, strTransferEncoding, value)
+		h.h = setArgBytes(h.h, strTransferEncoding, value, argsHasValue)
 	}
 }
 
@@ -267,7 +267,7 @@ func (h *RequestHeader) SetContentLength(contentLength int) {
 		h.h = delAllArgsBytes(h.h, strTransferEncoding)
 	} else {
 		h.contentLengthBytes = h.contentLengthBytes[:0]
-		h.h = setArgBytes(h.h, strTransferEncoding, strChunked)
+		h.h = setArgBytes(h.h, strTransferEncoding, strChunked, argsHasValue)
 	}
 }
 
@@ -898,7 +898,7 @@ func (h *RequestHeader) del(key []byte) {
 // Use Set for setting a single header for the given key.
 func (h *ResponseHeader) Add(key, value string) {
 	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
-	h.h = appendArg(h.h, b2s(k), value)
+	h.h = appendArg(h.h, b2s(k), value, argsHasValue)
 }
 
 // AddBytesK adds the given 'key: value' header.
@@ -981,14 +981,14 @@ func (h *ResponseHeader) SetCanonical(key, value []byte) {
 			h.SetConnectionClose()
 		} else {
 			h.ResetConnectionClose()
-			h.h = setArgBytes(h.h, key, value)
+			h.h = setArgBytes(h.h, key, value, argsHasValue)
 		}
 	case "Transfer-Encoding":
 		// Transfer-Encoding is managed automatically.
 	case "Date":
 		// Date is managed automatically.
 	default:
-		h.h = setArgBytes(h.h, key, value)
+		h.h = setArgBytes(h.h, key, value, argsHasValue)
 	}
 }
 
@@ -996,14 +996,14 @@ func (h *ResponseHeader) SetCanonical(key, value []byte) {
 //
 // It is save re-using the cookie after the function returns.
 func (h *ResponseHeader) SetCookie(cookie *Cookie) {
-	h.cookies = setArgBytes(h.cookies, cookie.Key(), cookie.Cookie())
+	h.cookies = setArgBytes(h.cookies, cookie.Key(), cookie.Cookie(), argsHasValue)
 }
 
 // SetCookie sets 'key: value' cookies.
 func (h *RequestHeader) SetCookie(key, value string) {
 	h.parseRawHeaders()
 	h.collectCookies()
-	h.cookies = setArg(h.cookies, key, value)
+	h.cookies = setArg(h.cookies, key, value, argsHasValue)
 }
 
 // SetCookieBytesK sets 'key: value' cookies.
@@ -1082,7 +1082,7 @@ func (h *RequestHeader) DelAllCookies() {
 // Use Set for setting a single header for the given key.
 func (h *RequestHeader) Add(key, value string) {
 	k := getHeaderKeyBytes(&h.bufKV, key, h.disableNormalizing)
-	h.h = appendArg(h.h, b2s(k), value)
+	h.h = appendArg(h.h, b2s(k), value, argsHasValue)
 }
 
 // AddBytesK adds the given 'key: value' header.
@@ -1166,12 +1166,12 @@ func (h *RequestHeader) SetCanonical(key, value []byte) {
 			h.SetConnectionClose()
 		} else {
 			h.ResetConnectionClose()
-			h.h = setArgBytes(h.h, key, value)
+			h.h = setArgBytes(h.h, key, value, argsHasValue)
 		}
 	case "Transfer-Encoding":
 		// Transfer-Encoding is managed automatically.
 	default:
-		h.h = setArgBytes(h.h, key, value)
+		h.h = setArgBytes(h.h, key, value, argsHasValue)
 	}
 }
 
@@ -1831,7 +1831,7 @@ func (h *ResponseHeader) parseHeaders(buf []byte) (int, error) {
 						h.connectionClose = true
 					} else {
 						h.connectionClose = false
-						h.h = appendArgBytes(h.h, s.key, s.value)
+						h.h = appendArgBytes(h.h, s.key, s.value, argsHasValue)
 					}
 					continue
 				}
@@ -1850,12 +1850,12 @@ func (h *ResponseHeader) parseHeaders(buf []byte) (int, error) {
 				if caseInsensitiveCompare(s.key, strTransferEncoding) {
 					if !bytes.Equal(s.value, strIdentity) {
 						h.contentLength = -1
-						h.h = setArgBytes(h.h, strTransferEncoding, strChunked)
+						h.h = setArgBytes(h.h, strTransferEncoding, strChunked, argsHasValue)
 					}
 					continue
 				}
 			}
-			h.h = appendArgBytes(h.h, s.key, s.value)
+			h.h = appendArgBytes(h.h, s.key, s.value, argsHasValue)
 		}
 	}
 	if s.err != nil {
@@ -1867,7 +1867,7 @@ func (h *ResponseHeader) parseHeaders(buf []byte) (int, error) {
 		h.contentLengthBytes = h.contentLengthBytes[:0]
 	}
 	if h.contentLength == -2 && !h.ConnectionUpgrade() && !h.mustSkipContentLength() {
-		h.h = setArgBytes(h.h, strTransferEncoding, strIdentity)
+		h.h = setArgBytes(h.h, strTransferEncoding, strIdentity, argsHasValue)
 		h.connectionClose = true
 	}
 	if h.noHTTP11 && !h.connectionClose {
@@ -1919,7 +1919,7 @@ func (h *RequestHeader) parseHeaders(buf []byte) (int, error) {
 						h.connectionClose = true
 					} else {
 						h.connectionClose = false
-						h.h = appendArgBytes(h.h, s.key, s.value)
+						h.h = appendArgBytes(h.h, s.key, s.value, argsHasValue)
 					}
 					continue
 				}
@@ -1927,13 +1927,13 @@ func (h *RequestHeader) parseHeaders(buf []byte) (int, error) {
 				if caseInsensitiveCompare(s.key, strTransferEncoding) {
 					if !bytes.Equal(s.value, strIdentity) {
 						h.contentLength = -1
-						h.h = setArgBytes(h.h, strTransferEncoding, strChunked)
+						h.h = setArgBytes(h.h, strTransferEncoding, strChunked, argsHasValue)
 					}
 					continue
 				}
 			}
 		}
-		h.h = appendArgBytes(h.h, s.key, s.value)
+		h.h = appendArgBytes(h.h, s.key, s.value, argsHasValue)
 	}
 	if s.err != nil {
 		h.connectionClose = true


Test patch:
diff --git a/args_test.go b/args_test.go
index 217952a..2a5cb9a 100644
--- a/args_test.go
+++ b/args_test.go
@@ -37,22 +37,24 @@ func TestArgsAdd(t *testing.T) {
 	a.Add("foo", "baz")
 	a.Add("foo", "1")
 	a.Add("ba", "23")
-	if a.Len() != 4 {
-		t.Fatalf("unexpected number of elements: %d. Expecting 4", a.Len())
+	a.Add("foo", "")
+	a.AddNoValue("foo")
+	if a.Len() != 6 {
+		t.Fatalf("unexpected number of elements: %d. Expecting 6", a.Len())
 	}
 	s := a.String()
-	expectedS := "foo=bar&foo=baz&foo=1&ba=23"
+	expectedS := "foo=bar&foo=baz&foo=1&ba=23&foo=&foo"
 	if s != expectedS {
 		t.Fatalf("unexpected result: %q. Expecting %q", s, expectedS)
 	}
 
 	var a1 Args
 	a1.Parse(s)
-	if a1.Len() != 4 {
-		t.Fatalf("unexpected number of elements: %d. Expecting 4", a.Len())
+	if a1.Len() != 6 {
+		t.Fatalf("unexpected number of elements: %d. Expecting 6", a.Len())
 	}
 
-	var barFound, bazFound, oneFound, baFound bool
+	var barFound, bazFound, oneFound, emptyFound1, emptyFound2, baFound bool
 	a1.VisitAll(func(k, v []byte) {
 		switch string(k) {
 		case "foo":
@@ -63,6 +65,12 @@ func TestArgsAdd(t *testing.T) {
 				bazFound = true
 			case "1":
 				oneFound = true
+			case "":
+				if emptyFound1 {
+					emptyFound2 = true
+				} else {
+					emptyFound1 = true
+				}
 			default:
 				t.Fatalf("unexpected value %q", v)
 			}
@@ -75,8 +83,8 @@ func TestArgsAdd(t *testing.T) {
 			t.Fatalf("unexpected key found %q", k)
 		}
 	})
-	if !barFound || !bazFound || !oneFound || !baFound {
-		t.Fatalf("something is missing: %v, %v, %v, %v", barFound, bazFound, oneFound, baFound)
+	if !barFound || !bazFound || !oneFound || !emptyFound1 || !emptyFound2 || !baFound {
+		t.Fatalf("something is missing: %v, %v, %v, %v, %v, %v", barFound, bazFound, oneFound, emptyFound1, emptyFound2, baFound)
 	}
 }
 
@@ -303,10 +311,12 @@ func TestArgsStringCompose(t *testing.T) {
 	a.Set("foo", "bar")
 	a.Set("aa", "bbb")
 	a.Set("привет", "мир")
+	a.SetNoValue("bb")
 	a.Set("", "xxxx")
 	a.Set("cvx", "")
+	a.SetNoValue("novalue")
 
-	expectedS := "foo=bar&aa=bbb&%D0%BF%D1%80%D0%B8%D0%B2%D0%B5%D1%82=%D0%BC%D0%B8%D1%80&=xxxx&cvx"
+	expectedS := "foo=bar&aa=bbb&%D0%BF%D1%80%D0%B8%D0%B2%D0%B5%D1%82=%D0%BC%D0%B8%D1%80&bb&=xxxx&cvx=&novalue"
 	s := a.String()
 	if s != expectedS {
 		t.Fatalf("Unexpected string %q. Exected %q", s, expectedS)