Skip to content

Commit

Permalink
Assert that push values tuples consist of string values (#5685)
Browse files Browse the repository at this point in the history
* Assert that push values tuples consist of string values

This fixes a bug in the parsing of push requests that could allowed users
sending non-string "log lines" from clients, such as full JSON objects.

The values were interpreted as strings, even though they weren't,
leading to unexpected log line transformation because the request did
not fail.

Fixes #5645

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

* Add changelog entry

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Co-authored-by: Cyril Tovena <cyril.tovena@gmail.com>
  • Loading branch information
chaudum and cyriltovena authored Mar 25, 2022
1 parent 5ecbbbe commit 6d7422a
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## Main
* [5685](https://github.com/grafana/loki/pull/5625) **chaudum** Fix bug in push request parser that allowed users to send arbitrary non-string data as "log line".
* [5707](https://github.com/grafana/loki/pull/5707) **franzwong** Promtail: Rename config name limit_config to limits_config.
* [5626](https://github.com/grafana/loki/pull/5626) **jeschkies** Support multi-tenant select logs and samples queries.
* [5622](https://github.com/grafana/loki/pull/5622) **chaudum**: Fix bug in query splitter that caused `interval` query parameter to be ignored and therefore returning more logs than expected.
Expand Down
7 changes: 6 additions & 1 deletion pkg/loghttp/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ func (e *Entry) UnmarshalJSON(data []byte) error {
i int
parseError error
)
_, err := jsonparser.ArrayEach(data, func(value []byte, _ jsonparser.ValueType, _ int, _ error) {
_, err := jsonparser.ArrayEach(data, func(value []byte, t jsonparser.ValueType, _ int, _ error) {
// assert that both items in array are of type string
if t != jsonparser.String {
parseError = jsonparser.MalformedStringError
return
}
switch i {
case 0: // timestamp
ts, err := jsonparser.ParseInt(value)
Expand Down
7 changes: 7 additions & 0 deletions pkg/loghttp/push/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ func TestParseRequest(t *testing.T) {
contentEncoding: ``,
valid: true,
},
{
path: `/loki/api/v1/push`,
body: `{"streams": [{ "stream": { "foo": "bar2" }, "values": [ [ "1570818238000000000", {"fizz": "buzz"} ] ] }]}`,
contentType: `application/json`,
contentEncoding: ``,
valid: false,
},
{
path: `/loki/api/v1/push`,
body: gzipString(`{"streams": [{ "stream": { "foo": "bar2" }, "values": [ [ "1570818238000000000", "fizzbuzz" ] ] }]}`),
Expand Down

0 comments on commit 6d7422a

Please sign in to comment.