Skip to content

Commit

Permalink
Merge branch 'main' into fix/render_eror_code
Browse files Browse the repository at this point in the history
  • Loading branch information
msaf1980 authored Jan 23, 2024
2 parents 48f14f2 + 6928eff commit bff3233
Show file tree
Hide file tree
Showing 9 changed files with 254 additions and 8 deletions.
3 changes: 2 additions & 1 deletion cmd/carbonapi/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,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:"-"`
Expand Down
6 changes: 6 additions & 0 deletions cmd/carbonapi/http/expand_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions cmd/carbonapi/http/find_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,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 {
Expand Down
20 changes: 20 additions & 0 deletions cmd/carbonapi/http/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
53 changes: 53 additions & 0 deletions cmd/carbonapi/http/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package http

import (
"fmt"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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)
}
})
}
}
13 changes: 6 additions & 7 deletions cmd/carbonapi/http/render_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions cmd/carbonapi/http/tags_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/") {
Expand Down
61 changes: 61 additions & 0 deletions cmd/mockbackend/testcases/pr817/carbonapi.yaml
Original file line number Diff line number Diff line change
@@ -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"
94 changes: 94 additions & 0 deletions cmd/mockbackend/testcases/pr817/pr817.yaml
Original file line number Diff line number Diff line change
@@ -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]

0 comments on commit bff3233

Please sign in to comment.