Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

application/x-www-form-urlencoded support #1381

Merged
merged 2 commits into from
Dec 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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))

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

})
}
}