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 1 commit
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
Next Next commit
application/x-www-form-urlencoded support
  • Loading branch information
owen-d committed Dec 6, 2019
commit 86100419c788d5f10b8bb1ea5ea98265206d6784
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
31 changes: 31 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,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 {
owen-d marked this conversation as resolved.
Show resolved Hide resolved
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) {
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) {
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)
}

})
}
}