From 4d9937f852269d1bbe5cba00b890b43ae10637ee Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sun, 6 Nov 2022 14:37:30 -0800 Subject: [PATCH 1/6] support User-Agent change and deletion from -H, simplify UI for headers to be extra headers only. fixes #648 --- fhttp/http_client.go | 13 +++++++++++-- fhttp/http_test.go | 24 +++++++++++++++++++----- rapi/restHandler.go | 1 - ui/templates/main.html | 7 +------ ui/uihandler.go | 5 +---- 5 files changed, 32 insertions(+), 18 deletions(-) diff --git a/fhttp/http_client.go b/fhttp/http_client.go index 321998f1b..23d3c9701 100644 --- a/fhttp/http_client.go +++ b/fhttp/http_client.go @@ -274,10 +274,19 @@ func (h *HTTPOptions) AddAndValidateExtraHeader(hdr string) error { } key := strings.TrimSpace(s[0]) value := strings.TrimSpace(s[1]) - if strings.EqualFold(key, "host") { + switch strings.ToLower(key) { + case "host": log.LogVf("Will be setting special Host header to %s", value) h.hostOverride = value - } else { + case "user-agent": + if value == "" { + log.Infof("User-Agent: header removed.") + h.extraHeaders.Del(key) + } else { + log.LogVf("User-Agent being Set to %s", 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) diff --git a/fhttp/http_test.go b/fhttp/http_test.go index a9d309138..74d746467 100644 --- a/fhttp/http_test.go +++ b/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")) } } @@ -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) } diff --git a/rapi/restHandler.go b/rapi/restHandler.go index 4fe6f5258..8affeb851 100644 --- a/rapi/restHandler.go +++ b/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 diff --git a/ui/templates/main.html b/ui/templates/main.html index 59c4f2161..7413e48b6 100644 --- a/ui/templates/main.html +++ b/ui/templates/main.html @@ -53,12 +53,7 @@

Φορτίο (fortio) v{{.Version}}{{if not .DoLoad}} control UI{{end}}

No Catch-Up (qps is a ceiling):
Percentiles:
Histogram Resolution:
- Headers:
- {{ range $name, $vals := .Headers }}{{range $val := $vals}} -
- {{end}}{{end}} -
-
+ Extra Headers:


