http: support additional content type

http_handler.go

@@ -23,19 +23,47 @@ package zap
 import (
 	"encoding/json"
 	"fmt"
+	"io"
+	"io/ioutil"
 	"net/http"
+	"net/url"
 
 	"go.uber.org/zap/zapcore"
 )
 
 // ServeHTTP is a simple JSON endpoint that can report on or change the current
 // logging level.
 //
-// GET requests return a JSON description of the current logging level. PUT
-// requests change the logging level and expect a payload like:
+// GET
+//
+// The GET request returns a JSON description of the current logging level like:
 //   {"level":"info"}
 //
-// It's perfectly safe to change the logging level while a program is running.
+// PUT
+//
+// The PUT request changes the logging level. It is perfectly safe to change the
+// logging level while a program is running. Two content types are supported:
+//
+//    Content-Type: application/x-www-form-urlencoded
+//
+// With this content type, the request body is expected to be URL encoded like:
+//
+//    level=debug
+//
+// This is the default content type for a curl PUT request. An example curl
+// request could look like this:
+//
+//    curl -X PUT localhost:8080/log/level -d level=debug
+//
+// For any other content type, the payload is expected to be JSON encoded and
+// look like:
+//
+//   {"level":"info"}
+//
+// An example curl request could look like this:
+//
+//    curl -X PUT localhost:8080/log/level -H "Content-Type: application/json" -d '{"level":"debug"}'
+//
 func (lvl AtomicLevel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	type errorResponse struct {
 		Error string `json:"error"`
@@ -53,25 +81,44 @@ func (lvl AtomicLevel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		enc.Encode(payload{Level: &current})
 
 	case http.MethodPut:
-		var req payload
-
-		if errmess := func() string {
-			if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-				return fmt.Sprintf("Request body must be well-formed JSON: %v", err)
-			}
-			if req.Level == nil {
-				return "Must specify a logging level."
+		requestedLvl, err := func(body io.Reader) (*zapcore.Level, error) {
+			switch r.Header.Get("Content-Type") {
+			case "application/x-www-form-urlencoded":
+				pld, err := ioutil.ReadAll(body)
+				if err != nil {
+					return nil, err
+				}
+				values, err := url.ParseQuery(string(pld))
+				if err != nil {
+					return nil, err
+				}
+				lvl := values.Get("level")
+				if lvl == "" {
+					return nil, fmt.Errorf("must specify logging level")
+				}
+				var l zapcore.Level
+				if err := l.UnmarshalText([]byte(lvl)); err != nil {
+					return nil, err
+				}
+				return &l, nil
+			default:
+				var pld payload
+				if err := json.NewDecoder(r.Body).Decode(&pld); err != nil {
+					return nil, fmt.Errorf("malformed request body: %v", err)
+				}
+				if pld.Level == nil {
+					return nil, fmt.Errorf("must specify logging level")
+				}
+				return pld.Level, nil
 			}
-			return ""
-		}(); errmess != "" {
+		}(r.Body)
+		if err != nil {
 			w.WriteHeader(http.StatusBadRequest)
-			enc.Encode(errorResponse{Error: errmess})
+			enc.Encode(errorResponse{Error: err.Error()})
 			return
 		}
-
-		lvl.SetLevel(*req.Level)
-		enc.Encode(req)
-
+		lvl.SetLevel(*requestedLvl)
+		enc.Encode(payload{Level: requestedLvl})
 	default:
 		w.WriteHeader(http.StatusMethodNotAllowed)
 		enc.Encode(errorResponse{

http_handler_test.go

@@ -22,110 +22,124 @@ package zap_test
 
 import (
 	"encoding/json"
-	"fmt"
-	"io"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"strings"
 	"testing"
 
-	. "go.uber.org/zap"
+	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func newHandler() (AtomicLevel, *Logger) {
-	lvl := NewAtomicLevel()
-	logger := New(zapcore.NewNopCore())
-	return lvl, logger
-}
-
-func assertCodeOK(t testing.TB, code int) {
-	assert.Equal(t, http.StatusOK, code, "Unexpected response status code.")
-}
-
-func assertCodeBadRequest(t testing.TB, code int) {
-	assert.Equal(t, http.StatusBadRequest, code, "Unexpected response status code.")
-}
-
-func assertCodeMethodNotAllowed(t testing.TB, code int) {
-	assert.Equal(t, http.StatusMethodNotAllowed, code, "Unexpected response status code.")
-}
-
-func assertResponse(t testing.TB, expectedLevel zapcore.Level, actualBody string) {
-	assert.Equal(t, fmt.Sprintf(`{"level":"%s"}`, expectedLevel)+"\n", actualBody, "Unexpected response body.")
-}
-
-func assertJSONError(t testing.TB, body string) {
-	// Don't need to test exact error message, but one should be present.
-	var payload map[string]interface{}
-	require.NoError(t, json.Unmarshal([]byte(body), &payload), "Expected error response to be JSON.")
-
-	msg, ok := payload["error"]
-	require.True(t, ok, "Error message is an unexpected type.")
-	assert.NotEqual(t, "", msg, "Expected an error message in response.")
-}
-
-func makeRequest(t testing.TB, method string, handler http.Handler, reader io.Reader) (int, string) {
-	ts := httptest.NewServer(handler)
-	defer ts.Close()
-
-	req, err := http.NewRequest(method, ts.URL, reader)
-	require.NoError(t, err, "Error constructing %s request.", method)
-
-	res, err := http.DefaultClient.Do(req)
-	require.NoError(t, err, "Error making %s request.", method)
-	defer res.Body.Close()
-
-	body, err := ioutil.ReadAll(res.Body)
-	require.NoError(t, err, "Error reading request body.")
-
-	return res.StatusCode, string(body)
-}
-
-func TestHTTPHandlerGetLevel(t *testing.T) {
-	lvl, _ := newHandler()
-	code, body := makeRequest(t, "GET", lvl, nil)
-	assertCodeOK(t, code)
-	assertResponse(t, lvl.Level(), body)
-}
-
-func TestHTTPHandlerPutLevel(t *testing.T) {
-	lvl, _ := newHandler()
-
-	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{"level":"warn"}`))
-
-	assertCodeOK(t, code)
-	assertResponse(t, lvl.Level(), body)
-}
-
-func TestHTTPHandlerPutUnrecognizedLevel(t *testing.T) {
-	lvl, _ := newHandler()
-	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{"level":"unrecognized-level"}`))
-	assertCodeBadRequest(t, code)
-	assertJSONError(t, body)
-}
-
-func TestHTTPHandlerNotJSON(t *testing.T) {
-	lvl, _ := newHandler()
-	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{`))
-	assertCodeBadRequest(t, code)
-	assertJSONError(t, body)
-}
-
-func TestHTTPHandlerNoLevelSpecified(t *testing.T) {
-	lvl, _ := newHandler()
-	code, body := makeRequest(t, "PUT", lvl, strings.NewReader(`{}`))
-	assertCodeBadRequest(t, code)
-	assertJSONError(t, body)
-}
-
-func TestHTTPHandlerMethodNotAllowed(t *testing.T) {
-	lvl, _ := newHandler()
-	code, body := makeRequest(t, "POST", lvl, strings.NewReader(`{`))
-	assertCodeMethodNotAllowed(t, code)
-	assertJSONError(t, body)
+func TestAtomicLevelServeHTTP(t *testing.T) {
+	tests := map[string]struct {
+		Method        string
+		ContentType   string
+		Body          string
+		ExpectedCode  int
+		ExpectedLevel zapcore.Level
+	}{
+		"GET": {
+			Method:        http.MethodGet,
+			ExpectedCode:  http.StatusOK,
+			ExpectedLevel: zap.InfoLevel,
+		},
+		"PUT JSON": {
+			Method:        http.MethodPut,
+			ExpectedCode:  http.StatusOK,
+			ExpectedLevel: zap.WarnLevel,
+			Body:          `{"level":"warn"}`,
+		},
+		"PUT URL encoded": {
+			Method:        http.MethodPut,
+			ExpectedCode:  http.StatusOK,
+			ExpectedLevel: zap.WarnLevel,
+			ContentType:   "application/x-www-form-urlencoded",
+			Body:          "level=warn",
+		},
+		"PUT JSON unrecognized": {
+			Method:       http.MethodPut,
+			ExpectedCode: http.StatusBadRequest,
+			Body:         `{"level":"unrecognized"}`,
+		},
+		"PUT URL encoded unrecognized": {
+			Method:       http.MethodPut,
+			ExpectedCode: http.StatusBadRequest,
+			ContentType:  "application/x-www-form-urlencoded",
+			Body:         "level=unrecognized",
+		},
+		"PUT JSON malformed": {
+			Method:       http.MethodPut,
+			ExpectedCode: http.StatusBadRequest,
+			Body:         `{"level":"warn`,
+		},
+		"PUT URL encoded malformed": {
+			Method:       http.MethodPut,
+			ExpectedCode: http.StatusBadRequest,
+			ContentType:  "application/x-www-form-urlencoded",
+			Body:         "level=%",
+		},
+		"PUT JSON unspecified": {
+			Method:       http.MethodPut,
+			ExpectedCode: http.StatusBadRequest,
+			Body:         `{}`,
+		},
+		"PUT URL encoded unspecified": {
+			Method:       http.MethodPut,
+			ExpectedCode: http.StatusBadRequest,
+			ContentType:  "application/x-www-form-urlencoded",
+			Body:         "",
+		},
+		"POST JSON": {
+			Method:       http.MethodPost,
+			ExpectedCode: http.StatusMethodNotAllowed,
+			Body:         `{"level":"warn"}`,
+		},
+		"POST URL": {
+			Method:       http.MethodPost,
+			ExpectedCode: http.StatusMethodNotAllowed,
+			ContentType:  "application/x-www-form-urlencoded",
+			Body:         "level=warn",
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			lvl := zap.NewAtomicLevel()
+			lvl.SetLevel(zapcore.InfoLevel)
+
+			ts := httptest.NewServer(lvl)
+			defer ts.Close()
+
+			req, err := http.NewRequest(test.Method, ts.URL, strings.NewReader(test.Body))
+			require.NoError(t, err, "Error constructing %s request.", req.Method)
+			if test.ContentType != "" {
+				req.Header.Set("Content-Type", test.ContentType)
+			}
+
+			res, err := http.DefaultClient.Do(req)
+			require.NoError(t, err, "Error making %s request.", req.Method)
+			defer res.Body.Close()
+
+			require.Equal(t, test.ExpectedCode, res.StatusCode, "Unexpected status code.")
+			if test.ExpectedCode != http.StatusOK {
+				// Don't need to test exact error message, but one should be present.
+				var pld struct {
+					Error string `json:"error"`
+				}
+				require.NoError(t, json.NewDecoder(res.Body).Decode(&pld), "Decoding response body")
+				assert.NotEmpty(t, pld.Error, "Expected an error message")
+				return
+			}
+
+			var pld struct {
+				Level zapcore.Level `json:"level"`
+			}
+			require.NoError(t, json.NewDecoder(res.Body).Decode(&pld), "Decoding response body")
+			assert.Equal(t, test.ExpectedLevel, pld.Level, "Unexpected logging level returned")
+		})
+	}
 }