Merge branch 'master' into 2358-mobileconfig

CHANGELOG.md

@@ -25,12 +25,14 @@ and this project adheres to
 
 - Make the mobileconfig HTTP API more robust and predictable, add parameters and
   improve error response ([#2358]).
+- Improved HTTP requests handling and timeouts. ([#2343]).
 - Our snap package now uses the `core20` image as its base [#2306].
 - Various internal improvements ([#2271], [#2297]).
 
 [#2271]: https://github.com/AdguardTeam/AdGuardHome/issues/2271
 [#2297]: https://github.com/AdguardTeam/AdGuardHome/issues/2297
 [#2306]: https://github.com/AdguardTeam/AdGuardHome/issues/2306
+[#2343]: https://github.com/AdguardTeam/AdGuardHome/issues/2343
 [#2358]: https://github.com/AdguardTeam/AdGuardHome/issues/2358
 
 ### Fixed

internal/home/auth.go

@@ -360,7 +360,7 @@ func handleLogout(w http.ResponseWriter, r *http.Request) {
 
 // RegisterAuthHandlers - register handlers
 func RegisterAuthHandlers() {
-	http.Handle("/control/login", postInstallHandler(ensureHandler("POST", handleLogin)))
+	Context.mux.Handle("/control/login", postInstallHandler(ensureHandler("POST", handleLogin)))
 	httpRegister("GET", "/control/logout", handleLogout)
 }
 

internal/home/control.go

@@ -107,24 +107,24 @@ func registerControlHandlers() {
 	httpRegister(http.MethodGet, "/control/status", handleStatus)
 	httpRegister(http.MethodPost, "/control/i18n/change_language", handleI18nChangeLanguage)
 	httpRegister(http.MethodGet, "/control/i18n/current_language", handleI18nCurrentLanguage)
-	http.HandleFunc("/control/version.json", postInstall(optionalAuth(handleGetVersionJSON)))
+	Context.mux.HandleFunc("/control/version.json", postInstall(optionalAuth(handleGetVersionJSON)))
 	httpRegister(http.MethodPost, "/control/update", handleUpdate)
 	httpRegister(http.MethodGet, "/control/profile", handleGetProfile)
 
 	// No auth is necessary for DOH/DOT configurations
-	http.HandleFunc("/apple/doh.mobileconfig", postInstall(handleMobileConfigDOH))
-	http.HandleFunc("/apple/dot.mobileconfig", postInstall(handleMobileConfigDOT))
+	Context.mux.HandleFunc("/apple/doh.mobileconfig", postInstall(handleMobileConfigDOH))
+	Context.mux.HandleFunc("/apple/dot.mobileconfig", postInstall(handleMobileConfigDOT))
 	RegisterAuthHandlers()
 }
 
 func httpRegister(method string, url string, handler func(http.ResponseWriter, *http.Request)) {
 	if len(method) == 0 {
 		// "/dns-query" handler doesn't need auth, gzip and isn't restricted by 1 HTTP method
-		http.HandleFunc(url, postInstall(handler))
+		Context.mux.HandleFunc(url, postInstall(handler))
 		return
 	}
 
-	http.Handle(url, postInstallHandler(optionalAuthHandler(gziphandler.GzipHandler(ensureHandler(method, handler)))))
+	Context.mux.Handle(url, postInstallHandler(optionalAuthHandler(gziphandler.GzipHandler(ensureHandler(method, handler)))))
 }
 
 // ----------------------------------
@@ -201,7 +201,6 @@ func preInstallHandler(handler http.Handler) http.Handler {
 // it also enforces HTTPS if it is enabled and configured
 func postInstall(handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
 	return func(w http.ResponseWriter, r *http.Request) {
-
 		if Context.firstRun &&
 			!strings.HasPrefix(r.URL.Path, "/install.") &&
 			!strings.HasPrefix(r.URL.Path, "/assets/") {

internal/home/control_install.go

@@ -372,7 +372,7 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) {
 }
 
 func (web *Web) registerInstallHandlers() {
-	http.HandleFunc("/control/install/get_addresses", preInstall(ensureGET(web.handleInstallGetAddresses)))
-	http.HandleFunc("/control/install/check_config", preInstall(ensurePOST(web.handleInstallCheckConfig)))
-	http.HandleFunc("/control/install/configure", preInstall(ensurePOST(web.handleInstallConfigure)))
+	Context.mux.HandleFunc("/control/install/get_addresses", preInstall(ensureGET(web.handleInstallGetAddresses)))
+	Context.mux.HandleFunc("/control/install/check_config", preInstall(ensurePOST(web.handleInstallCheckConfig)))
+	Context.mux.HandleFunc("/control/install/configure", preInstall(ensurePOST(web.handleInstallConfigure)))
 }

internal/home/filter_test.go

@@ -12,7 +12,8 @@ import (
 )
 
 func testStartFilterListener() net.Listener {
-	http.HandleFunc("/filters/1.txt", func(w http.ResponseWriter, r *http.Request) {
+	mux := http.NewServeMux()
+	mux.HandleFunc("/filters/1.txt", func(w http.ResponseWriter, r *http.Request) {
 		content := `||example.org^$third-party
 # Inline comment example
 ||example.com^$third-party
@@ -26,7 +27,7 @@ func testStartFilterListener() net.Listener {
 		panic(err)
 	}
 
-	go func() { _ = http.Serve(listener, nil) }()
+	go func() { _ = http.Serve(listener, mux) }()
 	return listener
 }
 

internal/home/home.go

@@ -67,6 +67,9 @@ type homeContext struct {
 
 	ipDetector *ipDetector
 
+	// mux is our custom http.ServeMux.
+	mux *http.ServeMux
+
 	// Runtime properties
 	// --
 
@@ -187,6 +190,8 @@ func setupContext(args options) {
 			os.Exit(0)
 		}
 	}
+
+	Context.mux = http.NewServeMux()
 }
 
 func setupConfig(args options) {
@@ -306,6 +311,10 @@ func run(args options) {
 		firstRun: Context.firstRun,
 		BindHost: config.BindHost,
 		BindPort: config.BindPort,
+
+		ReadTimeout:       ReadTimeout,
+		ReadHeaderTimeout: ReadHeaderTimeout,
+		WriteTimeout:      WriteTimeout,
 	}
 	Context.web = CreateWeb(&webConf)
 	if Context.web == nil {

internal/home/middlewares.go

@@ -2,7 +2,6 @@ package home
 
 import (
 	"net/http"
-	"strings"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghio"
 
@@ -41,19 +40,3 @@ func limitRequestBody(h http.Handler) (limited http.Handler) {
 		h.ServeHTTP(w, r)
 	})
 }
-
-// TODO(a.garipov): We currently have to use this, because everything registers
-// its HTTP handlers in http.DefaultServeMux.  In the future, refactor our HTTP
-// API initialization process and stop using the gosh darn http.DefaultServeMux
-// for anything at all.  Gosh darn global variables.
-func filterPProf(h http.Handler) (filtered http.Handler) {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if strings.HasPrefix(r.URL.Path, "/debug/pprof") {
-			http.NotFound(w, r)
-
-			return
-		}
-
-		h.ServeHTTP(w, r)
-	})
-}

internal/home/web.go

@@ -8,18 +8,45 @@ import (
 	"net/http"
 	"strconv"
 	"sync"
+	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/util"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/NYTimes/gziphandler"
 	"github.com/gobuffalo/packr"
 )
 
+const (
+	// ReadTimeout is the maximum duration for reading the entire request,
+	// including the body.
+	ReadTimeout = 10 * time.Second
+
+	// ReadHeaderTimeout is the amount of time allowed to read request
+	// headers.
+	ReadHeaderTimeout = 10 * time.Second
+
+	// WriteTimeout is the maximum duration before timing out writes of the
+	// response.
+	WriteTimeout = 10 * time.Second
+)
+
 type WebConfig struct {
 	firstRun  bool
 	BindHost  string
 	BindPort  int
 	PortHTTPS int
+
+	// ReadTimeout is an option to pass to http.Server for setting an
+	// appropriate field.
+	ReadTimeout time.Duration
+
+	// ReadHeaderTimeout is an option to pass to http.Server for setting an
+	// appropriate field.
+	ReadHeaderTimeout time.Duration
+
+	// WriteTimeout is an option to pass to http.Server for setting an
+	// appropriate field.
+	WriteTimeout time.Duration
 }
 
 // HTTPSServer - HTTPS Server
@@ -66,12 +93,12 @@ func CreateWeb(conf *WebConfig) *Web {
 	box := packr.NewBox("../../build/static")
 
 	// if not configured, redirect / to /install.html, otherwise redirect /install.html to /
-	http.Handle("/", postInstallHandler(optionalAuthHandler(gziphandler.GzipHandler(http.FileServer(box)))))
+	Context.mux.Handle("/", postInstallHandler(optionalAuthHandler(gziphandler.GzipHandler(http.FileServer(box)))))
 
 	// add handlers for /install paths, we only need them when we're not configured yet
 	if conf.firstRun {
 		log.Info("This is the first launch of AdGuard Home, redirecting everything to /install.html ")
-		http.Handle("/install.html", preInstallHandler(http.FileServer(box)))
+		Context.mux.Handle("/install.html", preInstallHandler(http.FileServer(box)))
 		w.registerInstallHandlers()
 	} else {
 		registerControlHandlers()
@@ -139,9 +166,12 @@ func (web *Web) Start() {
 		// we need to have new instance, because after Shutdown() the Server is not usable
 		address := net.JoinHostPort(web.conf.BindHost, strconv.Itoa(web.conf.BindPort))
 		web.httpServer = &http.Server{
-			ErrorLog: web.errLogger,
-			Addr:     address,
-			Handler:  withMiddlewares(http.DefaultServeMux, filterPProf, limitRequestBody),
+			ErrorLog:          web.errLogger,
+			Addr:              address,
+			Handler:           withMiddlewares(Context.mux, limitRequestBody),
+			ReadTimeout:       web.conf.ReadTimeout,
+			ReadHeaderTimeout: web.conf.ReadHeaderTimeout,
+			WriteTimeout:      web.conf.WriteTimeout,
 		}
 		err := web.httpServer.ListenAndServe()
 		if err != http.ErrServerClosed {
@@ -198,6 +228,10 @@ func (web *Web) tlsServerLoop() {
 				RootCAs:      Context.tlsRoots,
 				CipherSuites: Context.tlsCiphers,
 			},
+			Handler:           Context.mux,
+			ReadTimeout:       web.conf.ReadTimeout,
+			ReadHeaderTimeout: web.conf.ReadHeaderTimeout,
+			WriteTimeout:      web.conf.WriteTimeout,
 		}
 
 		printHTTPAddresses("https")