feat(render,find): strip private info from returned errors info

cmd/carbonapi/http/find_handlers.go

@@ -23,6 +23,7 @@ import (
 	"github.com/go-graphite/carbonapi/date"
 	"github.com/go-graphite/carbonapi/intervalset"
 	utilctx "github.com/go-graphite/carbonapi/util/ctx"
+	"github.com/go-graphite/carbonapi/zipper/helper"
 )
 
 // Find handler and it's helper functions
@@ -283,7 +284,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 			if returnCode < 300 {
 				multiGlobs = &pbv3.MultiGlobResponse{Metrics: []pbv3.GlobResponse{}}
 			} else {
-				setError(w, &accessLogDetails, http.StatusText(returnCode), returnCode, uid.String())
+				setError(w, &accessLogDetails, helper.MerryRootError(err), returnCode, uid.String())
 				// We don't want to log this as an error if it's something normal
 				// Normal is everything that is >= 500. So if config.Config.NotFoundStatusCode is 500 - this will be
 				// logged as error

cmd/carbonapi/http/helper.go

@@ -2,6 +2,7 @@ package http
 
 import (
 	"fmt"
+	"html"
 	"net/http"
 	"strings"
 	"time"
@@ -229,6 +230,27 @@ func splitRemoteAddr(addr string) (string, string) {
 	return tmp[0], tmp[1]
 }
 
+func stripKey(key string, n int) string {
+	if len(key) > n+3 {
+		key = key[:n/2] + "..." + key[n/2+1:]
+	}
+	return key
+}
+
+// stripError for strip network errors (ip and other private info)
+func stripError(err string) string {
+	if strings.Contains(err, "connection refused") {
+		return "connection refused"
+	} else if strings.Contains(err, " lookup ") {
+		return "lookup error"
+	} else if strings.Contains(err, "broken pipe") {
+		return "broken pipe"
+	} else if strings.Contains(err, " connection reset ") {
+		return "connection reset"
+	}
+	return html.EscapeString(err)
+}
+
 func buildParseErrorString(target, e string, err error) string {
 	msg := fmt.Sprintf("%s\n\n%-20s: %s\n", http.StatusText(http.StatusBadRequest), "Target", target)
 	if err != nil {
@@ -285,9 +307,56 @@ func timestampTruncate(ts int64, duration time.Duration, durations []config.Dura
 
 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)
+	if msg == "" {
+		msg = http.StatusText(status)
+	}
 	accessLogDetails.Reason = msg
 	accessLogDetails.HTTPCode = int32(status)
+	msg = html.EscapeString(stripError(msg))
+	http.Error(w, msg, status)
+}
+
+func joinErrors(errMap map[string]string, sep string, status int) (msg, reason string) {
+	if len(errMap) == 0 {
+		msg = http.StatusText(status)
+	} else {
+		var buf, rBuf strings.Builder
+		buf.Grow(512)
+		rBuf.Grow(512)
+
+		// map is unsorted, can produce flapping ordered output, not critical
+		for k, err := range errMap {
+			if buf.Len() > 0 {
+				buf.WriteString(sep)
+				rBuf.WriteString(sep)
+			}
+			buf.WriteString(html.EscapeString(stripKey(k, 128)))
+			rBuf.WriteString(k)
+			buf.WriteString(": ")
+			rBuf.WriteString(": ")
+			buf.WriteString(html.EscapeString(stripError(err)))
+			rBuf.WriteString(err)
+		}
+
+		msg = buf.String()
+		reason = rBuf.String()
+	}
+	return
+}
+
+func setErrors(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, errMamp map[string]string, status int, carbonapiUUID string) {
+	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
+	var msg string
+	if status != http.StatusOK {
+		if len(errMamp) == 0 {
+			msg = http.StatusText(status)
+			accessLogDetails.Reason = msg
+		} else {
+			msg, accessLogDetails.Reason = joinErrors(errMamp, "\n", status)
+		}
+	}
+	accessLogDetails.HTTPCode = int32(status)
+	http.Error(w, msg, status)
 }
 
 func queryLengthLimitExceeded(query []string, maxLength uint64) bool {

cmd/carbonapi/http/render_handler.go

@@ -353,16 +353,16 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
 		// Obtain error code from the errors
 		// In case we have only "Not Found" errors, result should be 404
 		// Otherwise it should be 500
-		var errMsgs []string
+		var errMsgs map[string]string
 		returnCode, errMsgs = helper.MergeHttpErrorMap(errors)
-		logger.Debug("error response or no response", zap.Strings("error", errMsgs))
+		logger.Debug("error response or no response", zap.Any("error", errMsgs))
 		// Allow override status code for 404-not-found replies.
 		if returnCode == http.StatusNotFound {
 			returnCode = config.Config.NotFoundStatusCode
 		}
 
 		if returnCode == http.StatusBadRequest || returnCode == http.StatusNotFound || returnCode == http.StatusForbidden || returnCode >= 500 {
-			setError(w, accessLogDetails, strings.Join(errMsgs, ","), returnCode, uid.String())
+			setErrors(w, accessLogDetails, errMsgs, returnCode, uid.String())
 			logAsError = true
 			return
 		}

cmd/carbonapi/http/tags_handler.go

@@ -21,9 +21,10 @@ import (
 func tagHandler(w http.ResponseWriter, r *http.Request) {
 	t0 := time.Now()
 	uuid := uuid.NewV4()
+	carbonapiUUID := uuid.String()
 
 	// TODO: Migrate to context.WithTimeout
-	ctx := utilctx.SetUUID(r.Context(), uuid.String())
+	ctx := utilctx.SetUUID(r.Context(), carbonapiUUID)
 	requestHeaders := utilctx.GetLogHeaders(ctx)
 	username, _, _ := r.BasicAuth()
 
@@ -39,7 +40,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	var accessLogDetails = &carbonapipb.AccessLogDetails{
 		Handler:        "tags",
 		Username:       username,
-		CarbonapiUUID:  uuid.String(),
+		CarbonapiUUID:  carbonapiUUID,
 		URL:            r.URL.Path,
 		PeerIP:         srcIP,
 		PeerPort:       srcPort,
@@ -81,7 +82,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	rawQuery := q.Encode()
 
 	if queryLengthLimitExceeded(r.Form["query"], config.Config.MaxQueryLength) {
-		setError(w, accessLogDetails, "query length limit exceeded", http.StatusBadRequest, uuid.String())
+		setError(w, accessLogDetails, "query length limit exceeded", http.StatusBadRequest, carbonapiUUID)
 		logAsError = true
 		return
 	}
@@ -123,7 +124,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("Content-Type", contentTypeJSON)
-	w.Header().Set(ctxHeaderUUID, uuid.String())
+	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
 	_, _ = w.Write(b)
 	accessLogDetails.Runtime = time.Since(t0).Seconds()
 	accessLogDetails.HTTPCode = http.StatusOK

cmd/mockbackend/e2etesting.go

@@ -13,6 +13,7 @@ import (
 	"net/url"
 	"os"
 	"reflect"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -45,6 +46,8 @@ type Query struct {
 type ExpectedResponse struct {
 	HttpCode        int              `yaml:"httpCode"`
 	ContentType     string           `yaml:"contentType"`
+	ErrBody         string           `yaml:"errBody"`
+	ErrSort         bool             `yaml:"errSort"`
 	ExpectedResults []ExpectedResult `yaml:"expectedResults"`
 }
 
@@ -177,6 +180,20 @@ func max(a, b int) int {
 	return b
 }
 
+func resortErr(errStr string) string {
+	first := strings.Index(errStr, "\n")
+	if first >= 0 && first != len(errStr)-1 {
+		// resort error string
+		errs := strings.Split(errStr, "\n")
+		if errs[len(errs)-1] == "" {
+			errs = errs[:len(errs)-1]
+		}
+		sort.Strings(errs)
+		errStr = strings.Join(errs, "\n") + "\n"
+	}
+	return errStr
+}
+
 func doTest(logger *zap.Logger, t *Query, verbose bool) []error {
 	client := http.Client{}
 	failures := make([]error, 0)
@@ -245,8 +262,17 @@ func doTest(logger *zap.Logger, t *Query, verbose bool) []error {
 		)
 	}
 
-	// We don't need to actually check body of response if we expect any sort of error (4xx/5xx)
+	// We don't need to actually check body of response if we expect any sort of error (4xx/5xx), but for check error handling do this
 	if t.ExpectedResponse.HttpCode >= 300 {
+		if t.ExpectedResponse.ErrBody != "" {
+			errStr := string(b)
+			if t.ExpectedResponse.ErrSort {
+				errStr = resortErr(errStr)
+			}
+			if t.ExpectedResponse.ErrBody != errStr {
+				failures = append(failures, merry2.Errorf("mismatch error body, got '%s', expected '%s'", string(b), t.ExpectedResponse.ErrBody))
+			}
+		}
 		return failures
 	}
 

cmd/mockbackend/e2etesting_test.go

@@ -0,0 +1,37 @@
+package main
+
+import (
+	"strconv"
+	"testing"
+)
+
+func Test_resortErr(t *testing.T) {
+	tests := []struct {
+		errStr string
+		want   string
+	}{
+		{
+			errStr: "b: connection refused\na: connection refused\n",
+			want:   "a: connection refused\nb: connection refused\n",
+		},
+		{
+			errStr: "a: connection refused\nb: connection refused\n",
+			want:   "a: connection refused\nb: connection refused\n",
+		},
+		{
+			errStr: "",
+			want:   "",
+		},
+		{
+			errStr: "\n",
+			want:   "\n",
+		},
+	}
+	for i, tt := range tests {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			if got := resortErr(tt.errStr); got != tt.want {
+				t.Errorf("resortErr(%q) = %q, want %q", tt.errStr, got, tt.want)
+			}
+		})
+	}
+}

cmd/mockbackend/testcases/connection_refused/carbonapi.yaml

@@ -0,0 +1,57 @@
+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: 0
+graphite:
+    host: ""
+    interval: "60s"
+    prefix: "carbon.api"
+    pattern: "{prefix}.{fqdn}"
+idleConnections: 10
+pidFile: ""
+upstreams:
+    buckets: 10
+    timeouts:
+        find: "2s"
+        render: "5s"
+        connect: "200ms"
+    concurrencyLimitPerServer: 0
+    keepAliveInterval: "30s"
+    maxIdleConnsPerHost: 100
+    backendsv2:
+        backends:
+          -
+            groupName: "mock-001"
+            protocol: "auto"
+            lbMethod: "all"
+            maxTries: 1
+            maxBatchSize: 0
+            keepAliveInterval: "10s"
+            concurrencyLimit: 0
+            forceAttemptHTTP2: true
+            maxIdleConnsPerHost: 1000
+            timeouts:
+                find: "3s"
+                render: "5s"
+                connect: "200ms"
+            servers:
+                - "http://127.0.0.1:9071"
+graphite09compat: false
+expireDelaySec: 10
+logger:
+    - logger: ""
+      file: "stderr"
+      level: "debug"
+      encoding: "console"
+      encodingTime: "iso8601"
+      encodingDuration: "seconds"

cmd/mockbackend/testcases/connection_refused/connection_refused.yaml

@@ -0,0 +1,72 @@
+version: "v1"
+test:
+    apps:
+        - name: "carbonapi"
+          binary: "./carbonapi"
+          args:
+              - "-config"
+              - "./cmd/mockbackend/testcases/connection_refused/carbonapi.yaml"
+              - "-exact-config"
+    queries:
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/render/?target=a&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "a: connection refused\n"
+              errSort: true
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/render/?target=a&target=b&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "a: connection refused\nb: connection refused\n"
+              errSort: true
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find/?query=a&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "connection refused\n"
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find/?query=a&query=b&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "connection refused\n"
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag4"
+          expectedResponse:
+              httpCode: 200
+              contentType: "application/json"
+              expectedResults:
+                  - tagsAutocompelete: []
+            # TODO: query must fail
+            #   httpCode: 503
+            #   contentType: "text/plain; charset=utf-8"
+            #   errBody: "connection refused\n"
+            #   errSort: true
+
+listeners:
+      - address: ":9070"
+        expressions:
+            "a":
+                pathExpression: "a"
+                data:
+                    - metricName: "a"
+                      values: [0,1,2,2,3]
+
+            # timeout
+            "c":
+                pathExpression: "c"
+                code: 404
+                replyDelayMS: 7000
+
+            "d":
+                pathExpression: "d"
+                code: 503