From 374c3f4dec450b4b4ae18e6767b1c9458f530c1b Mon Sep 17 00:00:00 2001 From: Charles Korn Date: Fri, 24 Feb 2023 14:03:00 +1100 Subject: [PATCH] Implement fully-featured content negotiation for API requests, and allow overriding the default API codec. Signed-off-by: Charles Korn --- go.mod | 2 +- web/api/v1/api.go | 45 ++++++++++++------------------- web/api/v1/api_test.go | 12 +++++---- web/api/v1/codec.go | 29 +++++++++++++++++++- web/api/v1/codec_test.go | 57 ++++++++++++++++++++++++++++++++++++++++ web/api/v1/json_codec.go | 4 +-- 6 files changed, 112 insertions(+), 37 deletions(-) create mode 100644 web/api/v1/codec_test.go diff --git a/go.mod b/go.mod index ac94408e4b..a12b3505d8 100644 --- a/go.mod +++ b/go.mod @@ -160,7 +160,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/morikuni/aec v1.0.0 // indirect - github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.0.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect diff --git a/web/api/v1/api.go b/web/api/v1/api.go index 79b41607b9..0a5d475565 100644 --- a/web/api/v1/api.go +++ b/web/api/v1/api.go @@ -32,6 +32,7 @@ import ( "github.com/go-kit/log/level" "github.com/grafana/regexp" jsoniter "github.com/json-iterator/go" + "github.com/munnerz/goautoneg" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/model" @@ -80,8 +81,6 @@ const ( var LocalhostRepresentations = []string{"127.0.0.1", "localhost", "::1"} -var defaultCodec = JSONCodec{} - type apiError struct { typ errorType err error @@ -212,7 +211,7 @@ type API struct { remoteWriteHandler http.Handler remoteReadHandler http.Handler - codecs map[string]Codec + codecs []Codec } // NewAPI returns an initialized API type. @@ -271,11 +270,9 @@ func NewAPI( statsRenderer: defaultStatsRenderer, remoteReadHandler: remote.NewReadHandler(logger, registerer, q, configFunc, remoteReadSampleLimit, remoteReadConcurrencyLimit, remoteReadMaxBytesInFrame), - - codecs: map[string]Codec{}, } - a.InstallCodec(defaultCodec) + a.InstallCodec(JSONCodec{}) if statsRenderer != nil { a.statsRenderer = statsRenderer @@ -289,13 +286,15 @@ func NewAPI( } // InstallCodec adds codec to this API's available codecs. -// If codec handles a content type handled by a codec already installed in this API, codec replaces the previous codec. +// Codecs installed first take precedence over codecs installed later when evaluating wildcards in Accept headers. +// The first installed codec is used as a fallback when the Accept header cannot be satisfied or if there is no Accept header. func (api *API) InstallCodec(codec Codec) { - if api.codecs == nil { - api.codecs = map[string]Codec{} - } + api.codecs = append(api.codecs, codec) +} - api.codecs[codec.ContentType()] = codec +// ClearCodecs removes all available codecs from this API, including the default codec installed by NewAPI. +func (api *API) ClearCodecs() { + api.codecs = nil } func setUnavailStatusOnTSDBNotReady(r apiFuncResult) apiFuncResult { @@ -1591,33 +1590,23 @@ func (api *API) respond(w http.ResponseWriter, req *http.Request, data interface return } - w.Header().Set("Content-Type", codec.ContentType()) + w.Header().Set("Content-Type", codec.ContentType().String()) w.WriteHeader(http.StatusOK) if n, err := w.Write(b); err != nil { level.Error(api.logger).Log("msg", "error writing response", "bytesWritten", n, "err", err) } } -// FIXME: HTTP content negotiation is hard (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation). -// Ideally, we shouldn't be implementing this ourselves - https://github.com/golang/go/issues/19307 is an open proposal to add -// this to the Go stdlib and has links to a number of other implementations. -// -// This is an initial MVP, and doesn't support features like wildcards or weighting. func (api *API) negotiateCodec(req *http.Request, resp *Response) Codec { - acceptHeader := req.Header.Get("Accept") - if acceptHeader == "" { - return defaultCodec - } - - for _, contentType := range strings.Split(acceptHeader, ",") { - codec, ok := api.codecs[strings.TrimSpace(contentType)] - if ok && codec.CanEncode(resp) { - return codec + for _, clause := range goautoneg.ParseAccept(req.Header.Get("Accept")) { + for _, codec := range api.codecs { + if codec.ContentType().Satisfies(clause) && codec.CanEncode(resp) { + return codec + } } } - level.Warn(api.logger).Log("msg", "could not find suitable codec for response, falling back to default codec", "accept_header", acceptHeader) - return defaultCodec + return api.codecs[0] } func (api *API) respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) { diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 0531c4fe53..81b5456d67 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -2769,9 +2769,11 @@ func TestRespondSuccess(t *testing.T) { logger: log.NewNopLogger(), } - api.InstallCodec(&testCodec{contentType: "test/cannot-encode", canEncode: false}) - api.InstallCodec(&testCodec{contentType: "test/can-encode", canEncode: true}) - api.InstallCodec(&testCodec{contentType: "test/can-encode-2", canEncode: true}) + api.ClearCodecs() + api.InstallCodec(JSONCodec{}) + api.InstallCodec(&testCodec{contentType: MIMEType{"test", "cannot-encode"}, canEncode: false}) + api.InstallCodec(&testCodec{contentType: MIMEType{"test", "can-encode"}, canEncode: true}) + api.InstallCodec(&testCodec{contentType: MIMEType{"test", "can-encode-2"}, canEncode: true}) s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { api.respond(w, r, "test", nil) @@ -3307,11 +3309,11 @@ func TestGetGlobalURL(t *testing.T) { } type testCodec struct { - contentType string + contentType MIMEType canEncode bool } -func (t *testCodec) ContentType() string { +func (t *testCodec) ContentType() MIMEType { return t.contentType } diff --git a/web/api/v1/codec.go b/web/api/v1/codec.go index d11bb1fa01..492e00a74a 100644 --- a/web/api/v1/codec.go +++ b/web/api/v1/codec.go @@ -13,10 +13,12 @@ package v1 +import "github.com/munnerz/goautoneg" + // A Codec performs encoding of API responses. type Codec interface { // ContentType returns the MIME time that this Codec emits. - ContentType() string + ContentType() MIMEType // CanEncode determines if this Codec can encode resp. CanEncode(resp *Response) bool @@ -24,3 +26,28 @@ type Codec interface { // Encode encodes resp, ready for transmission to an API consumer. Encode(resp *Response) ([]byte, error) } + +type MIMEType struct { + Type string + SubType string +} + +func (m MIMEType) String() string { + return m.Type + "/" + m.SubType +} + +func (m MIMEType) Satisfies(accept goautoneg.Accept) bool { + if accept.Type == "*" && accept.SubType == "*" { + return true + } + + if accept.Type == m.Type && accept.SubType == "*" { + return true + } + + if accept.Type == m.Type && accept.SubType == m.SubType { + return true + } + + return false +} diff --git a/web/api/v1/codec_test.go b/web/api/v1/codec_test.go new file mode 100644 index 0000000000..57184a9ef0 --- /dev/null +++ b/web/api/v1/codec_test.go @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package v1 + +import ( + "testing" + + "github.com/munnerz/goautoneg" + "github.com/stretchr/testify/require" +) + +func TestMIMEType_String(t *testing.T) { + m := MIMEType{Type: "application", SubType: "json"} + + require.Equal(t, "application/json", m.String()) +} + +func TestMIMEType_Satisfies(t *testing.T) { + m := MIMEType{Type: "application", SubType: "json"} + + scenarios := map[string]struct { + accept goautoneg.Accept + expected bool + }{ + "exact match": { + accept: goautoneg.Accept{Type: "application", SubType: "json"}, + expected: true, + }, + "sub-type wildcard match": { + accept: goautoneg.Accept{Type: "application", SubType: "*"}, + expected: true, + }, + "full wildcard match": { + accept: goautoneg.Accept{Type: "*", SubType: "*"}, + expected: true, + }, + "inverted": { + accept: goautoneg.Accept{Type: "json", SubType: "application"}, + expected: false, + }, + "inverted sub-type wildcard": { + accept: goautoneg.Accept{Type: "json", SubType: "*"}, + expected: false, + }, + "complete mismatch": { + accept: goautoneg.Accept{Type: "text", SubType: "plain"}, + expected: false, + }, + } + + for name, scenario := range scenarios { + t.Run(name, func(t *testing.T) { + actual := m.Satisfies(scenario.accept) + require.Equal(t, scenario.expected, actual) + }) + } +} diff --git a/web/api/v1/json_codec.go b/web/api/v1/json_codec.go index 455af717d0..79ebfee182 100644 --- a/web/api/v1/json_codec.go +++ b/web/api/v1/json_codec.go @@ -34,8 +34,8 @@ func init() { // JSONCodec is a Codec that encodes API responses as JSON. type JSONCodec struct{} -func (j JSONCodec) ContentType() string { - return "application/json" +func (j JSONCodec) ContentType() MIMEType { + return MIMEType{Type: "application", SubType: "json"} } func (j JSONCodec) CanEncode(_ *Response) bool {