Repository: fortio/fortio (go)

Issue:
return non 0 status (shell error) for grpcping -health
fortio curl already returns 0 for 200 and 1 for non 200, let's do the same for grpc health SERVING

Pull request description:
exit with status 1 if grpcping -health isn't SERVING.
 fixes #588, #456, #595 (invalid utf8 when random payload is used)

testing through newly added _down extra service:
```
$ go run . grpcping -health -healthservice ping_down -n 10 localhost
RTT histogram usec : count 10 avg 1415.4499 +/- 3828 min 124.292 max 12899.791000000001 sum 14154.499
# range, mid point, percentile, count
>= 124.292 <= 140 , 132.146 , 70.00, 7
> 140 <= 160 , 150 , 80.00, 1
> 180 <= 200 , 190 , 90.00, 1
> 10000 <= 12899.8 , 11449.9 , 100.00, 1
# target 50% 134.764
Health NOT_SERVING : 10
11:18:17 E fortio_main.go:526> Unexpected SERVING count 0 vs 10
exit status 1
```

Patch:
diff --git a/README.md b/README.md
index 2d051a0..f12e7a0 100644
--- a/README.md
+++ b/README.md
@@ -51,13 +51,13 @@ You can install from source:
 The [releases](https://github.com/fortio/fortio/releases) page has binaries for many OS/architecture combinations (see assets).
 
 ```shell
-curl -L https://github.com/fortio/fortio/releases/download/v1.32.3/fortio-linux_amd64-1.32.3.tgz \
+curl -L https://github.com/fortio/fortio/releases/download/v1.33.0/fortio-linux_amd64-1.33.0.tgz \
  | sudo tar -C / -xvzpf -
 # or the debian package
-wget https://github.com/fortio/fortio/releases/download/v1.32.3/fortio_1.32.3_amd64.deb
-dpkg -i fortio_1.32.3_amd64.deb
+wget https://github.com/fortio/fortio/releases/download/v1.33.0/fortio_1.33.0_amd64.deb
+dpkg -i fortio_1.33.0_amd64.deb
 # or the rpm
-rpm -i https://github.com/fortio/fortio/releases/download/v1.32.3/fortio-1.32.3-1.x86_64.rpm
+rpm -i https://github.com/fortio/fortio/releases/download/v1.33.0/fortio-1.33.0-1.x86_64.rpm
 # and more, see assets in release page
 ```
 
@@ -67,7 +67,7 @@ On a MacOS you can also install Fortio using [Homebrew](https://brew.sh/):
 brew install fortio
 ```
 
-On Windows, download https://github.com/fortio/fortio/releases/download/v1.32.3/fortio_win_1.32.3.zip and extract `fortio.exe` to any location, then using the Windows Command Prompt:
+On Windows, download https://github.com/fortio/fortio/releases/download/v1.33.0/fortio_win_1.33.0.zip and extract `fortio.exe` to any location, then using the Windows Command Prompt:
 ```
 fortio.exe server
 ```
@@ -115,7 +115,7 @@ Full list of command line flags (`fortio help`):
 <details>
 <!-- use release/updateFlags.sh to update this section -->
 <pre>
-Φορτίο 1.32.3 usage:
+Φορτίο 1.33.0 usage:
 where command is one of: load (load testing), server (starts ui, http-echo,
  redirect, proxies, tcp-echo and grpc ping servers), tcp-echo (only the tcp-echo
  server), report (report only UI server), redirect (only the redirect server),
diff --git a/fgrpc/pingsrv.go b/fgrpc/pingsrv.go
index cff169f..0fbe36a 100644
--- a/fgrpc/pingsrv.go
+++ b/fgrpc/pingsrv.go
@@ -82,6 +82,7 @@ func PingServer(port, cert, key, healthServiceName string, maxConcurrentStreams
 	reflection.Register(grpcServer)
 	healthServer := health.NewServer()
 	healthServer.SetServingStatus(healthServiceName, grpc_health_v1.HealthCheckResponse_SERVING)
+	healthServer.SetServingStatus(healthServiceName+"_down", grpc_health_v1.HealthCheckResponse_NOT_SERVING)
 	grpc_health_v1.RegisterHealthServer(grpcServer, healthServer)
 	RegisterPingServerServer(grpcServer, &pingSrv{})
 	go func() {
diff --git a/fhttp/http_client.go b/fhttp/http_client.go
index 4f17e25..893b392 100644
--- a/fhttp/http_client.go
+++ b/fhttp/http_client.go
@@ -201,13 +201,25 @@ func (h *HTTPOptions) InitHeaders() {
 	// before command line option -H are parsed/set.
 }
 
-// PayloadString returns the payload as a string. If payload is null return empty string
+// PayloadUTF8 returns the payload as a string. If payload is null return empty string
 // This is only needed due to grpc ping proto. It takes string instead of byte array.
-func (h *HTTPOptions) PayloadString() string {
-	if len(h.Payload) == 0 {
+func (h *HTTPOptions) PayloadUTF8() string {
+	p := h.Payload
+	pl := len(p)
+	if pl == 0 {
 		return ""
 	}
-	return string(h.Payload)
+	// grpc doesn't like invalid utf-8 strings, get rid of them
+	res := strings.ToValidUTF8(string(p), "")
+	l := len([]byte(res))
+	if l < pl {
+		// but then keep the expected bytes length (though it'll compressed unlike the original)
+		pad := pl - l
+		res += strings.Repeat("X", pad)
+		log.Infof("Padded payload with %d extra Xs to make valid UTF-8 after filtering invalid sequences", pad)
+		log.Debugf("Payload now %d bytes", len(res))
+	}
+	return res
 }
 
 // ValidateAndAddBasicAuthentication validates user credentials and adds basic authentication to http header,
@@ -301,7 +313,7 @@ type Client struct {
 	url                  string
 	path                 string // original path of the request's url
 	rawQuery             string // original query params
-	body                 string // original body of the request
+	body                 []byte // original body of the request
 	req                  *http.Request
 	client               *http.Client
 	transport            *http.Transport
@@ -356,15 +368,15 @@ func (c *Client) Fetch() (int, []byte, int) {
 		c.req.URL.RawQuery = rawQuery
 	}
 	if c.bodyContainsUUID {
-		body := c.body
+		body := string(c.body)
 		for strings.Contains(body, uuidToken) {
 			body = strings.Replace(body, uuidToken, generateUUID(), 1)
 		}
 		bodyBytes := []byte(body)
 		c.req.ContentLength = int64(len(bodyBytes))
 		c.req.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes))
-	} else if c.body != "" {
-		c.req.Body = ioutil.NopCloser(bytes.NewReader([]byte(c.body)))
+	} else if len(c.body) > 0 {
+		c.req.Body = ioutil.NopCloser(bytes.NewReader(c.body))
 	}
 
 	resp, err := c.client.Do(c.req)
@@ -430,8 +442,8 @@ func NewStdClient(o *HTTPOptions) (*Client, error) {
 		pathContainsUUID:     strings.Contains(req.URL.Path, uuidToken),
 		rawQuery:             req.URL.RawQuery,
 		rawQueryContainsUUID: strings.Contains(req.URL.RawQuery, uuidToken),
-		body:                 o.PayloadString(),
-		bodyContainsUUID:     strings.Contains(o.PayloadString(), uuidToken),
+		body:                 o.Payload,
+		bodyContainsUUID:     strings.Contains(string(o.Payload), uuidToken),
 		req:                  req,
 		client: &http.Client{
 			Timeout: o.HTTPReqTimeOut,
@@ -571,14 +583,15 @@ func NewFastClient(o *HTTPOptions) (Fetcher, error) {
 		uuidStrings = append(uuidStrings, uuidString)
 		urlString = strings.Replace(urlString, uuidToken, uuidString, 1)
 	}
-	payload := o.PayloadString()
+	payload := string(o.Payload)
 	for strings.Contains(payload, uuidToken) {
 		uuidString := generateUUID()
 		uuidStrings = append(uuidStrings, uuidString)
 		payload = strings.Replace(payload, uuidToken, uuidString, 1)
 	}
-	o.Payload = []byte(payload)
-
+	if len(uuidStrings) > 0 {
+		o.Payload = []byte(payload)
+	}
 	// Parse the url, extract components.
 	url, err := url.Parse(urlString)
 	if err != nil {
diff --git a/fortio_main.go b/fortio_main.go
index 0a0f203..afe15cf 100644
--- a/fortio_main.go
+++ b/fortio_main.go
@@ -426,7 +426,7 @@ func fortioLoad(justCurl bool, percList []float64) {
 			Service:            *healthSvcFlag,
 			Streams:            *streamsFlag,
 			AllowInitialErrors: *allowInitialErrorsFlag,
-			Payload:            httpOpts.PayloadString(),
+			Payload:            httpOpts.PayloadUTF8(),
 			Delay:              *pingDelayFlag,
 			UsePing:            *doPingLoadFlag,
 		}
@@ -516,12 +516,19 @@ func grpcClient() {
 		count = 1
 	}
 	httpOpts := bincommon.SharedHTTPOptions()
-	var err error
 	if *doHealthFlag {
-		_, err = fgrpc.GrpcHealthCheck(host, *healthSvcFlag, count, &httpOpts.TLSOptions)
-	} else {
-		_, err = fgrpc.PingClientCall(host, count, httpOpts.PayloadString(), *pingDelayFlag, &httpOpts.TLSOptions)
+		status, err := fgrpc.GrpcHealthCheck(host, *healthSvcFlag, count, &httpOpts.TLSOptions)
+		if err != nil {
+			// already logged
+			os.Exit(1)
+		}
+		if (*status)["SERVING"] != int64(count) {
+			log.Errf("Unexpected SERVING count %d vs %d", (*status)["SERVING"], count)
+			os.Exit(1)
+		}
+		return
 	}
+	_, err := fgrpc.PingClientCall(host, count, httpOpts.PayloadUTF8(), *pingDelayFlag, &httpOpts.TLSOptions)
 	if err != nil {
 		// already logged
 		os.Exit(1)


Test patch:
diff --git a/Webtest.sh b/Webtest.sh
index 979d181..6a6067d 100755
--- a/Webtest.sh
+++ b/Webtest.sh
@@ -106,14 +106,19 @@ docker exec $DOCKERNAME $FORTIO_BIN_PATH load -H Foo:Bar -H Blah:Blah -qps 1 -t
 # Do a grpcping
 docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping localhost
 # Do a grpcping to a scheme-prefixed destination. Fortio should append port number
-# re-enable once we get https://grpc.fortio.org:/ fully working with https too
-# docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping https://grpc.fortio.org
+# Do a TLS grpcping. Fortio.org should use valid cert.
+docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping https://grpc.fortio.org
 docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping grpc.fortio.org # uses default non tls 8079
-# Do a grpcping with -cert flag. Fortio should use valid cert.
-# docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping -cacert $CERT grpc.fortio.org:443
-# docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping -cacert $CERT https://grpc.fortio.org
 # Do a local grpcping. Fortio should append default grpc port number to destination
 docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping localhost
+# Do a local health ping
+docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping -health localhost
+docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping -health -healthservice ping localhost
+# Do a failing on purpose check
+if docker exec $DOCKERNAME $FORTIO_BIN_PATH grpcping -health -healthservice ping_down localhost; then
+  echo "*** Expecting grpcping -health to ping_down have exit with error/non zero status"
+  exit 1
+fi
 # pprof should be there, no 404/error
 PPROF_URL="$BASE_URL/debug/pprof/heap?debug=1"
 $CURL "$PPROF_URL" | grep -i TotalAlloc # should find this in memory profile
diff --git a/fgrpc/pingsrv_test.go b/fgrpc/pingsrv_test.go
index d1ca138..6d0dce5 100644
--- a/fgrpc/pingsrv_test.go
+++ b/fgrpc/pingsrv_test.go
@@ -88,6 +88,9 @@ func TestPingServer(t *testing.T) {
 	if r, err := GrpcHealthCheck(sAddr, "foo", 3, TLSSecure); err != nil || (*r)[serving] != 3 {
 		t.Errorf("Unexpected result %+v, %v with health check for same service as started (foo)", r, err)
 	}
+	if r, err := GrpcHealthCheck(sAddr, "foo_down", 3, TLSSecure); err != nil || (*r)["NOT_SERVING"] != 3 {
+		t.Errorf("Unexpected result %+v, %v with health check for _down variant of same service as started (foo/foo_down)", r, err)
+	}
 	if r, err := GrpcHealthCheck(iAddr, "willfail", 1, TLSInsecure); err == nil || r != nil {
 		t.Errorf("Was expecting error when using unknown service, didn't get one, got %+v", r)
 	}
diff --git a/fhttp/http_test.go b/fhttp/http_test.go
index e861353..7a5c0c1 100644
--- a/fhttp/http_test.go
+++ b/fhttp/http_test.go
@@ -27,6 +27,7 @@ import (
 	"strings"
 	"testing"
 	"time"
+	"unicode/utf8"
 
 	"fortio.org/fortio/fnet"
 	"fortio.org/fortio/log"
@@ -93,6 +94,18 @@ func TestNewHTTPRequest(t *testing.T) {
 	}
 }
 
+func TestPayloadUTF8(t *testing.T) {
+	opts := HTTPOptions{}
+	opts.Payload = fnet.GenerateRandomPayload(1024)
+	res := opts.PayloadUTF8()
+	if l := len(res); l != 1024 {
+		t.Errorf("Unexpected length %d", l)
+	}
+	if !utf8.ValidString(res) {
+		t.Errorf("PayloadUTF8() not returning valid utf-8 string")
+	}
+}
+
 func TestMultiInitAndEscape(t *testing.T) {
 	// 2 escaped already
 	o := NewHTTPOptions("localhost%3A8080/?delay=10ms:10,0.5s:15%25,0.25s:5")