From 86100419c788d5f10b8bb1ea5ea98265206d6784 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Fri, 6 Dec 2019 11:34:00 -0500 Subject: [PATCH 1/2] application/x-www-form-urlencoded support --- pkg/loghttp/labels_test.go | 3 ++ pkg/loghttp/params.go | 16 +++--- pkg/loghttp/params_test.go | 2 + pkg/loghttp/query_test.go | 6 +++ pkg/loghttp/tail_test.go | 3 ++ pkg/loki/modules.go | 1 + pkg/querier/http.go | 31 +++++++++++ pkg/querier/http_test.go | 106 +++++++++++++++++++++++++++++++++++++ 8 files changed, 160 insertions(+), 8 deletions(-) create mode 100644 pkg/querier/http_test.go diff --git a/pkg/loghttp/labels_test.go b/pkg/loghttp/labels_test.go index d2f452e52ced..00f4c23a4443 100644 --- a/pkg/loghttp/labels_test.go +++ b/pkg/loghttp/labels_test.go @@ -8,6 +8,7 @@ import ( "github.com/gorilla/mux" "github.com/grafana/loki/pkg/logproto" + "github.com/stretchr/testify/require" ) func TestParseLabelQuery(t *testing.T) { @@ -42,6 +43,8 @@ func TestParseLabelQuery(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + err := tt.r.ParseForm() + require.Nil(t, err) got, err := ParseLabelQuery(tt.r) if (err != nil) != tt.wantErr { t.Errorf("ParseLabelQuery() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/loghttp/params.go b/pkg/loghttp/params.go index 948b0b7a65c2..4f054c9d9baa 100644 --- a/pkg/loghttp/params.go +++ b/pkg/loghttp/params.go @@ -20,7 +20,7 @@ const ( ) func limit(r *http.Request) (uint32, error) { - l, err := parseInt(r.URL.Query().Get("limit"), defaultQueryLimit) + l, err := parseInt(r.Form.Get("limit"), defaultQueryLimit) if err != nil { return 0, err } @@ -28,24 +28,24 @@ func limit(r *http.Request) (uint32, error) { } func query(r *http.Request) string { - return r.URL.Query().Get("query") + return r.Form.Get("query") } func ts(r *http.Request) (time.Time, error) { - return parseTimestamp(r.URL.Query().Get("time"), time.Now()) + return parseTimestamp(r.Form.Get("time"), time.Now()) } func direction(r *http.Request) (logproto.Direction, error) { - return parseDirection(r.URL.Query().Get("direction"), logproto.BACKWARD) + return parseDirection(r.Form.Get("direction"), logproto.BACKWARD) } func bounds(r *http.Request) (time.Time, time.Time, error) { now := time.Now() - start, err := parseTimestamp(r.URL.Query().Get("start"), now.Add(-defaultSince)) + start, err := parseTimestamp(r.Form.Get("start"), now.Add(-defaultSince)) if err != nil { return time.Time{}, time.Time{}, err } - end, err := parseTimestamp(r.URL.Query().Get("end"), now) + end, err := parseTimestamp(r.Form.Get("end"), now) if err != nil { return time.Time{}, time.Time{}, err } @@ -53,7 +53,7 @@ func bounds(r *http.Request) (time.Time, time.Time, error) { } func step(r *http.Request, start, end time.Time) (time.Duration, error) { - value := r.URL.Query().Get("step") + value := r.Form.Get("step") if value == "" { return time.Duration(defaultQueryRangeStep(start, end)) * time.Second, nil } @@ -78,7 +78,7 @@ func defaultQueryRangeStep(start time.Time, end time.Time) int { } func tailDelay(r *http.Request) (uint32, error) { - l, err := parseInt(r.URL.Query().Get("delay_for"), 0) + l, err := parseInt(r.Form.Get("delay_for"), 0) if err != nil { return 0, err } diff --git a/pkg/loghttp/params_test.go b/pkg/loghttp/params_test.go index 7146a92e55dc..59fa6e4809fc 100644 --- a/pkg/loghttp/params_test.go +++ b/pkg/loghttp/params_test.go @@ -125,6 +125,8 @@ func TestHttp_ParseRangeQuery_Step(t *testing.T) { t.Run(testName, func(t *testing.T) { req := httptest.NewRequest("GET", testData.reqPath, nil) + err := req.ParseForm() + require.Nil(t, err) actual, err := ParseRangeQuery(req) require.NoError(t, err) diff --git a/pkg/loghttp/query_test.go b/pkg/loghttp/query_test.go index 09150c7b20b4..4a91b3429030 100644 --- a/pkg/loghttp/query_test.go +++ b/pkg/loghttp/query_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/grafana/loki/pkg/logproto" + "github.com/stretchr/testify/require" ) func TestParseRangeQuery(t *testing.T) { @@ -53,6 +54,9 @@ func TestParseRangeQuery(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + err := tt.r.ParseForm() + require.Nil(t, err) + got, err := ParseRangeQuery(tt.r) if (err != nil) != tt.wantErr { t.Errorf("ParseRangeQuery() error = %v, wantErr %v", err, tt.wantErr) @@ -91,6 +95,8 @@ func TestParseInstantQuery(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + err := tt.r.ParseForm() + require.Nil(t, err) got, err := ParseInstantQuery(tt.r) if (err != nil) != tt.wantErr { t.Errorf("ParseInstantQuery() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/loghttp/tail_test.go b/pkg/loghttp/tail_test.go index 36de31d16fc4..3c5aa71aedb2 100644 --- a/pkg/loghttp/tail_test.go +++ b/pkg/loghttp/tail_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/grafana/loki/pkg/logproto" + "github.com/stretchr/testify/require" ) func TestParseTailQuery(t *testing.T) { @@ -40,6 +41,8 @@ func TestParseTailQuery(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + err := tt.r.ParseForm() + require.Nil(t, err) got, err := ParseTailQuery(tt.r) if (err != nil) != tt.wantErr { t.Errorf("ParseTailQuery() error = %v, wantErr %v", err, tt.wantErr) diff --git a/pkg/loki/modules.go b/pkg/loki/modules.go index cee38c635a49..29faf75f6147 100644 --- a/pkg/loki/modules.go +++ b/pkg/loki/modules.go @@ -159,6 +159,7 @@ func (t *Loki) initQuerier() (err error) { httpMiddleware := middleware.Merge( t.httpAuthMiddleware, + querier.NewPrepopulateMiddleware(), ) t.server.HTTP.Path("/ready").Handler(http.HandlerFunc(t.querier.ReadinessHandler)) diff --git a/pkg/querier/http.go b/pkg/querier/http.go index ca232d3b17e8..a4b1fb6291f8 100644 --- a/pkg/querier/http.go +++ b/pkg/querier/http.go @@ -17,6 +17,7 @@ import ( "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "github.com/weaveworks/common/httpgrpc" + "github.com/weaveworks/common/middleware" ) const ( @@ -217,6 +218,36 @@ func (q *Querier) TailHandler(w http.ResponseWriter, r *http.Request) { } } +// NewPrepopulateMiddleware creates a middleware which will parse incoming http forms. +// This is important because some endpoints can POST x-www-form-urlencoded bodies instead of GET w/ query strings. +func NewPrepopulateMiddleware() middleware.Interface { + return middleware.Func(func(next http.Handler) http.Handler { + return &prepop{ + next: next, + } + }) +} + +type prepop struct { + next http.Handler +} + +func (p *prepop) ServeHTTP(w http.ResponseWriter, req *http.Request) { + err := req.ParseForm() + if err != nil { + status := http.StatusBadRequest + http.Error( + w, + httpgrpc.Errorf(http.StatusBadRequest, err.Error()).Error(), + status, + ) + return + + } + p.next.ServeHTTP(w, req) + +} + // parseRegexQuery parses regex and query querystring from httpRequest and returns the combined LogQL query. // This is used only to keep regexp query string support until it gets fully deprecated. func parseRegexQuery(httpRequest *http.Request) (string, error) { diff --git a/pkg/querier/http_test.go b/pkg/querier/http_test.go new file mode 100644 index 000000000000..b20b7f63e7c8 --- /dev/null +++ b/pkg/querier/http_test.go @@ -0,0 +1,106 @@ +package querier + +import ( + "bytes" + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPrepopulate(t *testing.T) { + + success := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.Write([]byte("ok")) + }) + + for _, tc := range []struct { + desc string + method string + qs string + body io.Reader + expected url.Values + error bool + }{ + { + desc: "passthrough GET w/ querystring", + method: "GET", + qs: "?" + url.Values{"foo": {"bar"}}.Encode(), + body: nil, + expected: url.Values{ + "foo": {"bar"}, + }, + }, + { + desc: "passthrough POST w/ querystring", + method: "POST", + qs: "?" + url.Values{"foo": {"bar"}}.Encode(), + body: nil, + expected: url.Values{ + "foo": {"bar"}, + }, + }, + { + desc: "parse form body", + method: "POST", + qs: "", + body: bytes.NewBuffer([]byte(url.Values{ + "match": {"up", "down"}, + }.Encode())), + expected: url.Values{ + "match": {"up", "down"}, + }, + }, + { + desc: "querystring extends form body", + method: "POST", + qs: "?" + url.Values{ + "match": {"sideways"}, + "foo": {"bar"}, + }.Encode(), + body: bytes.NewBuffer([]byte(url.Values{ + "match": {"up", "down"}, + }.Encode())), + expected: url.Values{ + "match": {"up", "down", "sideways"}, + "foo": {"bar"}, + }, + }, + { + desc: "nil body", + method: "POST", + body: nil, + error: true, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + req := httptest.NewRequest(tc.method, "http://testing"+tc.qs, tc.body) + + // For some reason nil bodies aren't maintained after passed to httptest.NewRequest, + // but are a failure condition for parsing the form data. + // Therefore set to nil if we're passing a nil body to force an error. + if tc.body == nil { + req.Body = nil + } + + if tc.method == "POST" { + req.Header["Content-Type"] = []string{"application/x-www-form-urlencoded"} + } + + w := httptest.NewRecorder() + mware := NewPrepopulateMiddleware().Wrap(success) + + mware.ServeHTTP(w, req) + + if tc.error { + require.Equal(t, http.StatusBadRequest, w.Result().StatusCode) + } else { + require.Equal(t, tc.expected, req.Form) + } + + }) + } +} From 59be388fedd691548eab97cba1267d4f8c4f5950 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Sat, 7 Dec 2019 10:31:45 -0500 Subject: [PATCH 2/2] handler func instead of unnecessary struct --- pkg/querier/http.go | 37 ++++++++++++++----------------------- pkg/querier/http_test.go | 4 ++-- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/pkg/querier/http.go b/pkg/querier/http.go index a4b1fb6291f8..0a83f31d85b9 100644 --- a/pkg/querier/http.go +++ b/pkg/querier/http.go @@ -222,30 +222,21 @@ func (q *Querier) TailHandler(w http.ResponseWriter, r *http.Request) { // This is important because some endpoints can POST x-www-form-urlencoded bodies instead of GET w/ query strings. func NewPrepopulateMiddleware() middleware.Interface { return middleware.Func(func(next http.Handler) http.Handler { - return &prepop{ - next: next, - } - }) -} - -type prepop struct { - next http.Handler -} - -func (p *prepop) ServeHTTP(w http.ResponseWriter, req *http.Request) { - err := req.ParseForm() - if err != nil { - status := http.StatusBadRequest - http.Error( - w, - httpgrpc.Errorf(http.StatusBadRequest, err.Error()).Error(), - status, - ) - return - - } - p.next.ServeHTTP(w, req) + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + err := req.ParseForm() + if err != nil { + status := http.StatusBadRequest + http.Error( + w, + httpgrpc.Errorf(http.StatusBadRequest, err.Error()).Error(), + status, + ) + return + } + next.ServeHTTP(w, req) + }) + }) } // parseRegexQuery parses regex and query querystring from httpRequest and returns the combined LogQL query. diff --git a/pkg/querier/http_test.go b/pkg/querier/http_test.go index b20b7f63e7c8..70eae4525880 100644 --- a/pkg/querier/http_test.go +++ b/pkg/querier/http_test.go @@ -12,9 +12,9 @@ import ( ) func TestPrepopulate(t *testing.T) { - success := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - w.Write([]byte("ok")) + _, err := w.Write([]byte("ok")) + require.Nil(t, err) }) for _, tc := range []struct {