Fix dead docs URLs in help text and superfluous WriteHeader in diagnostic handlers

cmd/cloudflared/main.go

@@ -50,7 +50,7 @@ var (
 
 func main() {
 	// FIXME: TUN-8148: Disable QUIC_GO ECN due to bugs in proper detection if supported
-	os.Setenv("QUIC_GO_DISABLE_ECN", "1")
+	_ = os.Setenv("QUIC_GO_DISABLE_ECN", "1")
 	metrics.RegisterBuildInfo(BuildType, BuildTime, Version)
 	_, _ = maxprocs.Set()
 	bInfo := cliutil.GetBuildInfo(BuildType, Version)
@@ -72,7 +72,7 @@ func main() {
 	app.Copyright = fmt.Sprintf(
 		`(c) %d Cloudflare Inc.
    Your installation of cloudflared software constitutes a symbol of your signature indicating that you accept
-   the terms of the Apache License Version 2.0 (https://developers.cloudflare.com/cloudflare-one/connections/connect-apps/license),
+   the terms of the Apache License Version 2.0 (https://github.com/cloudflare/cloudflared/blob/master/LICENSE),
    Terms (https://www.cloudflare.com/terms/) and Privacy Policy (https://www.cloudflare.com/privacypolicy/).`,
 		time.Now().Year(),
 	)
@@ -97,7 +97,7 @@ func main() {
 }
 
 func commands(version func(c *cli.Context)) []*cli.Command {
-	cmds := []*cli.Command{
+	cmds := []*cli.Command{ // nolint:prealloc // one-time startup registration; keep the literal readable
 		{
 			Name:   "update",
 			Action: cliutil.ConfiguredAction(updater.Update),

cmd/cloudflared/tunnel/cmd.go

@@ -1094,7 +1094,7 @@ func legacyTunnelFlag(msg string) string {
 	return fmt.Sprintf(
 		"%s This flag only takes effect if you define your origin with `--url` and if you do not use ingress rules."+
 			" The recommended way is to rely on ingress rules and define this property under `originRequest` as per"+
-			" https://developers.cloudflare.com/cloudflare-one/connections/connect-apps/configuration/configuration-file/ingress",
+			" https://developers.cloudflare.com/cloudflare-one/networks/connectors/cloudflare-tunnel/do-more-with-tunnels/local-management/configuration-file/",
 		msg,
 	)
 }

diagnostic/handlers.go

@@ -81,12 +81,15 @@ func (handler *Handler) SystemHandler(writer http.ResponseWriter, request *http.
 		Err:  err,
 	}
 
-	encoder := json.NewEncoder(writer)
-	err = encoder.Encode(response)
+	body, err := json.Marshal(response)
 	if err != nil {
 		logger.Error().Err(err).Msgf("error occurred whilst serializing information")
 		writer.WriteHeader(http.StatusInternalServerError)
+
+		return
 	}
+
+	writeResponse(writer, body, &logger)
 }
 
 type TunnelState struct {
@@ -108,13 +111,15 @@ func (handler *Handler) TunnelStateHandler(writer http.ResponseWriter, _ *http.R
 		handler.tracker.GetActiveConnections(),
 		handler.icmpSources,
 	}
-	encoder := json.NewEncoder(writer)
-
-	err := encoder.Encode(body)
+	response, err := json.Marshal(body)
 	if err != nil {
-		handler.log.Error().Err(err).Msgf("error occurred whilst serializing information")
+		log.Error().Err(err).Msgf("error occurred whilst serializing information")
 		writer.WriteHeader(http.StatusInternalServerError)
+
+		return
 	}
+
+	writeResponse(writer, response, &log)
 }
 
 func (handler *Handler) ConfigurationHandler(writer http.ResponseWriter, _ *http.Request) {
@@ -125,13 +130,15 @@ func (handler *Handler) ConfigurationHandler(writer http.ResponseWriter, _ *http
 		log.Info().Msg("Collection finished")
 	}()
 
-	encoder := json.NewEncoder(writer)
-
-	err := encoder.Encode(handler.cliFlags)
+	body, err := json.Marshal(handler.cliFlags)
 	if err != nil {
-		handler.log.Error().Err(err).Msgf("error occurred whilst serializing response")
+		log.Error().Err(err).Msgf("error occurred whilst serializing response")
 		writer.WriteHeader(http.StatusInternalServerError)
+
+		return
 	}
+
+	writeResponse(writer, body, &log)
 }
 
 func writeResponse(w http.ResponseWriter, bytes []byte, logger *zerolog.Logger) {

diagnostic/handlers_test.go

@@ -25,11 +25,6 @@ type SystemCollectorMock struct {
 	err        error
 }
 
-const (
-	systemInformationKey = "sikey"
-	errorKey             = "errkey"
-)
-
 func newTrackerFromConns(t *testing.T, connections []tunnelstate.IndexedConnectionInfo) *tunnelstate.ConnTracker {
 	t.Helper()
 
@@ -167,6 +162,82 @@ func TestTunnelStateHandler(t *testing.T) {
 	}
 }
 
+type recordingFailingWriter struct {
+	header          http.Header
+	wroteBody       bool
+	headerAfterBody bool
+}
+
+func newRecordingFailingWriter() *recordingFailingWriter {
+	return &recordingFailingWriter{header: make(http.Header)}
+}
+
+func (writer *recordingFailingWriter) Header() http.Header { return writer.header }
+
+func (writer *recordingFailingWriter) Write([]byte) (int, error) {
+	writer.wroteBody = true
+	return 0, errors.New("write error")
+}
+
+func (writer *recordingFailingWriter) WriteHeader(int) {
+	if writer.wroteBody {
+		writer.headerAfterBody = true
+	}
+}
+
+func TestHandlersDoNotWriteHeaderAfterBody(t *testing.T) {
+	t.Parallel()
+
+	log := zerolog.Nop()
+	handler := diagnostic.NewDiagnosticHandler(&log, 0, &SystemCollectorMock{
+		systemInfo: nil,
+		err:        nil,
+	}, uuid.New(), uuid.New(), newTrackerFromConns(t, nil), map[string]string{}, nil)
+
+	tests := []struct {
+		name string
+		run  func(t *testing.T, writer http.ResponseWriter)
+	}{
+		{
+			name: "system handler",
+			run: func(t *testing.T, writer http.ResponseWriter) {
+				t.Helper()
+				ctx := context.Background()
+				request, err := http.NewRequestWithContext(ctx, http.MethodGet, "/diag/system", nil)
+				require.NoError(t, err)
+				handler.SystemHandler(writer, request)
+			},
+		},
+		{
+			name: "tunnel state handler",
+			run: func(t *testing.T, writer http.ResponseWriter) {
+				t.Helper()
+				handler.TunnelStateHandler(writer, nil)
+			},
+		},
+		{
+			name: "configuration handler",
+			run: func(t *testing.T, writer http.ResponseWriter) {
+				t.Helper()
+				handler.ConfigurationHandler(writer, nil)
+			},
+		},
+	}
+
+	for _, tCase := range tests {
+		t.Run(tCase.name, func(t *testing.T) {
+			t.Parallel()
+			writer := newRecordingFailingWriter()
+			tCase.run(t, writer)
+			assert.False(
+				t,
+				writer.headerAfterBody,
+				"handler must not call WriteHeader after the response body has been written",
+			)
+		})
+	}
+}
+
 func TestConfigurationHandler(t *testing.T) {
 	t.Parallel()