New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

httputil/header: ParseValueAndParams behavior for headers value containing special character #539

Open
posener opened this Issue Mar 3, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@posener
Copy link

posener commented Mar 3, 2018

header.ParseValueAndParams can change it's behavior when parsing headers.
It looks for the header value and stops on any special character (\t"(),:;<=>?@[]\{}, characters that are lower than or equal to 31, and the 127th character) but then to enter the param parsing loop it looks only for ";" as the next character.

This leads to header with value containing special not parse the header correctly. (Or this is the right behavior?)

For example:

func main()  {
	h := make(http.Header)
	h.Add("key", "a;c=d")
	val, params := header.ParseValueAndParams(h, "key")
	fmt.Println(val, params)
}

Results in a map[c:d]

But

func main()  {
	h := make(http.Header)
-	h.Add("key", "a;c=d")
+	h.Add("key", "a a;c=d")
	val, params := header.ParseValueAndParams(h, "key")
	fmt.Println(val, params)
}

Results in a.

Maybe the value parsing should only stop at ";" character?

@dmitshur dmitshur changed the title header.ParseValueAndParams behavior for headers value containing special character httputil/header: ParseValueAndParams behavior for headers value containing special character Mar 3, 2018

@andybons

This comment has been minimized.

Copy link
Member

andybons commented Feb 6, 2019

Seems OK as long at it abides by the RFC. Any thoughts, @bradfitz @dmitshur?

@dmitshur

This comment has been minimized.

Copy link
Member

dmitshur commented Feb 6, 2019

I am not familiar with the semantics of ParseValueAndParams to have a strong opinion. If it's within spec and doesn't break gddo, I don't have objections.

That said, I would like to see this package move in the direction of being made internal. It should be serving the needs of gddo only.

Packages in x/net subrepo (e.g., golang.org/x/net/http/httpguts) or elsewhere (e.g., https://godoc.org/?q=http) can serve general-purpose HTTP header parsing needs better. They would be better equipped to incorporate such changes, and more people would be able to benefit from them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment