From feebd7007c5cecff347e50785a06d540eef48ec7 Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Fri, 8 Oct 2021 20:11:25 -0300 Subject: [PATCH 1/4] Change how push API checks for contentType - Currently, the ContentType is expected to be exactly "application/json". That's a problem because although a "application/json; charset=utf-8" is considered a valid JSON content type, it will be seen as an error. --- pkg/loghttp/push/push.go | 7 +++---- pkg/loghttp/push/push_test.go | 7 +++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/loghttp/push/push.go b/pkg/loghttp/push/push.go index dcd0eceb6493d..bf5b0c032b7d6 100644 --- a/pkg/loghttp/push/push.go +++ b/pkg/loghttp/push/push.go @@ -6,6 +6,7 @@ import ( "io" "math" "net/http" + "strings" "time" "github.com/cortexproject/cortex/pkg/util" @@ -78,8 +79,7 @@ func ParseRequest(logger log.Logger, userID string, r *http.Request, tenantsRete req logproto.PushRequest ) - switch contentType { - case applicationJSON: + if strings.Contains(contentType, applicationJSON) { var err error // todo once https://github.com/weaveworks/common/commit/73225442af7da93ec8f6a6e2f7c8aafaee3f8840 is in Loki. @@ -93,8 +93,7 @@ func ParseRequest(logger log.Logger, userID string, r *http.Request, tenantsRete if err != nil { return nil, err } - - default: + } else { // When no content-type header is set or when it is set to // `application/x-protobuf`: expect snappy compression. if err := util.ParseProtoReader(r.Context(), body, int(r.ContentLength), math.MaxInt32, &req, util.RawSnappy); err != nil { diff --git a/pkg/loghttp/push/push_test.go b/pkg/loghttp/push/push_test.go index 60e7ef67e88aa..2d3f460a4e935 100644 --- a/pkg/loghttp/push/push_test.go +++ b/pkg/loghttp/push/push_test.go @@ -72,6 +72,13 @@ func TestParseRequest(t *testing.T) { contentEncoding: `snappy`, valid: false, }, + { + path: `/loki/api/v1/push`, + body: gzipString(`{"streams": [{ "stream": { "foo": "bar2" }, "values": [ [ "1570818238000000000", "fizzbuzz" ] ] }]}`), + contentType: `application/json; charset=utf-8`, + contentEncoding: `gzip`, + valid: true, + }, } // Testing input array From 579defdcb70be95343d97f391f5c9715b4af3729 Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Sat, 9 Oct 2021 08:06:17 -0300 Subject: [PATCH 2/4] Use mime.ParseMediaType to parse contentType. - mime.ParseMediaType is more reliable than strings.Contains because if a wrong contentType (such as "application/jsonnn") is given, we still accept it - mime.ParseMediaType has support for contentType parameters. This way, valid contentTypes such as "application/json; charset=utf-8" will not be rejected --- pkg/loghttp/push/push.go | 14 +++++++++++--- pkg/loghttp/push/push_test.go | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/pkg/loghttp/push/push.go b/pkg/loghttp/push/push.go index bf5b0c032b7d6..fba001e26d6ba 100644 --- a/pkg/loghttp/push/push.go +++ b/pkg/loghttp/push/push.go @@ -5,8 +5,8 @@ import ( "fmt" "io" "math" + "mime" "net/http" - "strings" "time" "github.com/cortexproject/cortex/pkg/util" @@ -79,7 +79,14 @@ func ParseRequest(logger log.Logger, userID string, r *http.Request, tenantsRete req logproto.PushRequest ) - if strings.Contains(contentType, applicationJSON) { + contentType, _ /* params */, err := mime.ParseMediaType(contentType) + if err != nil { + return nil, err + } + + switch contentType { + case applicationJSON: + var err error // todo once https://github.com/weaveworks/common/commit/73225442af7da93ec8f6a6e2f7c8aafaee3f8840 is in Loki. @@ -93,7 +100,8 @@ func ParseRequest(logger log.Logger, userID string, r *http.Request, tenantsRete if err != nil { return nil, err } - } else { + + default: // When no content-type header is set or when it is set to // `application/x-protobuf`: expect snappy compression. if err := util.ParseProtoReader(r.Context(), body, int(r.ContentLength), math.MaxInt32, &req, util.RawSnappy); err != nil { diff --git a/pkg/loghttp/push/push_test.go b/pkg/loghttp/push/push_test.go index 2d3f460a4e935..ea8363635a998 100644 --- a/pkg/loghttp/push/push_test.go +++ b/pkg/loghttp/push/push_test.go @@ -79,6 +79,20 @@ func TestParseRequest(t *testing.T) { contentEncoding: `gzip`, valid: true, }, + { + path: `/loki/api/v1/push`, + body: gzipString(`{"streams": [{ "stream": { "foo": "bar2" }, "values": [ [ "1570818238000000000", "fizzbuzz" ] ] }]}`), + contentType: `application/jsonn; charset=utf-8`, + contentEncoding: `gzip`, + valid: false, + }, + { + path: `/loki/api/v1/push`, + body: gzipString(`{"streams": [{ "stream": { "foo": "bar2" }, "values": [ [ "1570818238000000000", "fizzbuzz" ] ] }]}`), + contentType: `application/json; charsetutf-8`, + contentEncoding: `gzip`, + valid: false, + }, } // Testing input array From c9daa9302969490b4d5011f2aae0b49b4001e21b Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Sat, 9 Oct 2021 08:10:42 -0300 Subject: [PATCH 3/4] Add changelog entry. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20ae757029b6a..880107c65ff21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Main * [4400](https://github.com/grafana/loki/pull/4400) **trevorwhitney**: Config: automatically apply memberlist config too all rings when provided +* [4443](https://github.com/grafana/loki/pull/4443) **DylanGuedes**: Loki: Change how push API checks for contentType # 2.3.0 (2021/08/06) From 3264bf63cc712d497fe8a09a30ceee790e3a8377 Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Sun, 10 Oct 2021 15:48:01 -0300 Subject: [PATCH 4/4] Change body in test to avoid lint issues. - Without this change, the lint fails because gzipString would be called with always the same input. Full error: ``` `gzipString` - `source` always receives ``{"streams": [{ "stream": { "foo": "bar2" }, "values": [ [ "1570818238000000000", "fizzbuzz" ] ] }]}` ("{\"streams\": [{ \"stream\": { \"foo\": \"bar2\" }, \"values\": [ [ ...) ` (unparam) ``` --- pkg/loghttp/push/push_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/loghttp/push/push_test.go b/pkg/loghttp/push/push_test.go index ea8363635a998..c0970bd450d4b 100644 --- a/pkg/loghttp/push/push_test.go +++ b/pkg/loghttp/push/push_test.go @@ -88,7 +88,7 @@ func TestParseRequest(t *testing.T) { }, { path: `/loki/api/v1/push`, - body: gzipString(`{"streams": [{ "stream": { "foo": "bar2" }, "values": [ [ "1570818238000000000", "fizzbuzz" ] ] }]}`), + body: gzipString(`{"streams": [{ "stream": { "foo4": "bar2" }, "values": [ [ "1570818238000000000", "fizzbuzz" ] ] }]}`), contentType: `application/json; charsetutf-8`, contentEncoding: `gzip`, valid: false,