Don’t Make This Mistake with Go HTTP Servers

Ryan Collingham
6 min readNov 18, 2021
HTTP: one of the protocols that power the web

This is a story that starts with OAuth2. OAuth2 is a standard authentication flow used to enable a service to authenticate with a third-party API using a client ID and secret token.

A Mysterious Header Appears

Recently, I had to integrate a variant of the OAuth2 flow into a service.

Instead of implementing the flow myself, I reached for the oauth2 package from https://pkg.go.dev/golang.org/x/oauth2. This library provides methods which return *http.Client instances. When a request is made, these clients automatically refresh the cached authentication token if required, before including the token in the relevant auth header in the outgoing request. This saves you from implementing the authentication flow and having to manually include the auth token in all outgoing requests, letting you focus on the important parts of what you are requesting.

After integrating the authentication flow, I created a test with a mock HTTP server to handle the other side of the flow. However, when I ran the test I got a strange error: `oauth2: server response missing access_token`. After adding some extra logging, I could see that the access_token was included in the JSON response, so I was perplexed as to why the oauth2 library thought it was missing!

Even though the JSON body looked fine, the extra logging showed something else was unusual with the response — the Content-Type header. The logs were showing this header had a value of “text/plain” instead of “application/json” as I had expected. Taking a look at the oauth2 code, I could immediately understand the missing access_token error — the “text/plain” header was causing the oauth2 library to try to parse the response body as a URL-encoded form instead of as a JSON response.

That discovery explained why the error was being returned — but one mystery remained: where was the rogue “text/plain” header coming from? To recap, the response was being sent from the mock auth server I had written for the test, but when I looked at the code I could not work out how the header was being set:

func (h Handler) ServeHTTP(
w http.ResponseWriter, req *http.Request,
) {
switch req.URL.Path {
case "/auth":
w.WriteHeader(http.StatusOK)
w.Header().Add("Content-Type", "application/json")
w.Write([]byte(authRsp))
...
}
}

I was confused as to why, despite setting the Content-Type to one value in my server code, a completely different value was being received at the other end. However, after reading the docs for the http package it became clear

type ResponseWriter interface {
// Header returns the header map that will be sent by
// WriteHeader. The Header map also is the mechanism with which
// Handlers can set HTTP trailers.
//
// Changing the header map after a call to WriteHeader (or
// Write) has no effect unless the modified headers are
// trailers.
...// Write writes the data to the connection as part of an HTTP reply.
//
// If WriteHeader has not yet been called, Write calls
// WriteHeader(http.StatusOK) before writing the data. If the Header
// does not contain a Content-Type line, Write adds a Content-Type
// set to the result of passing the initial 512 bytes of written
// data to DetectContentType.

So the problem was, since I was modifying the Header map after calling WriteHeader(), the “application/json” Content-Type header was never being written to the response. Instead, when Write() was called to write the response body, the Content-Type header was automatically inferred by calling the http.DetectContentyType() function. For some reason, this function was returning the content type as “text/plain” instead of the expected JSON type.

From here the solution was trivial — simply move the w.Header().Add(…) call up above the call to w.WriteHeader() and everything works as expected! In fact, we could remove the w.WriteHeader() call entirely, since the first call to w.Write() will call w.WriteHeader(http.StatusOK) for us automatically. Although I do prefer to leave the WriteHeader call in, in order to be explicit — explicit code is almost always better than implicit code.

Lessons to learn

When writing functions to handle HTTP requests in Go, you need to become familiar with the http.ResponseWriter interface and its quirks. This interface requires you to call methods in a particular order, in order to avoid unexpected results. As a general rule of thumb, you should always call methods in this order:

  1. w.Header() to get the header map, and then call .Add() to add any header fields as required, including Content-Type.
  2. w.WriteHeader() with the status code, or you can choose to leave it out if returning http.StatusOK with a response body.
  3. w.Write() must go last, to write the response body.

A few things came together to make my oauth2 a particularly tricky issue to debug. For one thing, modifying the Header map after calling w.WriteHeader() is an action that silently fails. There should never be any reason to do this in a correct program, so it would be helpful if the http package could add a check to panic if any headers are added after the call to w.WriteHeader().

Second of all, in my view the w.Write() method tries to be a bit too clever and magical by attempting to automatically detect the Content-Type of the data being written — and in this case gets it completely wrong. If there simply was no Content-Type header at all present on the response, the oauth2 library would have assumed the data to be JSON-encoded by default and things would have worked as expected — the issue arose from the http code “helpfully” sending an incorrect Content-Type header.

Is the ResponseWriter a good design?

Personally I find the design of the ResponseWriter interface not the most intuitive. I think that it invites this kind of error where methods to mutate the response object are not called in the prescribed order. My original (wrong) assumption was that `w.WriteHeader(http.StatusOK)` would only write the status header — not that it would also write all of the headers from `w.Header()`. At the very least, having to explicitly pass in the list of headers would make it obvious that they are being consumed in addition to the status code that is already provided.

When designing any API, a good rule of thumb is to minimise the number of possible ways it can be used “wrongly” — to make it as foolproof as possible. Furthermore, a good API should make it obvious how it should be used based on the name and signature of its methods alone. If possible, relying on documentation to warn against forbidden usage patterns should be avoided. Unfortunately, an API based around side-effects does not lend itself well to this sort of design philosophy.

One way to eliminate the possibility of misuse is to change the API from one dependent on side-effects into one that uses pure functions. A pure function is one that does not cause side-effects (including mutating its arguments) but instead, returns one or more values. Given the same inputs, a pure function should always return the same outputs. Not only does such a design remove the possibility of calling ResponseWriter methods in the wrong order, it also makes unit-testing much easier — instead of having to pass in a mock ResponseWriter object and checking that its methods are called in the correct order and with the expected arguments, you can simply check that the object returned from your handler function matches the values you expect.

With pure functions in mind, I created a very small package https://github.com/ryanc414/purehttp. With this, instead of implementing the http.Handler interface, you can instead implement a purehttp.HandlerFunc. Here is an example of what it looks like:

func main() {
srv := http.Server{
Addr: "localhost:8080",
Handler: purehttp.NewHandler(MyHandler),
}
srv.ListenAndServe()
}
func MyHandler(req *http.Request) (*purehttp.Response, error) {
if req.Method != http.MethodPost {
return &purehttp.Response{StatusCode: http.StatusMethodNotAllowed}, nil
}
if err := req.ParseForm(); err != nil {
return nil, err
}
if req.Form.Get("foo") == "" {
return &purehttp.Response{StatusCode: http.StatusBadRequest}, nil
}
response := map[string]interface{}{
"code": 420,
"message": "hello world",
}
rspData, err := json.Marshal(response)
if err != nil {
return nil, err
}
return &purehttp.Response{Body: rspData, JSON: true}, nil
}

All of the information required to write the response is included in the purehttp.Response{} object. Returning a non-nil error value will result in an internal server error response being sent.

In Conclusion

We’ve talked about the ways the http.ResponseWriter interface can be used correctly and incorrectly, and about how refactoring the interface to follow a pure-functional design pattern could reduce the potential for error and allow for HTTP handlers to be more easily unit-testable. If any of this helps you to avoid making the same mistake, or makes you curious about the benefits of pure functions, please leave a comment below!

--

--