Skip to content
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

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

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/10204.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
api: The `Content-Type` header is now always set when a body is present in a request.
```
6 changes: 6 additions & 0 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,12 @@ func (r *request) toHTTP() (*http.Request, error) {
req.Host = r.url.Host
req.Header = r.header

// Content-Type must always be set when a body is present
// See https://github.com/hashicorp/consul/issues/10011
if req.Body != nil && req.Header.Get("Content-Type") == "" {
req.Header.Set("Content-Type", "application/json")
}

// Setup auth
if r.config.HttpAuth != nil {
req.SetBasicAuth(r.config.HttpAuth.Username, r.config.HttpAuth.Password)
Expand Down
30 changes: 29 additions & 1 deletion api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io/ioutil"
"net"
"net/http"
"net/url"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -810,7 +811,17 @@ func TestAPI_SetWriteOptions(t *testing.T) {

func TestAPI_Headers(t *testing.T) {
t.Parallel()
c, s := makeClient(t)

var request *http.Request
c, s := makeClientWithConfig(t, func(c *Config) {
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.Proxy = func(r *http.Request) (*url.URL, error) {
// Keep track of the last request sent
request = r
return nil, nil
}
c.Transport = transport
}, nil)
defer s.Stop()

if len(c.Headers()) != 0 {
Expand All @@ -836,6 +847,23 @@ func TestAPI_Headers(t *testing.T) {
if r.header.Get("Auth") != "Token" {
t.Fatalf("Auth header not set: %v", r.header)
}

kv := c.KV()
_, err := kv.Put(&KVPair{Key: "test-headers", Value: []byte("foo")}, nil)
require.NoError(t, err)
require.Equal(t, "application/json", request.Header.Get("Content-Type"))

_, _, err = kv.Get("test-headers", nil)
require.NoError(t, err)
require.Equal(t, "", request.Header.Get("Content-Type"))

_, err = kv.Delete("test-headers", nil)
require.NoError(t, err)
require.Equal(t, "", request.Header.Get("Content-Type"))

err = c.Snapshot().Restore(nil, strings.NewReader("foo"))
require.Error(t, err)
require.Equal(t, "application/octet-stream", request.Header.Get("Content-Type"))
}

func TestAPI_RequestToHTTP(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions api/operator_license.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ func (op *Operator) LicensePut(license string, opts *WriteOptions) (*LicenseRepl
r := op.c.newRequest("PUT", "/v1/operator/license")
r.setWriteOptions(opts)
r.body = strings.NewReader(license)
r.header.Set("Content-Type", "application/octet-stream")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

_, resp, err := requireOK(op.c.doRequest(r))
if err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions api/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func (s *Snapshot) Save(q *QueryOptions) (io.ReadCloser, *QueryMeta, error) {
func (s *Snapshot) Restore(q *WriteOptions, in io.Reader) error {
r := s.c.newRequest("PUT", "/v1/snapshot")
r.body = in
r.header.Set("Content-Type", "application/octet-stream")
r.setWriteOptions(q)
_, _, err := requireOK(s.c.doRequest(r))
if err != nil {
Expand Down