Always set the Content-Type header when a body is present

[remilapeyre]

Closes #10011

[banks]

Thanks @remilapeyre this is a great PR.

I think it can probably be merged as-is I just had one Q about those non-JSON exceptions and whether we need to do any more diligence to make sure we didn't miss any other edge cases?

[banks]

Great job finding these exceptions! How did you find them out of interest? Did tests break for these endpoints because of the wrong content type?

I can't think of any other exceptions but then I doubt I'd have remembered this one either so wondering if there is anything else we should do to be sure we caught all the non-json endpoints.

If you already went through exhaustively though then great!

[remilapeyre]

I just remembered those two and looked briefly at the documentation and thought I had found them all. Looking at the output of rg -P 'curl.*?```' website/content/api-docs/ --multiline --multiline-dotall actually finds more cases where the payload sent is not JSON. I've added them but I'm not sure if it's possible to be sure to have found them all without doing something more complicated.

[banks]

Oh nice idea!

I tried a few things in the code to pick up possible others:

$ rg -P 'io.Copy' agent/*endpoint.go
agent/operator_license_endpoint.go
37:	if _, err := io.Copy(buf, req.Body); err != nil {

agent/kvs_endpoint.go
219:	if _, err := io.Copy(buf, req.Body); err != nil {

agent/event_endpoint.go
49:		if _, err := io.Copy(&buf, req.Body); err != nil

Which you've already got all of anyway.

I also tried:

rg -P '\.Body' agent/*.go | rg -v 'decodeBody\(|DecodeJSON\(|_test.go'
agent/watch_handler.go:		defer resp.Body.Close()
agent/watch_handler.go:		io.Copy(output, resp.Body)
agent/txn_endpoint.go:	req.Body = http.MaxBytesReader(resp, req.Body, maxTxnLen)
agent/txn_endpoint.go:							Body:                           check.Definition.Body,
agent/snapshot_endpoint.go:		if err := s.agent.delegate.SnapshotRPC(&args, req.Body, resp, nil); err != nil {
agent/operator_license_endpoint.go:	if _, err := io.Copy(buf, req.Body); err != nil {
agent/namespace_endpoint.go:	if req.Body == nil {
agent/kvs_endpoint.go:	if _, err := io.Copy(buf, req.Body); err != nil {
agent/event_endpoint.go:		if _, err := io.Copy(&buf, req.Body); err != nil {
agent/http.go:	if req.Body == nil {
agent/http.go:	dec := json.NewDecoder(req.Body)
agent/acl_endpoint.go:	policyBytes, err := ioutil.ReadAll(req.Body)
agent/agent.go:				Body:            chkType.Body,

Which are all false-positives except one: https://www.consul.io/api-docs/acl#translate-rules expects a raw HCL ACL token policy as the body so it probably needs a different content type. So I think we need an update here too:

consul/api/acl.go

Lines 896 to 918 in cc5dba1

	// RulesTranslate translates the legacy rule syntax into the current syntax.
	//
	// Deprecated: Support for the legacy syntax translation will be removed
	// when legacy ACL support is removed.
	func (a *ACL) RulesTranslate(rules io.Reader) (string, error) {
	r := a.c.newRequest("POST", "/v1/acl/rules/translate")
	r.body = rules
	rtt, resp, err := requireOK(a.c.doRequest(r))
	if err != nil {
	return "", err
	}
	defer resp.Body.Close()
	qm := &QueryMeta{}
	parseQueryMeta(resp, qm)
	qm.RequestTime = rtt
	ruleBytes, err := ioutil.ReadAll(resp.Body)
	if err != nil {
	return "", fmt.Errorf("Failed to read translated rule body: %v", err)
	}
	return string(ruleBytes), nil
	}

I think with that we can merge!

Thanks for this Remi! Much appreciated.

[remilapeyre]

Hi Paul I added the Content Type for the ACL and fixed the conflicts. I think it should be good now :)

[banks]

Perfect!

Thanks for getting this in Remi!

[banks]

GitHub did something strange here when I merged - it errored on my several times but when I went to merge manually I found the merge actually worked and this is already in master: 213524a

Despite that GH isn't marking this PR as merged and if I attempt to merge again it just creates another empty commit on master and fails again.

So I'll manually close this and go and do the 1.10 backport cherry-pick by hand.