header.Set does not reset but adds cookies

[sigmundxia]

Hello developers,

I noticed that the header.Set function seems to have a problem when handling cookies. According to the definition of Set, it seems that when operating cookies, all current cookies should be cleared and then new values ​​should be added. However, when the following code actually performs the operation, the original cookies are still retained, making the actual effect become adding cookies:

fasthttp/header.go

Lines 1474 to 1477 in 65e989e

	case caseInsensitiveCompare(strCookie, key):
	h.collectCookies()
	h.cookies = parseRequestCookies(h.cookies, value)
	return true

fasthttp/cookie.go

Lines 505 to 516 in 65e989e

	func parseRequestCookies(cookies []argsKV, src []byte) []argsKV {
	var s cookieScanner
	s.b = src
	var kv *argsKV
	cookies, kv = allocArg(cookies)
	for s.next(kv) {
	if len(kv.key) > 0 || len(kv.value) > 0 {
	cookies, kv = allocArg(cookies)
	}
	}
	return releaseArg(cookies)
	}

This also caused a downstream issue.

May I ask if I have misunderstood the meaning of the operation, and this effect is actually expected by fasthttp? Or is there indeed some problem? If there is indeed a problem, I am happy to submit a pull request to fix it.

Best wishes!

[newacorn]

Yes, among all special headers, Cookie is indeed unique in that it is the only one that allows multiple values and should be considered separately. Although there is a Header.SetCookie method that performs similar behavior, it is specifically designed for the Cookie header. Even if the semantics are slightly incorrect, it can be justified as long as documentation is provided to explain it.

Given that Request.Header includes Add and Set methods, which were designed with precise semantics in mind, addressing the issue would ensure consistency and accuracy in handling multi-value headers.

I think this has almost no backward compatibility issues.

I personally support your submission of this pull request.

[sigmundxia]

Thanks for your detailed reply! I'll try to make a pull request.

[sigmundxia]

After the discussion in pull request, a better solution would be to add a note about the behavior of cookies in header.Set. Since the pull request was merged, I think this issue can be closed :)