add new -pprof on flag to explictly allow pprof, off by default. (#845)

* add new -pprof on flag to explictly allow pprof, off by default. fixes #844. also refactored UI config so httpoptions isn't called twice (and thus not logging twice in log verbose)

* fix the e2e test that checks that pprof works in server

* add -pprof to the right server

* ./release/bumpRelease.sh 1.60.2

README.md

@@ -1,4 +1,4 @@
-<!-- 1.60.1 -->
+<!-- 1.60.2 -->
 # Fortio
 
 [![Awesome Go](https://fortio.org/mentioned-badge.svg)](https://github.com/avelino/awesome-go#networking)
@@ -60,13 +60,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.60.1/fortio-linux_amd64-1.60.1.tgz \
+curl -L https://github.com/fortio/fortio/releases/download/v1.60.2/fortio-linux_amd64-1.60.2.tgz \
  | sudo tar -C / -xvzpf -
 # or the debian package
-wget https://github.com/fortio/fortio/releases/download/v1.60.1/fortio_1.60.1_amd64.deb
-dpkg -i fortio_1.60.1_amd64.deb
+wget https://github.com/fortio/fortio/releases/download/v1.60.2/fortio_1.60.2_amd64.deb
+dpkg -i fortio_1.60.2_amd64.deb
 # or the rpm
-rpm -i https://github.com/fortio/fortio/releases/download/v1.60.1/fortio-1.60.1-1.x86_64.rpm
+rpm -i https://github.com/fortio/fortio/releases/download/v1.60.2/fortio-1.60.2-1.x86_64.rpm
 # and more, see assets in release page
 ```
 
@@ -76,7 +76,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.60.1/fortio_win_1.60.1.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.60.2/fortio_win_1.60.2.zip and extract `fortio.exe` to any location, then using the Windows Command Prompt:
 ```
 fortio.exe server
 ```
@@ -128,7 +128,7 @@ Full list of command line flags (`fortio help`):
 <!-- use release/updateFlags.sh to update this section -->
 <pre>
 <!-- USAGE_START -->
-Φορτίο 1.60.1 usage:
+Φορτίο 1.60.2 usage:
         fortio command [flags] target
 where command is one of: load (load testing), server (starts ui, rest api,
  http-echo, redirect, proxies, tcp-echo, udp-echo and grpc ping servers),
@@ -305,6 +305,8 @@ fails to keep up temporarily
 than -maxpayloadsizekb. Setting this switches http to POST.
   -ping
         grpc load test: use ping instead of health
+  -pprof
+        Enable pprof http endpoint in the Web UI handler server
   -profile file
         write .cpu and .mem profiles to file
   -proxy-all-headers

Webtest.sh

@@ -44,7 +44,7 @@ if [[ $? == 124 || $? == 0 ]]; then
   exit 1
 fi
 
-DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --net host --name $DOCKERNAME fortio/fortio:webtest server -ui-path $FORTIO_UI_PREFIX -loglevel $LOGLEVEL -maxpayloadsizekb $MAXPAYLOAD -timeout=$TIMEOUT)
+DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --net host --name $DOCKERNAME fortio/fortio:webtest server -ui-path $FORTIO_UI_PREFIX -loglevel $LOGLEVEL -maxpayloadsizekb $MAXPAYLOAD -timeout=$TIMEOUT -pprof)
 function cleanup {
   set +e # errors are ok during cleanup
 #  docker logs "$DOCKERID" # uncomment to debug
@@ -157,7 +157,8 @@ docker exec $DOCKERSECNAME $FORTIO_BIN_PATH load -grpc -cacert $TEST_CERT_VOL/ca
 docker stop "$DOCKERID"
 docker rm $DOCKERNAME
 DOCKERNAME=fortio_report
-DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --name $DOCKERNAME fortio/fortio:webtest report -loglevel $LOGLEVEL)
+# Even with pprof the report mode won't have pprof endpoint
+DOCKERID=$(docker run -d --ulimit nofile=$FILE_LIMIT --name $DOCKERNAME fortio/fortio:webtest report -loglevel $LOGLEVEL -pprof)
 docker ps
 CURL="docker exec $DOCKERNAME $FORTIO_BIN_PATH curl -loglevel $LOGLEVEL"
 if $CURL "$PPROF_URL" ; then

cli/fortio_main.go

@@ -179,6 +179,7 @@ var (
 	accessLogFileFormat = flag.String("access-log-format", "json",
 		"`format` for access log. Supported values: [json, influx]")
 	calcQPS = flag.Bool("calc-qps", false, "Calculate the qps based on number of requests (-n) and duration (-t)")
+	pprofOn = flag.Bool("pprof", false, "Enable pprof http endpoint in the Web UI handler server")
 )
 
 // serverArgCheck always returns true after checking arguments length.
@@ -251,20 +252,31 @@ func FortioMain(hook bincommon.FortioHook) {
 		}
 	case "server":
 		isServer = serverArgCheck()
+		tlsOptions := &bincommon.SharedHTTPOptions().TLSOptions
 		if *tcpPortFlag != disabled {
 			fnet.TCPEchoServer("tcp-echo", *tcpPortFlag)
 		}
 		if *udpPortFlag != disabled {
 			fnet.UDPEchoServer("udp-echo", *udpPortFlag, *udpAsyncFlag)
 		}
 		if *grpcPortFlag != disabled {
-			fgrpc.PingServer(*grpcPortFlag, *healthSvcFlag, uint32(*maxStreamsFlag), &bincommon.SharedHTTPOptions().TLSOptions)
+			fgrpc.PingServer(*grpcPortFlag, *healthSvcFlag, uint32(*maxStreamsFlag), tlsOptions)
 		}
 		if *redirectFlag != disabled {
 			fhttp.RedirectToHTTPS(*redirectFlag)
 		}
 		if *echoPortFlag != disabled {
-			if !ui.Serve(hook, baseURL, *echoPortFlag, *echoDbgPathFlag, *uiPathFlag, *dataDirFlag, percList()) {
+			uiCfg := ui.ServerConfig{
+				BaseURL:        baseURL,
+				Port:           *echoPortFlag,
+				DebugPath:      *echoDbgPathFlag,
+				UIPath:         *uiPathFlag,
+				DataDir:        *dataDirFlag,
+				PProfOn:        *pprofOn,
+				PercentileList: percList(),
+				TLSOptions:     tlsOptions,
+			}
+			if !ui.Serve(hook, &uiCfg) {
 				os.Exit(1) // error already logged
 			}
 		}

fhttp/http_server.go

@@ -486,6 +486,7 @@ func ServeTCP(port, debugPath string) (*http.ServeMux, *net.TCPAddr) {
 
 // SetupPPROF add pprof to the mux (mirror the init() of http pprof).
 func SetupPPROF(mux *http.ServeMux) {
+	log.Warnf("pprof endpoints enabled on /debug/pprof/*")
 	mux.HandleFunc("/debug/pprof/", LogAndCall("pprof:index", pprof.Index))
 	mux.HandleFunc("/debug/pprof/cmdline", LogAndCall("pprof:cmdline", pprof.Cmdline))
 	mux.HandleFunc("/debug/pprof/profile", LogAndCall("pprof:profile", pprof.Profile))

ui/uihandler.go

@@ -644,29 +644,38 @@ func getMetricsPath(debugPath string) string {
 	return strings.TrimSuffix(debugPath, "/") + "/metrics"
 }
 
+type ServerConfig struct {
+	BaseURL, Port, DebugPath, UIPath, DataDir string
+	PProfOn                                   bool
+	PercentileList                            []float64
+	TLSOptions                                *fhttp.TLSOptions
+}
+
 // Serve starts the fhttp.Serve() plus the UI server on the given port
 // and paths (empty disables the feature). uiPath should end with /
 // (be a 'directory' path). Returns true if server is started successfully.
-func Serve(hook bincommon.FortioHook, baseurl, port, debugpath, uipath, datadir string, percentileList []float64) bool {
+func Serve(hook bincommon.FortioHook, cfg *ServerConfig) bool {
 	startTime = time.Now()
-	// Kinda ugly that we get most params past in but we get the tls stuff from flags directly,
-	// it avoids making an already too long list of string params longer. probably should make a FortioConfig struct.
-	mux, addr := fhttp.ServeTLS(port, debugpath, &bincommon.SharedHTTPOptions().TLSOptions)
+	mux, addr := fhttp.ServeTLS(cfg.Port, cfg.DebugPath, cfg.TLSOptions)
 	if addr == nil {
 		return false // Error already logged
 	}
-	if uipath == "" {
+	if cfg.UIPath == "" {
 		return true
 	}
-	fhttp.SetupPPROF(mux)
-	uiPath = uipath
+	if cfg.PProfOn {
+		fhttp.SetupPPROF(mux) // This now logs a warning as it's a potential risk
+	} else {
+		log.LogVf("Not serving pprof endpoint.")
+	}
+	uiPath = cfg.UIPath
 	if uiPath[len(uiPath)-1] != '/' {
 		log.Warnf("Adding missing trailing / to UI path '%s'", uiPath)
 		uiPath += "/"
 	}
-	debugPath = debugpath
-	echoPath = fhttp.EchoDebugPath(debugpath)
-	metricsPath = getMetricsPath(debugpath)
+	debugPath = cfg.DebugPath
+	echoPath = fhttp.EchoDebugPath(debugPath)
+	metricsPath = getMetricsPath(debugPath)
 	mux.HandleFunc(uiPath, Handler)
 	fetchPath = uiPath + fetchURI
 	// For backward compatibility with http:// only fetcher
@@ -676,8 +685,8 @@ func Serve(hook bincommon.FortioHook, baseurl, port, debugpath, uipath, datadir
 	fhttp.CheckConnectionClosedHeader = true // needed for proxy to avoid errors
 
 	// New REST apis (includes the data/ handler)
-	rapi.AddHandlers(hook, mux, baseurl, uiPath, datadir)
-	rapi.DefaultPercentileList = percentileList
+	rapi.AddHandlers(hook, mux, cfg.BaseURL, uiPath, cfg.DataDir)
+	rapi.DefaultPercentileList = cfg.PercentileList
 
 	logoPath = version.Short() + "/static/img/fortio-logo-gradient-no-bg.svg"
 	chartJSPath = version.Short() + "/static/js/Chart.min.js"
@@ -719,7 +728,7 @@ func Serve(hook bincommon.FortioHook, baseurl, port, debugpath, uipath, datadir
 		debugPath, echoPath, dflagsPath, metricsPath)
 	mux.HandleFunc(metricsPath, metrics.Exporter)
 
-	urlHostPort = fnet.NormalizeHostPort(port, addr)
+	urlHostPort = fnet.NormalizeHostPort(cfg.Port, addr)
 	uiMsg := "\t UI started - visit:\n\t\t"
 	if strings.Contains(urlHostPort, "-unix-socket=") {
 		uiMsg += fmt.Sprintf("fortio curl %s http://localhost%s", urlHostPort, uiPath)