diff --git a/cmd/carbonapi/config/config.go b/cmd/carbonapi/config/config.go index 122665b46..39019e3c6 100644 --- a/cmd/carbonapi/config/config.go +++ b/cmd/carbonapi/config/config.go @@ -107,7 +107,8 @@ type ConfigType struct { TruncateTimeMap map[time.Duration]time.Duration `mapstructure:"truncateTime"` TruncateTime []DurationTruncate `mapstructure:"-" json:"-"` // produce from TruncateTimeMap and sort in reverse order - CombineMultipleTargetsInOne bool `mapstructure:"combineMultipleTargetsInOne"` + MaxQueryLength uint64 `mapstructure:"maxQueryLength"` + CombineMultipleTargetsInOne bool `mapstructure:"combineMultipleTargetsInOne"` ResponseCache cache.BytesCache `mapstructure:"-" json:"-"` BackendCache cache.BytesCache `mapstructure:"-" json:"-"` diff --git a/cmd/carbonapi/http/expand_handler.go b/cmd/carbonapi/http/expand_handler.go index df09abe62..89e048e6c 100644 --- a/cmd/carbonapi/http/expand_handler.go +++ b/cmd/carbonapi/http/expand_handler.go @@ -76,6 +76,12 @@ func expandHandler(w http.ResponseWriter, r *http.Request) { return } + if queryLengthLimitExceeded(query, config.Config.MaxQueryLength) { + setError(w, &accessLogDetails, "query length limit exceeded", http.StatusBadRequest, uid.String()) + logAsError = true + return + } + var pv3Request pbv3.MultiGlobRequest pv3Request.Metrics = query pv3Request.StartTime = from64 diff --git a/cmd/carbonapi/http/find_handlers.go b/cmd/carbonapi/http/find_handlers.go index e4141bf4c..7b7349326 100644 --- a/cmd/carbonapi/http/find_handlers.go +++ b/cmd/carbonapi/http/find_handlers.go @@ -221,6 +221,12 @@ func findHandler(w http.ResponseWriter, r *http.Request) { return } + if queryLengthLimitExceeded(query, config.Config.MaxQueryLength) { + setError(w, &accessLogDetails, "query length limit exceeded", http.StatusBadRequest, uid.String()) + logAsError = true + return + } + if format == completerFormat { var replacer = strings.NewReplacer("/", ".") for i := range query { diff --git a/cmd/carbonapi/http/helper.go b/cmd/carbonapi/http/helper.go index 8d239b216..d35f5a56b 100644 --- a/cmd/carbonapi/http/helper.go +++ b/cmd/carbonapi/http/helper.go @@ -282,3 +282,23 @@ func timestampTruncate(ts int64, duration time.Duration, durations []config.Dura } return ts } + +func setError(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, msg string, status int, carbonapiUUID string) { + w.Header().Set(ctxHeaderUUID, carbonapiUUID) + http.Error(w, http.StatusText(status)+": "+msg, status) + accessLogDetails.Reason = msg + accessLogDetails.HTTPCode = int32(status) +} + +func queryLengthLimitExceeded(query []string, maxLength uint64) bool { + if maxLength > 0 { + var queryLengthSum uint64 = 0 + for _, q := range query { + queryLengthSum += uint64(len(q)) + } + if queryLengthSum > maxLength { + return true + } + } + return false +} diff --git a/cmd/carbonapi/http/helper_test.go b/cmd/carbonapi/http/helper_test.go index 0f1342b44..1d2261f09 100644 --- a/cmd/carbonapi/http/helper_test.go +++ b/cmd/carbonapi/http/helper_test.go @@ -2,6 +2,7 @@ package http import ( "fmt" + "strings" "testing" "time" @@ -68,3 +69,55 @@ func Test_timestampTruncate(t *testing.T) { }) } } + +func Test_queryLengthLimitExceeded(t *testing.T) { + tests := []struct { + query []string + maxLength uint64 + want bool + }{ + { + query: []string{"a.b.c.d.e", "a.a.a.a.a.a.a.a.a.a.a.a"}, + maxLength: 20, + want: true, + }, + { + query: []string{"a.b.c", "a.b.d"}, + maxLength: 10, + want: false, + }, + { + query: []string{"a.b.c", "a.b.d"}, + maxLength: 9, + want: true, + }, + { + query: []string{"a.b.c.d.e", "a.a.a.a.a.a.a.a.a.a.a.a"}, + maxLength: 0, + want: false, + }, + { + query: []string{"a.b.b.c.*", "a.d.e", "a.b.c", "a.b.e", "a.a.a.b", "a.f.s.x.w.g"}, + maxLength: 30, + want: true, + }, + { + query: []string{"a.b.c.d.e", "a.b.c.d.f", "b.a.*"}, + maxLength: 10000, + want: false, + }, + { + query: []string{}, + maxLength: 0, + want: false, + }, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("queryLengthLimitExceeded([%s], %d) -> %t", strings.Join(tt.query, ", "), tt.maxLength, tt.want), func(t *testing.T) { + if got := queryLengthLimitExceeded(tt.query, tt.maxLength); got != tt.want { + t.Errorf("queryLengthLimitExceeded() = %t, want %t", got, tt.want) + } + }) + } +} diff --git a/cmd/carbonapi/http/render_handler.go b/cmd/carbonapi/http/render_handler.go index 56ae44738..c82ba1333 100644 --- a/cmd/carbonapi/http/render_handler.go +++ b/cmd/carbonapi/http/render_handler.go @@ -42,13 +42,6 @@ func cleanupParams(r *http.Request) { r.Form.Del("_t") // Used by jquery.graphite.js } -func setError(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, msg string, status int, carbonapiUUID string) { - w.Header().Set(ctxHeaderUUID, carbonapiUUID) - http.Error(w, http.StatusText(status)+": "+msg, status) - accessLogDetails.Reason = msg - accessLogDetails.HTTPCode = int32(status) -} - func getCacheTimeout(logger *zap.Logger, r *http.Request, now32, until32 int64, duration time.Duration, cacheConfig *config.CacheConfig) int32 { if tstr := r.FormValue("cacheTimeout"); tstr != "" { t, err := strconv.Atoi(tstr) @@ -232,6 +225,12 @@ func renderHandler(w http.ResponseWriter, r *http.Request) { } } + if queryLengthLimitExceeded(targets, config.Config.MaxQueryLength) { + setError(w, accessLogDetails, "total target length limit exceeded", http.StatusBadRequest, uid.String()) + logAsError = true + return + } + if useCache { tc := time.Now() response, err := config.Config.ResponseCache.Get(responseCacheKey) diff --git a/cmd/carbonapi/http/tags_handler.go b/cmd/carbonapi/http/tags_handler.go index 1fbcca3c3..2663f37d3 100644 --- a/cmd/carbonapi/http/tags_handler.go +++ b/cmd/carbonapi/http/tags_handler.go @@ -80,6 +80,12 @@ func tagHandler(w http.ResponseWriter, r *http.Request) { q.Del("pretty") rawQuery := q.Encode() + if queryLengthLimitExceeded(r.Form["query"], config.Config.MaxQueryLength) { + setError(w, accessLogDetails, "query length limit exceeded", http.StatusBadRequest, uuid.String()) + logAsError = true + return + } + // TODO(civil): Implement caching var res []string if strings.HasSuffix(r.URL.Path, "tags") || strings.HasSuffix(r.URL.Path, "tags/") { diff --git a/cmd/mockbackend/testcases/pr817/carbonapi.yaml b/cmd/mockbackend/testcases/pr817/carbonapi.yaml new file mode 100644 index 000000000..368baaa66 --- /dev/null +++ b/cmd/mockbackend/testcases/pr817/carbonapi.yaml @@ -0,0 +1,61 @@ +listen: "localhost:8081" +expvar: + enabled: true + pprofEnabled: false + listen: "" +concurency: 1000 +notFoundStatusCode: 200 +cache: + type: "mem" + size_mb: 0 + defaultTimeoutSec: 60 +cpus: 0 +tz: "" +maxBatchSize: 500 +maxQueryLength: 20 +combineMultipleTargetsInOne: true +graphite: + host: "" + interval: "60s" + prefix: "carbon.api" + pattern: "{prefix}.{fqdn}" +idleConnections: 10 +pidFile: "" +upstreams: + buckets: 10 + timeouts: + find: "2s" + render: "10s" + connect: "200ms" + concurrencyLimitPerServer: 100 + keepAliveInterval: "30s" + maxIdleConnsPerHost: 100 + doMultipleRequestsIfSplit: true + backendsv2: + backends: + - + groupName: "mock-001" + protocol: "auto" + lbMethod: "all" + maxTries: 3 + maxBatchSize: 500 + keepAliveInterval: "10s" + concurrencyLimit: 0 + forceAttemptHTTP2: true + maxIdleConnsPerHost: 1000 + doMultipleRequestsIfSplit: true + timeouts: + find: "15s" + render: "50s" + connect: "200ms" + servers: + - "http://127.0.0.1:9070" + graphite09compat: false +expireDelaySec: 10 +logger: + - logger: "" + file: "stderr" + level: "debug" + encoding: "console" + encodingTime: "iso8601" + encodingDuration: "seconds" diff --git a/cmd/mockbackend/testcases/pr817/pr817.yaml b/cmd/mockbackend/testcases/pr817/pr817.yaml new file mode 100644 index 000000000..0330930dc --- /dev/null +++ b/cmd/mockbackend/testcases/pr817/pr817.yaml @@ -0,0 +1,94 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/testcases/pr817/carbonapi.yaml" + queries: + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render?target=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.b&format=json" + expectedResponse: + httpCode: 400 + contentType: "text/plain; charset=utf-8" + emptyBody: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.*&format=json" + expectedResponse: + httpCode: 400 + contentType: "text/plain; charset=utf-8" + emptyBody: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/expand?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.b&format=json" + expectedResponse: + httpCode: 400 + contentType: "text/plain; charset=utf-8" + emptyBody: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/tags?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.b" + expectedResponse: + httpCode: 400 + contentType: "text/plain; charset=utf-8" + emptyBody: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?query=a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.a.b" + expectedResponse: + httpCode: 400 + contentType: "text/plain; charset=utf-8" + emptyBody: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/render/?target=a.b.c&target=a.b.d&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - metrics: + - target: "a.b.c" + datapoints: [[0,1],[1,2],[2,3],[2,4],[3,5]] + - target: "a.b.d" + datapoints: [[31,1],[10,2],[4,3],[7,4],[3,5]] + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find?query=a.b.*&format=json" + expectedResponse: + httpCode: 200 + contentType: "application/json" + # - endpoint: "http://127.0.0.1:8081" + # type: "GET" + # URL: "/metrics/expand?query=a.*&format=json" + # expectedResponse: + # httpCode: 200 + # contentType: "application/json" + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/tags?query=a.b.c" + expectedResponse: + httpCode: 200 + contentType: "application/json" + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?query=a.b.c" + expectedResponse: + httpCode: 200 + contentType: "application/json" + +listeners: + - address: ":9070" + expressions: + "a.b.c": + pathExpression: "a.b.c" + data: + - metricName: "a.b.c" + values: [0,1,2,2,3] + "a.b.d": + pathExpression: "a.b.d" + data: + - metricName: "a.b.d" + values: [31,10,4,7,3]