From 280b78e45d30c8dc3f74b6cd3f378d8ead1bc99a Mon Sep 17 00:00:00 2001
From: Chris Goller <goller@gmail.com>
Date: Tue, 10 Sep 2019 09:23:54 -0500
Subject: [PATCH] feat(http): add validation checks to PUT requests
It was possible to create checks with invalid data causing
odd internal errors to return from the layers far down below.
Likely, we need to add more checks to the validation, but, I have
not thoroughly checked.
---
http/check_service.go | 47 +++++++++++++++++++++++++++----------------
http/check_test.go | 16 ++++++++++-----
2 files changed, 41 insertions(+), 22 deletions(-)
diff --git a/http/check_service.go b/http/check_service.go
index 8e62534505..a22a48370a 100644
--- a/http/check_service.go
+++ b/http/check_service.go
@@ -342,22 +342,6 @@ func decodePostCheckRequest(ctx context.Context, r *http.Request) (influxdb.Chec
}
func decodePutCheckRequest(ctx context.Context, r *http.Request) (influxdb.Check, error) {
- buf := new(bytes.Buffer)
- _, err := buf.ReadFrom(r.Body)
- if err != nil {
- return nil, &influxdb.Error{
- Code: influxdb.EInvalid,
- Err: err,
- }
- }
- defer r.Body.Close()
- chk, err := check.UnmarshalJSON(buf.Bytes())
- if err != nil {
- return nil, &influxdb.Error{
- Code: influxdb.EInvalid,
- Err: err,
- }
- }
params := httprouter.ParamsFromContext(ctx)
id := params.ByName("id")
if id == "" {
@@ -366,11 +350,40 @@ func decodePutCheckRequest(ctx context.Context, r *http.Request) (influxdb.Check
Msg: "url missing id",
}
}
+
i := new(influxdb.ID)
if err := i.DecodeFromString(id); err != nil {
- return nil, err
+ return nil, &influxdb.Error{
+ Code: influxdb.EInvalid,
+ Msg: "invalid check id format",
+ }
+ }
+
+ defer r.Body.Close()
+ buf := new(bytes.Buffer)
+ _, err := buf.ReadFrom(r.Body)
+ if err != nil {
+ return nil, &influxdb.Error{
+ Code: influxdb.EInvalid,
+ Msg: "unable to read HTTP body",
+ Err: err,
+ }
+ }
+
+ chk, err := check.UnmarshalJSON(buf.Bytes())
+ if err != nil {
+ return nil, &influxdb.Error{
+ Code: influxdb.EInvalid,
+ Msg: "malformed check body",
+ Err: err,
+ }
}
chk.SetID(*i)
+
+ if err := chk.Valid(); err != nil {
+ return nil, err
+ }
+
return chk, nil
}
diff --git a/http/check_test.go b/http/check_test.go
index 5fc722cf16..b6a23c0fa4 100644
--- a/http/check_test.go
+++ b/http/check_test.go
@@ -1077,9 +1077,11 @@ func TestService_handleUpdateCheck(t *testing.T) {
id: "020f755c3c082000",
chk: &check.Deadman{
Base: check.Base{
- Name: "example",
- Status: influxdb.Active,
- TaskID: 3,
+ Name: "example",
+ Status: influxdb.Active,
+ TaskID: 3,
+ OwnerID: 42,
+ OrgID: influxTesting.MustIDBase16("020f755c3c082000"),
},
Level: notification.Critical,
},
@@ -1099,6 +1101,7 @@ func TestService_handleUpdateCheck(t *testing.T) {
"updatedAt": "0001-01-01T00:00:00Z",
"id": "020f755c3c082000",
"orgID": "020f755c3c082000",
+ "ownerID": "000000000000002a",
"level": "CRIT",
"name": "example",
"query": {
@@ -1140,7 +1143,10 @@ func TestService_handleUpdateCheck(t *testing.T) {
id: "020f755c3c082000",
chk: &check.Deadman{
Base: check.Base{
- Name: "example",
+ Name: "example",
+ Status: influxdb.Active,
+ OwnerID: 42,
+ OrgID: influxTesting.MustIDBase16("020f755c3c082000"),
},
},
},
@@ -1183,7 +1189,7 @@ func TestService_handleUpdateCheck(t *testing.T) {
body, _ := ioutil.ReadAll(res.Body)
if res.StatusCode != tt.wants.statusCode {
- t.Errorf("%q. handlePutCheck() = %v, want %v %v", tt.name, res.StatusCode, tt.wants.statusCode, w.Header())
+ t.Errorf("%q. handlePutCheck() = %v, want %v %v %v", tt.name, res.StatusCode, tt.wants.statusCode, w.Header(), string(body))
}
if tt.wants.contentType != "" && content != tt.wants.contentType {
t.Errorf("%q. handlePutCheck() = %v, want %v", tt.name, content, tt.wants.contentType)