support User-Agent change and deletion from -H, simplify UI for heade…

…rs (#649)

* support User-Agent change and deletion from -H, simplify UI for headers to be extra headers only. fixes #648

* move the X-Proxy-Agent setup to OnBehalfOfRequest and add workaround for go client feature of setting User-Agent when not present

* additional test for multi proxy. allow spaces in header value to differentiate between delete and emit empty for stdclient and User-Agent:. fixed debug output as well for the stdclient request.

* add coverage to deleting the ua

README.md

@@ -956,6 +956,8 @@ body:
 
 ```
 
+Note: if you do not want the default fortio User-Agent to be sent pass `-H user-agent:`. If you want to send a present yet empty User-Agent: header, pass `-H "user-agent: "` (ie only whitespace sends empty one, empty value doesn't send any).
+
 ### Report only UI
 
 If you have json files saved from running the full UI or downloaded, using the `-sync` option, from an amazon or google cloud storage bucket or from a peer fortio server (to synchronize from a peer fortio, use `http://`_peer_`:8080/data/index.tsv` as the sync URL). You can then serve just the reports:

fhttp/http_client.go

@@ -273,11 +273,22 @@ func (h *HTTPOptions) AddAndValidateExtraHeader(hdr string) error {
 		return fmt.Errorf("invalid extra header '%s', expecting Key: Value", hdr)
 	}
 	key := strings.TrimSpace(s[0])
-	value := strings.TrimSpace(s[1])
-	if strings.EqualFold(key, "host") {
+	// No TrimSpace for the value, so we can set empty "" vs just whitespace " " which
+	// will get trimmed later but treated differently: not emitted vs emitted empty for User-Agent.
+	value := s[1]
+	switch strings.ToLower(key) {
+	case "host":
 		log.LogVf("Will be setting special Host header to %s", value)
-		h.hostOverride = value
-	} else {
+		h.hostOverride = strings.TrimSpace(value) // This one needs to be trimmed
+	case "user-agent":
+		if value == "" {
+			log.Infof("Deleting default User-Agent: header.")
+			h.extraHeaders.Del(key)
+		} else {
+			log.Infof("User-Agent being Set to %q", value)
+			h.extraHeaders.Set(key, value)
+		}
+	default:
 		log.LogVf("Setting regular extra header %s: %s", key, value)
 		h.extraHeaders.Add(key, value)
 		log.Debugf("headers now %+v", h.extraHeaders)
@@ -333,6 +344,10 @@ func newHTTPRequest(o *HTTPOptions) (*http.Request, error) {
 	if o.hostOverride != "" {
 		req.Host = o.hostOverride
 	}
+	// Another workaround for std client otherwise trying to set a default User-Agent
+	if _, ok := req.Header["User-Agent"]; !ok {
+		req.Header.Set("User-Agent", "")
+	}
 	if !log.LogDebug() {
 		return req, nil
 	}
@@ -474,7 +489,6 @@ func NewStdClient(o *HTTPOptions) (*Client, error) {
 	if req == nil {
 		return nil, err
 	}
-
 	client := Client{
 		url:                  o.URL,
 		path:                 req.URL.Path,

fhttp/http_forwarder.go

@@ -72,7 +72,7 @@ func makeMirrorRequest(baseURL string, r *http.Request, data []byte) *http.Reque
 	return req
 }
 
-// CopyHeaders copies all or trace headers.
+// CopyHeaders copies all or trace headers from `r` into `req`.
 func CopyHeaders(req, r *http.Request, all bool) {
 	// Copy only trace headers unless all is true.
 	for k, v := range r.Header {
@@ -85,6 +85,11 @@ func CopyHeaders(req, r *http.Request, all bool) {
 			log.Debugf("Skipping header %q", k)
 		}
 	}
+	if _, ok := r.Header["User-Agent"]; !ok {
+		// explicitly disable User-Agent so it's not set
+		// to default value (go client lib 'feature' workaround)
+		req.Header.Set("User-Agent", "")
+	}
 }
 
 // MakeSimpleRequest makes a new request for url but copies trace headers from input request r.
@@ -97,9 +102,7 @@ func MakeSimpleRequest(url string, r *http.Request, copyAllHeaders bool) *http.R
 	}
 	// Copy only trace headers or all of them:
 	CopyHeaders(req, r, copyAllHeaders)
-	if copyAllHeaders {
-		req.Header.Add("X-Proxy-Agent", jrpc.UserAgent)
-	} else {
+	if !copyAllHeaders {
 		req.Header.Set(jrpc.UserAgentHeader, jrpc.UserAgent)
 	}
 	return req

fhttp/http_forwarder_test.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"net/http"
 	"testing"
+
+	"fortio.org/fortio/jrpc"
 )
 
 func TestMultiProxy(t *testing.T) {
@@ -27,12 +29,18 @@ func TestMultiProxy(t *testing.T) {
 	for i := 0; i < 2; i++ {
 		serial := (i == 0)
 		mcfg := MultiServerConfig{Serial: serial}
-		mcfg.Targets = []TargetConf{{Destination: urlBase, MirrorOrigin: true}, {Destination: urlBase + "echo?status=555"}}
+		mcfg.Targets = []TargetConf{
+			{Destination: urlBase, MirrorOrigin: true},
+			{Destination: urlBase + "debug", MirrorOrigin: false},
+			{Destination: urlBase + "echo?status=555"},
+		}
 		_, multiAddr := MultiServer("0", &mcfg)
 		url := fmt.Sprintf("http://%s/debug", multiAddr)
 		payload := "A test payload"
 		opts := HTTPOptions{URL: url, Payload: []byte(payload)}
+		opts.AddAndValidateExtraHeader("User-agent:")
 		opts.AddAndValidateExtraHeader("b3: traceid...")
+		opts.AddAndValidateExtraHeader("X-FA: bar") // so it comes just before X-Fortio-Multi-Id
 		code, data := Fetch(&opts)
 		if serial && code != http.StatusOK {
 			t.Errorf("Got %d %s instead of ok in serial mode (first response sets code) for %s", code, DebugSummary(data, 256), url)
@@ -41,18 +49,49 @@ func TestMultiProxy(t *testing.T) {
 			t.Errorf("Got %d %s instead of 555 in parallel mode (non ok response sets code) for %s", code, DebugSummary(data, 256), url)
 		}
 		if !bytes.Contains(data, []byte(payload)) {
-			t.Errorf("Result %s doesn't contain expected payload echo back %q", DebugSummary(data, 1024), payload)
+			t.Errorf("Missing expected payload %q in %s", payload, DebugSummary(data, 1024))
+		}
+		searchFor := "B3: traceid..."
+		if !bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Missing expected trace header %q in %s", searchFor, DebugSummary(data, 1024))
+		}
+		searchFor = "\nX-Fa: bar\nX-Fortio-Multi-Id: 1\n"
+		if !bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Missing expected general header %q in 1st req %s", searchFor, DebugSummary(data, 1024))
+		}
+		searchFor = "\nX-Fa: bar\nX-Fortio-Multi-Id: 2\n"
+		if bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Unexpected non trace header %q in 2nd req %s", searchFor, DebugSummary(data, 1024))
 		}
 		// Issue #624
 		if bytes.Contains(data, []byte("gzip")) {
-			t.Errorf("Result %s contains unexpected gzip (accept encoding)", DebugSummary(data, 1024))
+			t.Errorf("Unexpected gzip (accept encoding)in %s", DebugSummary(data, 1024))
+		}
+		searchFor = "X-Fortio-Multi-Id: 1"
+		if !bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Missing expected %q in %s", searchFor, DebugSummary(data, 1024))
+		}
+		// Second request should be found
+		searchFor = "X-Fortio-Multi-Id: 2"
+		if !bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Missing expected %q in %s", searchFor, DebugSummary(data, 1024))
+		}
+		// Third request errors 100% so shouldn't be found
+		searchFor = "X-Fortio-Multi-Id: 3"
+		if bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Unexpected %q in %s", searchFor, DebugSummary(data, 1024))
+		}
+		searchFor = "\nX-Proxy-Agent: " + jrpc.UserAgent + "\n"
+		if !bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Missing %q in %s", searchFor, DebugSummary(data, 2048))
 		}
-		if !bytes.Contains(data, []byte("X-Fortio-Multi-Id: 1")) {
-			t.Errorf("Result %s doesn't contain expected X-Fortio-Multi-Id: 1", DebugSummary(data, 1024))
+		searchFor = "\nUser-Agent: " + jrpc.UserAgent + "\nX-Fortio-Multi-Id: 2\n"
+		if !bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Missing %q in %s", searchFor, DebugSummary(data, 2048))
 		}
-		// Second request errors 100% so shouldn't be found
-		if bytes.Contains(data, []byte("X-Fortio-Multi-Id: 2")) {
-			t.Errorf("Result %s contains unexpected X-Fortio-Multi-Id: 2", DebugSummary(data, 1024))
+		searchFor = "\nUser-Agent: " + jrpc.UserAgent + "\nX-Fortio-Multi-Id: 1\n"
+		if bytes.Contains(data, []byte(searchFor)) {
+			t.Errorf("Unexpected %q in %s", searchFor, DebugSummary(data, 2048))
 		}
 	}
 }

