Proxied ClickHouse query POST body is set to empty

[genzgd]

We're building off of an older commit but I believe this bug still exists. The GetRequestValues function in proxy/params/params.go parses the request body to find certain parameters. There are separate branches for the JSON content type and "everything else" -- and the "everything else" branch tries to parse the body as a form.

Since a ClickHouse POST can include just query text, parsing it as a form returns nothing. When the body is "restored" by SetRequestValues, it is set to the empty string returned by the Form parse, losing the original query text. This breaks all proxied POST requests to ClickHouse.

It seems like GetRequestValues should check for an appropriate content type before parsing the body as a Form, and if it's not a form content type, just return the body string "unmodified"

[jranson]

@genzgd Thanks, i will take a look. For confirming the issue and , later, the fix, would you mind providing an example request that receives the expected response, and another request that does not work as expected. Appreciate it!

[genzgd]

This curl fails for a ClickHouse origin:

curl -vk https://trickster --data "SHOW DATABASES" -H "X-ClickHouse-User: default" -H "X-ClickHouse-Key: <password>" -H "Content-type: text/plain"

It returns something like

Code: 62, e.displayText() = DB::Exception: Syntax error: failed at position 5 ('+'): +DATABASES=. Expected one of: TABLES, CLUSTER, CHANGED, GRANTS, CREATE, ACCESS, QUOTA, SETTINGS, CURRENT ROLES, PRIVILEGES, PROCESSLIST, CLUSTERS, DATABASES, CURRENT QUOTA, ENABLED ROLES, CREATE, DICTIONARIES, USERS, ROLES, SETTINGS PROFILES, PROFILES, ROW POLICIES, POLICIES, QUOTAS (version 21.4.4.30 (official build))

The + and = show that it was trying to convert SHOW DATABASES into parameters.

It should return a list of databases.

We've fixed it privately with this code for the params.go:

// GetRequestValues returns the Query Parameters for the request
// regardless of method
func GetRequestValues(r *http.Request) (url.Values, string, bool) {
	if !methods.HasBody(r.Method) {
		return r.URL.Query(), r.URL.RawQuery, false
	}
	contentType := r.Header.Get(headers.NameContentType)
	if contentType == headers.ValueApplicationJSON {
		b, _ := ioutil.ReadAll(r.Body)
		r.Body.Close()
		r.Body = ioutil.NopCloser(bytes.NewReader(b))
		return url.Values{}, string(b), true
	}
	if contentType == headers.ValueXFormURLEncoded || contentType == headers.ValueMultipartFormData {
		r.ParseForm()
		pf := r.PostForm
		s := pf.Encode()
		r.ContentLength = int64(len(s))
		r.Body = ioutil.NopCloser(bytes.NewReader([]byte(s)))
		return pf, s, true
	}
	return url.Values{}, "", true
}

// SetRequestValues Values sets the Query Parameters for the request
// regardless of method
func SetRequestValues(r *http.Request, v url.Values) {
	s := v.Encode()
	if !methods.HasBody(r.Method) {
		r.URL.RawQuery = s
	} else if len(s) > 0 {
		// reset the body
		r.ContentLength = int64(len(s))
		r.Body = ioutil.NopCloser(bytes.NewReader([]byte(s)))
	}
}

Not saying it's the best solution, but we don't touch the body unless it's a JSON or form encoded content type

[jranson]

Thanks for that!