diff --git a/ui/uihandler.go b/ui/uihandler.go index c4a45ca54..b37a3b12e 100644 --- a/ui/uihandler.go +++ b/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, }) From 3d9e5c2b37b6a0cea0f655895a8e50b2cfc1dd15 Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sun, 6 Nov 2022 16:23:04 -0800 Subject: [PATCH 2/6] move the X-Proxy-Agent setup to OnBehalfOfRequest and add workaround for go client feature of setting User-Agent when not present --- fhttp/http_forwarder.go | 11 +++++++---- fhttp/http_utils.go | 8 +++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/fhttp/http_forwarder.go b/fhttp/http_forwarder.go index 666893655..378bdb479 100644 --- a/fhttp/http_forwarder.go +++ b/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 diff --git a/fhttp/http_utils.go b/fhttp/http_utils.go index 40a05b646..c3cc894e8 100644 --- a/fhttp/http_utils.go +++ b/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://" From 302fe8db003dec517d4d0ad7b5fd0ae1a14c87ad Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sun, 6 Nov 2022 18:40:40 -0800 Subject: [PATCH 3/6] 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. --- README.md | 2 ++ fhttp/http_client.go | 13 ++++++++---- fhttp/http_forwarder_test.go | 38 ++++++++++++++++++++++++++++-------- 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 1be4e23d6..50b6d8642 100644 --- a/README.md +++ b/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 an empty User-Agent: header, pass `-H "user-agent: "` (ie only whitespace sends empty one, empty value doesn't send any). You can only send an empty User-Agent using the FastClient as the standard one removes empty User-Agent (but not other empty headers...). + ### 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: diff --git a/fhttp/http_client.go b/fhttp/http_client.go index 23d3c9701..338887258 100644 --- a/fhttp/http_client.go +++ b/fhttp/http_client.go @@ -273,17 +273,19 @@ 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]) + // 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 case "user-agent": if value == "" { - log.Infof("User-Agent: header removed.") + log.Infof("Deleting default User-Agent: header.") h.extraHeaders.Del(key) } else { - log.LogVf("User-Agent being Set to %s", value) + log.Infof("User-Agent being Set to %q", value) h.extraHeaders.Set(key, value) } default: @@ -342,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 } @@ -483,7 +489,6 @@ func NewStdClient(o *HTTPOptions) (*Client, error) { if req == nil { return nil, err } - client := Client{ url: o.URL, path: req.URL.Path, diff --git a/fhttp/http_forwarder_test.go b/fhttp/http_forwarder_test.go index 09dbe461d..17dc97a62 100644 --- a/fhttp/http_forwarder_test.go +++ b/fhttp/http_forwarder_test.go @@ -27,12 +27,15 @@ 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("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 +44,37 @@ 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)) } - 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)) + // 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)) } - // 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)) + // 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)) } } } From 2a7581d512c7883c385ed76a5178c673bcb28e5b Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sun, 6 Nov 2022 18:45:30 -0800 Subject: [PATCH 4/6] make linter happy --- fhttp/http_forwarder_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fhttp/http_forwarder_test.go b/fhttp/http_forwarder_test.go index 17dc97a62..7f5871754 100644 --- a/fhttp/http_forwarder_test.go +++ b/fhttp/http_forwarder_test.go @@ -27,9 +27,11 @@ 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}, + mcfg.Targets = []TargetConf{ + {Destination: urlBase, MirrorOrigin: true}, {Destination: urlBase + "debug", MirrorOrigin: false}, - {Destination: urlBase + "echo?status=555"}} + {Destination: urlBase + "echo?status=555"}, + } _, multiAddr := MultiServer("0", &mcfg) url := fmt.Sprintf("http://%s/debug", multiAddr) payload := "A test payload" From 7fb2192759432857230e834dd416ac3b06064dea Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sun, 6 Nov 2022 19:01:51 -0800 Subject: [PATCH 5/6] glad I have good tests --- fhttp/http_client.go | 2 +- fhttp/http_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fhttp/http_client.go b/fhttp/http_client.go index 338887258..6ec79a7b1 100644 --- a/fhttp/http_client.go +++ b/fhttp/http_client.go @@ -279,7 +279,7 @@ func (h *HTTPOptions) AddAndValidateExtraHeader(hdr string) error { switch strings.ToLower(key) { case "host": log.LogVf("Will be setting special Host header to %s", value) - h.hostOverride = value + h.hostOverride = strings.TrimSpace(value) // This one needs to be trimmed case "user-agent": if value == "" { log.Infof("Deleting default User-Agent: header.") diff --git a/fhttp/http_test.go b/fhttp/http_test.go index 74d746467..e5efa3fa2 100644 --- a/fhttp/http_test.go +++ b/fhttp/http_test.go @@ -124,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" { From 79019454496d6cbc551f1c9d2ef9b4978f482c5a Mon Sep 17 00:00:00 2001 From: Laurent Demailly Date: Sun, 6 Nov 2022 19:56:44 -0800 Subject: [PATCH 6/6] add coverage to delete the ua --- README.md | 2 +- fhttp/http_forwarder_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 50b6d8642..580003c76 100644 --- a/README.md +++ b/README.md @@ -956,7 +956,7 @@ body: ``` -Note: if you do not want the default fortio User-Agent to be sent pass `-H user-agent:`. If you want to send an empty User-Agent: header, pass `-H "user-agent: "` (ie only whitespace sends empty one, empty value doesn't send any). You can only send an empty User-Agent using the FastClient as the standard one removes empty User-Agent (but not other empty headers...). +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 diff --git a/fhttp/http_forwarder_test.go b/fhttp/http_forwarder_test.go index 7f5871754..3ea888be2 100644 --- a/fhttp/http_forwarder_test.go +++ b/fhttp/http_forwarder_test.go @@ -19,6 +19,8 @@ import ( "fmt" "net/http" "testing" + + "fortio.org/fortio/jrpc" ) func TestMultiProxy(t *testing.T) { @@ -36,6 +38,7 @@ func TestMultiProxy(t *testing.T) { 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) @@ -78,6 +81,18 @@ func TestMultiProxy(t *testing.T) { 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)) + } + 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)) + } + 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)) + } } }