From 2d1849a7b9dfa78627c2a586b188138722bf2d84 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 3 May 2022 21:42:24 -0700 Subject: [PATCH] tsweb: remove JSONHandlerFunc It's unused and we've decided it's not what we want. Change-Id: I425a0104e8869630b498a0adfd0f455876d6f92b Signed-off-by: Brad Fitzpatrick --- tsweb/jsonhandler.go | 157 ------------------- tsweb/jsonhandler_test.go | 307 -------------------------------------- tsweb/tsweb.go | 25 ++++ tsweb/tsweb_test.go | 28 ++++ 4 files changed, 53 insertions(+), 464 deletions(-) delete mode 100644 tsweb/jsonhandler.go delete mode 100644 tsweb/jsonhandler_test.go diff --git a/tsweb/jsonhandler.go b/tsweb/jsonhandler.go deleted file mode 100644 index da5182b22..000000000 --- a/tsweb/jsonhandler.go +++ /dev/null @@ -1,157 +0,0 @@ -// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package tsweb - -import ( - "bytes" - "compress/gzip" - "encoding/json" - "fmt" - "io/ioutil" - "net/http" - "strconv" - "strings" - "sync" - - "go4.org/mem" -) - -type response struct { - Status string `json:"status"` - Error string `json:"error,omitempty"` - Data any `json:"data,omitempty"` -} - -// JSONHandlerFunc is an HTTP ReturnHandler that writes JSON responses to the client. -// -// Return a HTTPError to show an error message, otherwise JSONHandlerFunc will -// only report "internal server error" to the user with status code 500. -type JSONHandlerFunc func(r *http.Request) (status int, data any, err error) - -// ServeHTTPReturn implements the ReturnHandler interface. -// -// Use the following code to unmarshal the request body -// -// body := new(DataType) -// if err := json.NewDecoder(r.Body).Decode(body); err != nil { -// return http.StatusBadRequest, nil, err -// } -// -// See jsonhandler_test.go for examples. -func (fn JSONHandlerFunc) ServeHTTPReturn(w http.ResponseWriter, r *http.Request) error { - w.Header().Set("Content-Type", "application/json") - var resp *response - status, data, err := fn(r) - if err != nil { - if werr, ok := err.(HTTPError); ok { - resp = &response{ - Status: "error", - Error: werr.Msg, - Data: data, - } - // Unwrap the HTTPError here because we are communicating with - // the client in this handler. We don't want the wrapping - // ReturnHandler to do it too. - err = werr.Err - if werr.Msg != "" { - err = fmt.Errorf("%s: %w", werr.Msg, err) - } - // take status from the HTTPError to encourage error handling in one location - if status != 0 && status != werr.Code { - err = fmt.Errorf("[unexpected] non-zero status that does not match HTTPError status, status: %d, HTTPError.code: %d: %w", status, werr.Code, err) - } - status = werr.Code - } else { - status = http.StatusInternalServerError - resp = &response{ - Status: "error", - Error: "internal server error", - } - } - } else if status == 0 { - status = http.StatusInternalServerError - resp = &response{ - Status: "error", - Error: "internal server error", - } - } else if err == nil { - resp = &response{ - Status: "success", - Data: data, - } - } - - b, jerr := json.Marshal(resp) - if jerr != nil { - w.WriteHeader(http.StatusInternalServerError) - w.Write([]byte(`{"status":"error","error":"json marshal error"}`)) - if err != nil { - return fmt.Errorf("%w, and then we could not respond: %v", err, jerr) - } - return jerr - } - - if AcceptsEncoding(r, "gzip") { - encb, err := gzipBytes(b) - if err != nil { - return err - } - w.Header().Set("Content-Encoding", "gzip") - w.Header().Set("Content-Length", strconv.Itoa(len(encb))) - w.WriteHeader(status) - w.Write(encb) - } else { - w.Header().Set("Content-Length", strconv.Itoa(len(b))) - w.WriteHeader(status) - w.Write(b) - } - return err -} - -var gzWriterPool sync.Pool // of *gzip.Writer - -// gzipBytes returns the gzipped encoding of b. -func gzipBytes(b []byte) (zb []byte, err error) { - var buf bytes.Buffer - zw, ok := gzWriterPool.Get().(*gzip.Writer) - if ok { - zw.Reset(&buf) - } else { - zw = gzip.NewWriter(&buf) - } - defer gzWriterPool.Put(zw) - if _, err := zw.Write(b); err != nil { - return nil, err - } - if err := zw.Close(); err != nil { - return nil, err - } - zb = buf.Bytes() - zw.Reset(ioutil.Discard) - return zb, nil -} - -// AcceptsEncoding reports whether r accepts the named encoding -// ("gzip", "br", etc). -func AcceptsEncoding(r *http.Request, enc string) bool { - h := r.Header.Get("Accept-Encoding") - if h == "" { - return false - } - if !strings.Contains(h, enc) && !mem.ContainsFold(mem.S(h), mem.S(enc)) { - return false - } - remain := h - for len(remain) > 0 { - var part string - part, remain, _ = strings.Cut(remain, ",") - part = strings.TrimSpace(part) - part, _, _ = strings.Cut(part, ";") - if part == enc { - return true - } - } - return false -} diff --git a/tsweb/jsonhandler_test.go b/tsweb/jsonhandler_test.go deleted file mode 100644 index 6514e7af6..000000000 --- a/tsweb/jsonhandler_test.go +++ /dev/null @@ -1,307 +0,0 @@ -// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package tsweb - -import ( - "bytes" - "compress/gzip" - "encoding/json" - "fmt" - "io" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/google/go-cmp/cmp" -) - -type Data struct { - Name string - Price int -} - -type Response struct { - Status string - Error string - Data *Data -} - -func TestNewJSONHandler(t *testing.T) { - checkStatus := func(t *testing.T, w *httptest.ResponseRecorder, status string, code int) *Response { - d := &Response{ - Data: &Data{}, - } - - bodyBytes := w.Body.Bytes() - if w.Result().Header.Get("Content-Encoding") == "gzip" { - zr, err := gzip.NewReader(bytes.NewReader(bodyBytes)) - if err != nil { - t.Fatalf("gzip read error at start: %v", err) - } - bodyBytes, err = io.ReadAll(zr) - if err != nil { - t.Fatalf("gzip read error: %v", err) - } - } - - t.Logf("%s", bodyBytes) - err := json.Unmarshal(bodyBytes, d) - if err != nil { - t.Logf(err.Error()) - return nil - } - - if d.Status == status { - t.Logf("ok: %s", d.Status) - } else { - t.Fatalf("wrong status: got: %s, want: %s", d.Status, status) - } - - if w.Code != code { - t.Fatalf("wrong status code: got: %d, want: %d", w.Code, code) - } - - if w.Header().Get("Content-Type") != "application/json" { - t.Fatalf("wrong content type: %s", w.Header().Get("Content-Type")) - } - - return d - } - - h21 := JSONHandlerFunc(func(r *http.Request) (int, any, error) { - return http.StatusOK, nil, nil - }) - - t.Run("200 simple", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/", nil) - h21.ServeHTTPReturn(w, r) - checkStatus(t, w, "success", http.StatusOK) - }) - - t.Run("403 HTTPError", func(t *testing.T) { - h := JSONHandlerFunc(func(r *http.Request) (int, any, error) { - return 0, nil, Error(http.StatusForbidden, "forbidden", nil) - }) - - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/", nil) - h.ServeHTTPReturn(w, r) - checkStatus(t, w, "error", http.StatusForbidden) - }) - - h22 := JSONHandlerFunc(func(r *http.Request) (int, any, error) { - return http.StatusOK, &Data{Name: "tailscale"}, nil - }) - - t.Run("200 get data", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "/", nil) - h22.ServeHTTPReturn(w, r) - checkStatus(t, w, "success", http.StatusOK) - }) - - h31 := JSONHandlerFunc(func(r *http.Request) (int, any, error) { - body := new(Data) - if err := json.NewDecoder(r.Body).Decode(body); err != nil { - return 0, nil, Error(http.StatusBadRequest, err.Error(), err) - } - - if body.Name == "" { - return 0, nil, Error(http.StatusBadRequest, "name is empty", nil) - } - - return http.StatusOK, nil, nil - }) - t.Run("200 post data", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{"Name": "tailscale"}`)) - h31.ServeHTTPReturn(w, r) - checkStatus(t, w, "success", http.StatusOK) - }) - - t.Run("400 bad json", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{`)) - h31.ServeHTTPReturn(w, r) - checkStatus(t, w, "error", http.StatusBadRequest) - }) - - t.Run("400 post data error", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{}`)) - h31.ServeHTTPReturn(w, r) - resp := checkStatus(t, w, "error", http.StatusBadRequest) - if resp.Error != "name is empty" { - t.Fatalf("wrong error") - } - }) - - h32 := JSONHandlerFunc(func(r *http.Request) (int, any, error) { - body := new(Data) - if err := json.NewDecoder(r.Body).Decode(body); err != nil { - return 0, nil, Error(http.StatusBadRequest, err.Error(), err) - } - if body.Name == "root" { - return 0, nil, fmt.Errorf("invalid name") - } - if body.Price == 0 { - return 0, nil, Error(http.StatusBadRequest, "price is empty", nil) - } - - return http.StatusOK, &Data{Price: body.Price * 2}, nil - }) - - t.Run("200 post data", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{"Price": 10}`)) - h32.ServeHTTPReturn(w, r) - resp := checkStatus(t, w, "success", http.StatusOK) - t.Log(resp.Data) - if resp.Data.Price != 20 { - t.Fatalf("wrong price: %d %d", resp.Data.Price, 10) - } - }) - - t.Run("gzipped", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{"Price": 10}`)) - r.Header.Set("Accept-Encoding", "gzip") - h32.ServeHTTPReturn(w, r) - res := w.Result() - if ct := res.Header.Get("Content-Encoding"); ct != "gzip" { - t.Fatalf("encoding = %q; want gzip", ct) - } - resp := checkStatus(t, w, "success", http.StatusOK) - t.Log(resp.Data) - if resp.Data.Price != 20 { - t.Fatalf("wrong price: %d %d", resp.Data.Price, 10) - } - }) - - t.Run("gzipped_400", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{"Price": 10}`)) - r.Header.Set("Accept-Encoding", "gzip") - value := []string{"foo", "foo", "foo"} - JSONHandlerFunc(func(r *http.Request) (int, any, error) { - return 400, value, nil - }).ServeHTTPReturn(w, r) - res := w.Result() - if ct := res.Header.Get("Content-Encoding"); ct != "gzip" { - t.Fatalf("encoding = %q; want gzip", ct) - } - if res.StatusCode != 400 { - t.Errorf("Status = %v; want 400", res.StatusCode) - } - }) - - t.Run("400 post data error", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{}`)) - h32.ServeHTTPReturn(w, r) - resp := checkStatus(t, w, "error", http.StatusBadRequest) - if resp.Error != "price is empty" { - t.Fatalf("wrong error") - } - }) - - t.Run("500 internal server error (unspecified error, not of type HTTPError)", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", strings.NewReader(`{"Name": "root"}`)) - h32.ServeHTTPReturn(w, r) - resp := checkStatus(t, w, "error", http.StatusInternalServerError) - if resp.Error != "internal server error" { - t.Fatalf("wrong error") - } - }) - - t.Run("500 misuse", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", nil) - JSONHandlerFunc(func(r *http.Request) (int, any, error) { - return http.StatusOK, make(chan int), nil - }).ServeHTTPReturn(w, r) - resp := checkStatus(t, w, "error", http.StatusInternalServerError) - if resp.Error != "json marshal error" { - t.Fatalf("wrong error") - } - }) - - t.Run("500 empty status code", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", nil) - JSONHandlerFunc(func(r *http.Request) (status int, data any, err error) { - return - }).ServeHTTPReturn(w, r) - checkStatus(t, w, "error", http.StatusInternalServerError) - }) - - t.Run("403 forbidden, status returned by JSONHandlerFunc and HTTPError agree", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", nil) - JSONHandlerFunc(func(r *http.Request) (int, any, error) { - return http.StatusForbidden, nil, Error(http.StatusForbidden, "403 forbidden", nil) - }).ServeHTTPReturn(w, r) - want := &Response{ - Status: "error", - Data: &Data{}, - Error: "403 forbidden", - } - got := checkStatus(t, w, "error", http.StatusForbidden) - if diff := cmp.Diff(want, got); diff != "" { - t.Fatalf(diff) - } - }) - - t.Run("403 forbidden, status returned by JSONHandlerFunc and HTTPError do not agree", func(t *testing.T) { - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/", nil) - err := JSONHandlerFunc(func(r *http.Request) (int, any, error) { - return http.StatusInternalServerError, nil, Error(http.StatusForbidden, "403 forbidden", nil) - }).ServeHTTPReturn(w, r) - if !strings.HasPrefix(err.Error(), "[unexpected]") { - t.Fatalf("returned error should have `[unexpected]` to note the disagreeing status codes: %v", err) - } - want := &Response{ - Status: "error", - Data: &Data{}, - Error: "403 forbidden", - } - got := checkStatus(t, w, "error", http.StatusForbidden) - if diff := cmp.Diff(want, got); diff != "" { - t.Fatalf("(-want,+got):\n%s", diff) - } - }) -} - -func TestAcceptsEncoding(t *testing.T) { - tests := []struct { - in, enc string - want bool - }{ - {"", "gzip", false}, - {"gzip", "gzip", true}, - {"foo,gzip", "gzip", true}, - {"foo, gzip", "gzip", true}, - {"foo, gzip ", "gzip", true}, - {"gzip, foo ", "gzip", true}, - {"gzip, foo ", "br", false}, - {"gzip, foo ", "fo", false}, - {"gzip;q=1.2, foo ", "gzip", true}, - {" gzip;q=1.2, foo ", "gzip", true}, - } - for i, tt := range tests { - h := make(http.Header) - if tt.in != "" { - h.Set("Accept-Encoding", tt.in) - } - got := AcceptsEncoding(&http.Request{Header: h}, tt.enc) - if got != tt.want { - t.Errorf("%d. got %v; want %v", i, got, tt.want) - } - } -} diff --git a/tsweb/tsweb.go b/tsweb/tsweb.go index 63b2e6071..91baec0ec 100644 --- a/tsweb/tsweb.go +++ b/tsweb/tsweb.go @@ -24,6 +24,7 @@ import ( "strings" "time" + "go4.org/mem" "inet.af/netaddr" "tailscale.com/envknob" "tailscale.com/metrics" @@ -87,6 +88,30 @@ func AllowDebugAccess(r *http.Request) bool { return false } + +// AcceptsEncoding reports whether r accepts the named encoding +// ("gzip", "br", etc). +func AcceptsEncoding(r *http.Request, enc string) bool { + h := r.Header.Get("Accept-Encoding") + if h == "" { + return false + } + if !strings.Contains(h, enc) && !mem.ContainsFold(mem.S(h), mem.S(enc)) { + return false + } + remain := h + for len(remain) > 0 { + var part string + part, remain, _ = strings.Cut(remain, ",") + part = strings.TrimSpace(part) + part, _, _ = strings.Cut(part, ";") + if part == enc { + return true + } + } + return false +} + // Protected wraps a provided debug handler, h, returning a Handler // that enforces AllowDebugAccess and returns forbidden replies for // unauthorized requests. diff --git a/tsweb/tsweb_test.go b/tsweb/tsweb_test.go index b5c081b09..24e75f8fe 100644 --- a/tsweb/tsweb_test.go +++ b/tsweb/tsweb_test.go @@ -552,3 +552,31 @@ func (promWriter) WritePrometheus(w io.Writer, prefix string) { func (promWriter) String() string { return "" } + +func TestAcceptsEncoding(t *testing.T) { + tests := []struct { + in, enc string + want bool + }{ + {"", "gzip", false}, + {"gzip", "gzip", true}, + {"foo,gzip", "gzip", true}, + {"foo, gzip", "gzip", true}, + {"foo, gzip ", "gzip", true}, + {"gzip, foo ", "gzip", true}, + {"gzip, foo ", "br", false}, + {"gzip, foo ", "fo", false}, + {"gzip;q=1.2, foo ", "gzip", true}, + {" gzip;q=1.2, foo ", "gzip", true}, + } + for i, tt := range tests { + h := make(http.Header) + if tt.in != "" { + h.Set("Accept-Encoding", tt.in) + } + got := AcceptsEncoding(&http.Request{Header: h}, tt.enc) + if got != tt.want { + t.Errorf("%d. got %v; want %v", i, got, tt.want) + } + } +}