fhttp/http_test.go

@@ -67,13 +67,22 @@ func TestGetHeaders(t *testing.T) {
 	if err == nil {
 		t.Errorf("Expected error for header without value, did not get one")
 	}
-	o.ResetHeaders()
+	o.InitHeaders()
 	h = o.AllHeaders()
 	if h.Get("Host") != "" {
 		t.Errorf("After reset Host header should be nil, got '%v'", h.Get("Host"))
 	}
+	if len(h) != 1 {
+		t.Errorf("Header count mismatch after reset, got %d instead of 1. %+v", len(h), h)
+	}
+	// test user-agent delete:
+	o.AddAndValidateExtraHeader("UsER-AgENT:")
+	h = o.AllHeaders()
 	if len(h) != 0 {
-		t.Errorf("Header count mismatch after reset, got %d instead of 1", len(h))
+		t.Errorf("Header count mismatch after delete, got %d instead of 0. %+v", len(h), h)
+	}
+	if h.Get("User-Agent") != "" {
+		t.Errorf("User-Agent header should be empty after delete, got '%v'", h.Get("User-Agent"))
 	}
 }
 
@@ -115,7 +124,7 @@ func TestMultiInitAndEscape(t *testing.T) {
 	if o.URL != expected {
 		t.Errorf("Got initially '%s', expected '%s'", o.URL, expected)
 	}
-	o.AddAndValidateExtraHeader("FoO: BaR")
+	o.AddAndValidateExtraHeader("FoO:BaR")
 	// re init should not erase headers
 	o.Init(o.URL)
 	if o.AllHeaders().Get("Foo") != "BaR" {
@@ -1120,6 +1129,11 @@ func TestDebugHandlerSortedHeaders(t *testing.T) {
 	o.AddAndValidateExtraHeader("CCC: ccc")
 	o.AddAndValidateExtraHeader("ZZZ: zzz")
 	o.AddAndValidateExtraHeader("AAA: aaa")
+	// test that headers usually Add (list) but stay in order of being set
+	o.AddAndValidateExtraHeader("BBB: aa2")
+	// test that User-Agent is special, only last value is kept - and replaces the default jrpc.UserAgent
+	o.AddAndValidateExtraHeader("User-Agent: ua1")
+	o.AddAndValidateExtraHeader("User-Agent: ua2")
 	client, _ := NewClient(&o)
 	now := time.Now()
 	code, data, header := client.Fetch() // used to panic/bug #127
@@ -1140,13 +1154,13 @@ func TestDebugHandlerSortedHeaders(t *testing.T) {
 		"Host: localhost:%d\n"+
 		"Aaa: aaa\n"+
 		"Accept-Encoding: gzip\n"+
-		"Bbb: bbb\n"+
+		"Bbb: bbb,aa2\n"+
 		"Ccc: ccc\n"+
 		"Content-Length: 4\n"+
 		"Content-Type: application/octet-stream\n"+
-		"User-Agent: %s\n"+
+		"User-Agent: ua2\n"+
 		"Zzz: zzz\n\n"+
-		"body:\n\nabcd\n", a.Port, jrpc.UserAgent)
+		"body:\n\nabcd\n", a.Port)
 	if body != expected {
 		t.Errorf("Get body: %s not as expected: %s", body, expected)
 	}

fhttp/http_utils.go

@@ -33,6 +33,7 @@ import (
 
 	"fortio.org/fortio/dflag"
 	"fortio.org/fortio/fnet"
+	"fortio.org/fortio/jrpc"
 	"fortio.org/fortio/log"
 	"fortio.org/fortio/stats"
 )
@@ -572,9 +573,14 @@ func OnBehalfOf(o *HTTPOptions, r *http.Request) {
 	_ = o.AddAndValidateExtraHeader("X-On-Behalf-Of: " + r.RemoteAddr)
 }
 
-// OnBehalfOfRequest same as OnBehalfOf but places the header directly on the dst request object.
+// OnBehalfOfRequest same as OnBehalfOf but places the header directly on the dst request object
+// but also adds a X-Proxy-Agent header if the user-agent isn't already the same as this running
+// server's version.
 func OnBehalfOfRequest(to *http.Request, from *http.Request) {
 	to.Header.Add("X-On-Behalf-Of", from.RemoteAddr)
+	if to.Header.Get("User-Agent") != jrpc.UserAgent {
+		to.Header.Add("X-Proxy-Agent", jrpc.UserAgent)
+	}
 }
 
 // AddHTTPS replaces "http://" in url with "https://" or prepends "https://"

rapi/restHandler.go

@@ -266,7 +266,6 @@ func RESTRunHandler(w http.ResponseWriter, r *http.Request) { //nolint:funlen
 	httpopts := &fhttp.HTTPOptions{}
 	httpopts.HTTPReqTimeOut = timeout // to be normalized in init 0 replaced by default value
 	httpopts = httpopts.Init(url)
-	httpopts.ResetHeaders()
 	httpopts.DisableFastClient = stdClient
 	httpopts.SequentialWarmup = sequentialWarmup
 	httpopts.Insecure = httpsInsecure

ui/templates/main.html

@@ -53,12 +53,7 @@ <h1>Φορτίο (fortio) v{{.Version}}{{if not .DoLoad}} control UI{{end}}</h1>
     No Catch-Up (qps is a ceiling): <input type="checkbox" name="nocatchup" /><br />
     Percentiles: <input type="text" name="p" size="20" value="50, 75, 90, 99, 99.9" /> <br />
     Histogram Resolution: <input type="text" name="r" size="8" value="0.0001" /> <br />
-    Headers: <br />
-  {{ range $name, $vals := .Headers }}{{range $val := $vals}}
-    <input type="text" name="H" size=40 value="{{$name}}: {{ $val }}" /> <br />
-  {{end}}{{end}} <!-- 3 extra header lines, TODO(#283): add a JS 'more headers' button -->
-    <input type="text" name="H" size=40 value="" /> <br />
-    <input type="text" name="H" size=40 value="" /> <br />
+    Extra Headers:<br />
     <input type="text" name="H" size=40 value="" /> <br />
     <button type="button" onclick="addCustomHeader()">+</button>
     <br />

ui/uihandler.go

@@ -185,8 +185,6 @@ func Handler(w http.ResponseWriter, r *http.Request) {
 	httpopts := &fhttp.HTTPOptions{}
 	httpopts.HTTPReqTimeOut = timeout // to be normalized in init 0 replaced by default value
 	httpopts = httpopts.Init(url)
-	defaultHeaders := httpopts.AllHeaders()
-	httpopts.ResetHeaders()
 	httpopts.DisableFastClient = stdClient
 	httpopts.SequentialWarmup = sequentialWarmup
 	httpopts.Insecure = httpsInsecure
@@ -221,7 +219,6 @@ func Handler(w http.ResponseWriter, r *http.Request) {
 		}
 		err := mainTemplate.Execute(w, &struct {
 			R                           *http.Request
-			Headers                     http.Header
 			Version                     string
 			LogoPath                    string
 			DebugPath                   string
@@ -237,7 +234,7 @@ func Handler(w http.ResponseWriter, r *http.Request) {
 			DoStop                      bool
 			DoLoad                      bool
 		}{
-			r, defaultHeaders, version.Short(), logoPath, debugPath, echoPath, chartJSPath,
+			r, version.Short(), logoPath, debugPath, echoPath, chartJSPath,
 			startTime.Format(time.ANSIC), url, labels, runid,
 			fhttp.RoundDuration(time.Since(startTime)), durSeconds, urlHostPort, mode == stop, mode == run,
 		})