Skip to content

Commit

Permalink
application/x-www-form-urlencoded support (#1381)
Browse files Browse the repository at this point in the history
* application/x-www-form-urlencoded support

* handler func instead of unnecessary struct
  • Loading branch information
owen-d authored and cyriltovena committed Dec 7, 2019
1 parent 2da8d6f commit 5227f5b
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 8 deletions.
3 changes: 3 additions & 0 deletions pkg/loghttp/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions pkg/loghttp/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,40 +20,40 @@ 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
}
return uint32(l), nil
}

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
}
return start, end, nil
}

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
}
Expand All @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/loghttp/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions pkg/loghttp/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"time"

"github.com/grafana/loki/pkg/logproto"
"github.com/stretchr/testify/require"
)

func TestParseRangeQuery(t *testing.T) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions pkg/loghttp/tail_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/grafana/loki/pkg/logproto"
"github.com/stretchr/testify/require"
)

func TestParseTailQuery(t *testing.T) {
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions pkg/loki/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,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))

Expand Down
22 changes: 22 additions & 0 deletions pkg/querier/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -217,6 +218,27 @@ 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 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.
// This is used only to keep regexp query string support until it gets fully deprecated.
func parseRegexQuery(httpRequest *http.Request) (string, error) {
Expand Down
106 changes: 106 additions & 0 deletions pkg/querier/http_test.go
Original file line number Diff line number Diff line change
@@ -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) {
_, err := w.Write([]byte("ok"))
require.Nil(t, err)
})

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)
}

})
}
}

0 comments on commit 5227f5b

Please sign in to comment.