From a43ddc853eb657ba47f299e2aada55755f310ad5 Mon Sep 17 00:00:00 2001 From: Will McCutchen <will@mccutch.org> Date: Thu, 29 Jun 2023 19:12:09 -0400 Subject: [PATCH] Standardize common content types (#134) Cleans up inconsistent and incorrect content types for plain text, html, JSON, and binary responses. --- httpbin/handlers.go | 10 +++---- httpbin/handlers_test.go | 20 ++++++------- httpbin/helpers.go | 61 ++++++++++++++++++---------------------- httpbin/responses.go | 6 ++-- 4 files changed, 46 insertions(+), 51 deletions(-) diff --git a/httpbin/handlers.go b/httpbin/handlers.go index 097473c..0e63ceb 100644 --- a/httpbin/handlers.go +++ b/httpbin/handlers.go @@ -650,7 +650,7 @@ func (h *HTTPBin) Drip(w http.ResponseWriter, r *http.Request) { } } - w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Type", binaryContentType) w.Header().Set("Content-Length", fmt.Sprintf("%d", numBytes)) w.WriteHeader(code) @@ -728,12 +728,12 @@ func (h *HTTPBin) Robots(w http.ResponseWriter, r *http.Request) { robotsTxt := []byte(`User-agent: * Disallow: /deny `) - writeResponse(w, http.StatusOK, "text/plain", robotsTxt) + writeResponse(w, http.StatusOK, textContentType, robotsTxt) } // Deny renders a basic page that robots should never access func (h *HTTPBin) Deny(w http.ResponseWriter, r *http.Request) { - writeResponse(w, http.StatusOK, "text/plain", []byte(`YOU SHOULDN'T BE HERE`)) + writeResponse(w, http.StatusOK, textContentType, []byte(`YOU SHOULDN'T BE HERE`)) } // Cache returns a 304 if an If-Modified-Since or an If-None-Match header is @@ -875,7 +875,7 @@ func handleBytes(w http.ResponseWriter, r *http.Request, streaming bool) { return } - w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Type", binaryContentType) w.WriteHeader(http.StatusOK) var chunk []byte @@ -1061,7 +1061,7 @@ func (h *HTTPBin) Base64(w http.ResponseWriter, r *http.Request) { http.Error(w, fmt.Sprintf("%s failed: %s", b.operation, base64Error), http.StatusBadRequest) return } - writeResponse(w, http.StatusOK, "text/plain", result) + writeResponse(w, http.StatusOK, textContentType, result) } // DumpRequest - returns the given request in its HTTP/1.x wire representation. diff --git a/httpbin/handlers_test.go b/httpbin/handlers_test.go index 10d4593..6a49c1d 100644 --- a/httpbin/handlers_test.go +++ b/httpbin/handlers_test.go @@ -168,7 +168,7 @@ func TestGet(t *testing.T) { resp := must.DoReq(t, client, req) assert.StatusCode(t, resp, http.StatusMethodNotAllowed) - assert.ContentType(t, resp, "text/plain; charset=utf-8") + assert.ContentType(t, resp, textContentType) }) protoTests := []struct { @@ -1467,7 +1467,7 @@ func TestDeflate(t *testing.T) { req := newTestRequest(t, "GET", "/deflate") resp := must.DoReq(t, client, req) - assert.ContentType(t, resp, "application/json; encoding=utf-8") + assert.ContentType(t, resp, jsonContentType) assert.Header(t, resp, "Content-Encoding", "deflate") assert.StatusCode(t, resp, http.StatusOK) @@ -1710,7 +1710,7 @@ func TestDrip(t *testing.T) { elapsed := time.Since(start) assert.StatusCode(t, resp, test.code) - assert.Header(t, resp, "Content-Type", "application/octet-stream") + assert.ContentType(t, resp, binaryContentType) assert.Header(t, resp, "Content-Length", strconv.Itoa(test.numbytes)) // Note: while the /drip endpoint seems like an ideal use case for @@ -1922,7 +1922,7 @@ func TestRange(t *testing.T) { assert.Header(t, resp, "ETag", fmt.Sprintf("range%d", wantBytes)) assert.Header(t, resp, "Accept-Ranges", "bytes") assert.Header(t, resp, "Content-Length", strconv.Itoa(int(wantBytes))) - assert.ContentType(t, resp, "text/plain; charset=utf-8") + assert.ContentType(t, resp, textContentType) body := must.ReadAll(t, resp.Body) if len(body) != int(wantBytes) { @@ -2066,7 +2066,7 @@ func TestRobots(t *testing.T) { t.Parallel() req := newTestRequest(t, "GET", "/robots.txt") resp := must.DoReq(t, client, req) - assert.ContentType(t, resp, "text/plain") + assert.ContentType(t, resp, textContentType) assert.BodyContains(t, resp, `Disallow: /deny`) } @@ -2074,7 +2074,7 @@ func TestDeny(t *testing.T) { t.Parallel() req := newTestRequest(t, "GET", "/deny") resp := must.DoReq(t, client, req) - assert.ContentType(t, resp, "text/plain") + assert.ContentType(t, resp, textContentType) assert.BodyContains(t, resp, `YOU SHOULDN'T BE HERE`) } @@ -2214,7 +2214,7 @@ func TestBytes(t *testing.T) { req := newTestRequest(t, "GET", url) resp := must.DoReq(t, client, req) assert.StatusCode(t, resp, http.StatusOK) - assert.ContentType(t, resp, "application/octet-stream") + assert.ContentType(t, resp, binaryContentType) body := must.ReadAll(t, resp.Body) if len(body) != 1024 { @@ -2230,7 +2230,7 @@ func TestBytes(t *testing.T) { resp := must.DoReq(t, client, req) assert.StatusCode(t, resp, http.StatusOK) - assert.ContentType(t, resp, "application/octet-stream") + assert.ContentType(t, resp, binaryContentType) want := "\xbf\xcd*\xfa\x15\xa2\xb3r\xc7\a\x98Z\"\x02J\x8e" assert.BodyEquals(t, resp, want) @@ -2560,7 +2560,7 @@ func TestBase64(t *testing.T) { resp := must.DoReq(t, client, req) defer consumeAndCloseBody(resp) assert.StatusCode(t, resp, http.StatusOK) - assert.ContentType(t, resp, "text/plain") + assert.ContentType(t, resp, textContentType) assert.BodyEquals(t, resp, test.want) }) } @@ -2631,7 +2631,7 @@ func TestDumpRequest(t *testing.T) { req.Header.Set("x-test-header1", "Test-Value1") resp := must.DoReq(t, client, req) - assert.ContentType(t, resp, "text/plain; charset=utf-8") + assert.ContentType(t, resp, textContentType) assert.BodyEquals(t, resp, "GET /dump/request?foo=bar HTTP/1.1\r\nHost: test-host\r\nAccept-Encoding: gzip\r\nUser-Agent: Go-http-client/1.1\r\nX-Test-Header1: Test-Value1\r\nX-Test-Header2: Test-Value2\r\n\r\n") } diff --git a/httpbin/helpers.go b/httpbin/helpers.go index 9eca145..bca6813 100644 --- a/httpbin/helpers.go +++ b/httpbin/helpers.go @@ -143,44 +143,38 @@ func parseFiles(fileHeaders map[string][]*multipart.FileHeader) (map[string][]st // Note: this function expects callers to limit the the maximum size of the // request body. See, e.g., the limitRequestSize middleware. func parseBody(w http.ResponseWriter, r *http.Request, resp *bodyResponse) error { + defer r.Body.Close() + // Always set resp.Data to the incoming request body, in case we don't know // how to handle the content type body, err := io.ReadAll(r.Body) if err != nil { - r.Body.Close() return err } - resp.Data = string(body) - - // if we read an empty body, there's no need to do anything further - if len(resp.Data) == 0 { - return nil - } // After reading the body to populate resp.Data, we need to re-wrap it in // an io.Reader for further processing below - r.Body.Close() r.Body = io.NopCloser(bytes.NewBuffer(body)) - ct := r.Header.Get("Content-Type") - - // Strip of charset encoding, if present - if strings.Contains(ct, ";") { - ct = strings.Split(ct, ";")[0] + // if we read an empty body, there's no need to do anything further + if len(body) == 0 { + return nil } - switch { - // cases where we don't need to parse the body - case strings.HasPrefix(ct, "html/"): - fallthrough - case strings.HasPrefix(ct, "text/"): - // string body is already set above + // Always store the "raw" incoming request body + resp.Data = string(body) + + contentType, _, _ := strings.Cut(r.Header.Get("Content-Type"), ";") + + switch contentType { + case "text/html", "text/plain": + // no need for extra parsing, string body is already set above return nil - case ct == "application/x-www-form-urlencoded": - // r.ParseForm() does not populate r.PostForm for DELETE or GET requests, but - // we need it to for compatibility with the httpbin implementation, so - // we trick it with this ugly hack. + case "application/x-www-form-urlencoded": + // r.ParseForm() does not populate r.PostForm for DELETE or GET + // requests, but we need it to for compatibility with the httpbin + // implementation, so we trick it with this ugly hack. if r.Method == http.MethodDelete || r.Method == http.MethodGet { originalMethod := r.Method r.Method = http.MethodPost @@ -190,7 +184,8 @@ func parseBody(w http.ResponseWriter, r *http.Request, resp *bodyResponse) error return err } resp.Form = r.PostForm - case ct == "multipart/form-data": + + case "multipart/form-data": // The memory limit here only restricts how many parts will be kept in // memory before overflowing to disk: // https://golang.org/pkg/net/http/#Request.ParseMultipartForm @@ -203,16 +198,16 @@ func parseBody(w http.ResponseWriter, r *http.Request, resp *bodyResponse) error return err } resp.Files = files - case ct == "application/json": - err := json.NewDecoder(r.Body).Decode(&resp.JSON) - if err != nil && err != io.EOF { + + case "application/json": + if err := json.NewDecoder(r.Body).Decode(&resp.JSON); err != nil { return err } default: - // If we don't have a special case for the content type, we'll just return it encoded as base64 data url - // we strip off any charset information, since we will re-encode the body - resp.Data = encodeData(body, ct) + // If we don't have a special case for the content type, return it + // encoded as base64 data url + resp.Data = encodeData(body, contentType) } return nil @@ -220,13 +215,11 @@ func parseBody(w http.ResponseWriter, r *http.Request, resp *bodyResponse) error // return provided string as base64 encoded data url, with the given content type func encodeData(body []byte, contentType string) string { - data := base64.URLEncoding.EncodeToString(body) - // If no content type is provided, default to application/octet-stream if contentType == "" { - contentType = "application/octet-stream" + contentType = binaryContentType } - + data := base64.URLEncoding.EncodeToString(body) return string("data:" + contentType + ";base64," + data) } diff --git a/httpbin/responses.go b/httpbin/responses.go index f8daade..da39f34 100644 --- a/httpbin/responses.go +++ b/httpbin/responses.go @@ -6,8 +6,10 @@ import ( ) const ( - jsonContentType = "application/json; encoding=utf-8" - htmlContentType = "text/html; charset=utf-8" + binaryContentType = "application/octet-stream" + htmlContentType = "text/html; charset=utf-8" + jsonContentType = "application/json; charset=utf-8" + textContentType = "text/plain; charset=utf-8" ) type headersResponse struct { -- GitLab