Pull request: return 501 when we don't support features

Merge in DNS/adguard-home from 2295-dhcp-windows to master

Updates #2295.

Squashed commit of the following:

commit 3b00a90c3d9bc33e9af478e4062c0f938d4f327d
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Nov 16 16:45:43 2020 +0300

    all: use the 501 handlers instead of the real ones, revert other changes

commit 0a3b37736a21abd6181e0d28c32069e8d7a576d0
Merge: 45feba755 6358240e9
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Nov 16 15:59:15 2020 +0300

    Merge branch 'master' into 2295-dhcp-windows and update

commit 45feba755dde37e43cc8075b896e1576157341e6
Merge: cd987d8bc a19523b25
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Nov 16 15:51:16 2020 +0300

    Merge branch 'master' into 2295-dhcp-windows

commit cd987d8bc2cd524b7454d9037b595069714645f9
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Nov 13 15:55:23 2020 +0300

    all: improve tests and refactor dhcp checking code even more

commit 3aad675443f325b5909523bcc1c987aa04ac61d9
Merge: 70c477e61 09196118e
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Nov 13 14:44:43 2020 +0300

    Merge branch 'master' into 2295-dhcp-windows

commit 70c477e61cdc1237603918f1c44470c1549f1136
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Nov 13 14:34:06 2020 +0300

    home: fix dhcpd test on windows

commit e59597d783fb9304e63f94eee2b5a5d67a5b2169
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Nov 13 13:38:25 2020 +0300

    all: mention the feature in the changelog

commit 5555c8d881b1c20b5b0a0cb096a17cf56e209c06
Merge: c3b6a5a93 e802e6645
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Nov 13 13:35:35 2020 +0300

    Merge branch 'master' into 2295-dhcp-windows

commit c3b6a5a930693090838eb1ef9f75a09b5b223ba6
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Nov 12 20:37:09 2020 +0300

    util: fix comment

commit ed92dfdb5d3a6c4ba5d032cbe781e7fd87882813
Author: ArtemBaskal <asbaskal@miem.hse.ru>
Date:   Thu Nov 12 20:24:14 2020 +0300

    Adapt client

commit e6f0494c20a4ad5388492af9091568eea5c6e2d6
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Nov 12 13:35:25 2020 +0300

    return 501 when we don't support features

CHANGELOG.md

@@ -20,4 +20,6 @@ and this project adheres to
 
 ### Fixed
 
+- `404 Not Found` errors on the DHCP settings page on *Windows*.  The page now
+  correctly shows that DHCP is not currently available on that OS (#2295).
 - Infinite loop in `/dhcp/find_active_dhcp` (#2301).

client/src/actions/index.js

@@ -373,10 +373,14 @@ export const getDhcpStatusFailure = createAction('GET_DHCP_STATUS_FAILURE');
 export const getDhcpStatus = () => async (dispatch) => {
     dispatch(getDhcpStatusRequest());
     try {
-        const status = await apiClient.getDhcpStatus();
         const globalStatus = await apiClient.getGlobalStatus();
+        if (globalStatus.dhcp_available) {
+            const status = await apiClient.getDhcpStatus();
             status.dhcp_available = globalStatus.dhcp_available;
             dispatch(getDhcpStatusSuccess(status));
+        } else {
+            dispatch(getDhcpStatusFailure());
+        }
     } catch (error) {
         dispatch(addErrorToast({ error }));
         dispatch(getDhcpStatusFailure());

client/src/components/Settings/Dhcp/Interfaces.js

@@ -74,10 +74,11 @@ const Interfaces = () => {
 
     const interfaceValue = interface_name && interfaces[interface_name];
 
-    return !processingInterfaces
+    if (processingInterfaces || !interfaces) {
-            && interfaces
+        return null;
-            && <>
+    }
-                <div className="row dhcp__interfaces">
+
+    return <div className="row dhcp__interfaces">
         <div className="col col__dhcp">
             <Field
                     name="interface_name"
@@ -94,8 +95,7 @@ const Interfaces = () => {
         </div>
         {interfaceValue
         && renderInterfaceValues(interfaceValue)}
-                </div>
+    </div>;
-            </>;
 };
 
 renderInterfaceValues.propTypes = {

client/src/components/Settings/Dhcp/index.js

@@ -65,9 +65,14 @@ const Dhcp = () => {
 
     useEffect(() => {
         dispatch(getDhcpStatus());
-        dispatch(getDhcpInterfaces());
     }, []);
 
+    useEffect(() => {
+        if (dhcp_available) {
+            dispatch(getDhcpInterfaces());
+        }
+    }, [dhcp_available]);
+
     useEffect(() => {
         const [ipv4] = interfaces?.[interface_name]?.ipv4_addresses ?? [];
         const [ipv6] = interfaces?.[interface_name]?.ipv6_addresses ?? [];

internal/dhcpd/check_other_dhcp.go

@@ -86,7 +86,7 @@ func CheckIfOtherDHCPServersPresentV4(ifaceName string) (bool, error) {
 	}
 
 	for {
-		ok, next, err := tryConn(req, c, iface)
+		ok, next, err := tryConn4(req, c, iface)
 		if next {
 			if err != nil {
 				log.Debug("dhcpv4: trying a connection: %s", err)
@@ -103,12 +103,12 @@ func CheckIfOtherDHCPServersPresentV4(ifaceName string) (bool, error) {
 	}
 }
 
-// TODO(a.garipov): Refactor further.  Inspect error handling, remove the next
+// TODO(a.garipov): Refactor further.  Inspect error handling, remove parameter
-// parameter, address the TODO, etc.
+// next, address the TODO, merge with tryConn6, etc.
-func tryConn(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, next bool, err error) {
+func tryConn4(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, next bool, err error) {
 	// TODO: replicate dhclient's behavior of retrying several times with
 	// progressively longer timeouts.
-	log.Tracef("waiting %v for an answer", defaultDiscoverTime)
+	log.Tracef("dhcpv4: waiting %v for an answer", defaultDiscoverTime)
 
 	b := make([]byte, 1500)
 	err = c.SetDeadline(time.Now().Add(defaultDiscoverTime))
@@ -127,7 +127,7 @@ func tryConn(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, ne
 		return false, false, fmt.Errorf("receiving packet: %w", err)
 	}
 
-	log.Tracef("received packet, %d bytes", n)
+	log.Tracef("dhcpv4: received packet, %d bytes", n)
 
 	response, err := dhcpv4.FromBytes(b[:n])
 	if err != nil {
@@ -149,7 +149,7 @@ func tryConn(req *dhcpv4.DHCPv4, c net.PacketConn, iface *net.Interface) (ok, ne
 		return false, true, nil
 	}
 
-	log.Tracef("the packet is from an active dhcp server")
+	log.Tracef("dhcpv4: the packet is from an active dhcp server")
 
 	return true, false, nil
 }
@@ -208,43 +208,77 @@ func CheckIfOtherDHCPServersPresentV6(ifaceName string) (bool, error) {
 	}
 
 	for {
-		log.Debug("DHCPv6: Waiting %v for an answer", defaultDiscoverTime)
+		ok, next, err := tryConn6(req, c)
-		b := make([]byte, 4096)
+		if next {
-		_ = c.SetReadDeadline(time.Now().Add(defaultDiscoverTime))
-		n, _, err := c.ReadFrom(b)
-		if isTimeout(err) {
-			log.Debug("DHCPv6: didn't receive DHCP response")
-			return false, nil
-		}
 			if err != nil {
-			return false, fmt.Errorf("couldn't receive packet: %w", err)
+				log.Debug("dhcpv6: trying a connection: %s", err)
 			}
 
-		log.Debug("DHCPv6: Received packet (%v bytes)", n)
-
-		resp, err := dhcpv6.FromBytes(b[:n])
-		if err != nil {
-			log.Debug("DHCPv6: dhcpv6.FromBytes: %s", err)
 			continue
 		}
-
+		if ok {
-		log.Debug("DHCPv6: received message from server: %s", resp.Summary())
-
-		cid := req.Options.ClientID()
-		msg, err := resp.GetInnerMessage()
-		if err != nil {
-			log.Debug("DHCPv6: resp.GetInnerMessage: %s", err)
-			continue
-		}
-		rcid := msg.Options.ClientID()
-		if resp.Type() == dhcpv6.MessageTypeAdvertise &&
-			msg.TransactionID == req.TransactionID &&
-			rcid != nil &&
-			cid.Equal(*rcid) {
-			log.Debug("DHCPv6: The packet is from an active DHCP server")
 			return true, nil
 		}
+		if err != nil {
+			return false, err
+		}
+	}
+}
 
-		log.Debug("DHCPv6: received message from server doesn't match our request")
+// TODO(a.garipov): See the comment on tryConn4.  Sigh…
+func tryConn6(req *dhcpv6.Message, c net.PacketConn) (ok, next bool, err error) {
+	// TODO: replicate dhclient's behavior of retrying several times with
+	// progressively longer timeouts.
+	log.Tracef("dhcpv6: waiting %v for an answer", defaultDiscoverTime)
+
+	b := make([]byte, 4096)
+	err = c.SetDeadline(time.Now().Add(defaultDiscoverTime))
+	if err != nil {
+		return false, false, fmt.Errorf("setting deadline: %w", err)
 	}
+
+	n, _, err := c.ReadFrom(b)
+	if err != nil {
+		if isTimeout(err) {
+			log.Debug("dhcpv6: didn't receive dhcp response")
+
+			return false, false, nil
+		}
+
+		return false, false, fmt.Errorf("receiving packet: %w", err)
+	}
+
+	log.Tracef("dhcpv6: received packet, %d bytes", n)
+
+	response, err := dhcpv6.FromBytes(b[:n])
+	if err != nil {
+		log.Debug("dhcpv6: encoding: %s", err)
+
+		return false, true, err
+	}
+
+	log.Debug("dhcpv6: received message from server: %s", response.Summary())
+
+	cid := req.Options.ClientID()
+	msg, err := response.GetInnerMessage()
+	if err != nil {
+		log.Debug("dhcpv6: resp.GetInnerMessage(): %s", err)
+
+		return false, true, err
+	}
+
+	rcid := msg.Options.ClientID()
+	if !(response.Type() == dhcpv6.MessageTypeAdvertise &&
+		msg.TransactionID == req.TransactionID &&
+		rcid != nil &&
+		cid.Equal(*rcid)) {
+
+		log.Debug("dhcpv6: received message from server doesn't match our request")
+
+		return false, true, nil
+	}
+
+	log.Tracef("dhcpv6: the packet is from an active dhcp server")
+
+	return true, false, nil
 }

internal/dhcpd/dhcpd.go

@@ -1,3 +1,4 @@
+// Package dhcpd provides a DHCP server.
 package dhcpd
 
 import (
@@ -5,6 +6,7 @@ import (
 	"net"
 	"net/http"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 	"time"
@@ -13,8 +15,10 @@ import (
 	"github.com/AdguardTeam/golibs/log"
 )
 
-const defaultDiscoverTime = time.Second * 3
+const (
-const leaseExpireStatic = 1
+	defaultDiscoverTime = time.Second * 3
+	leaseExpireStatic   = 1
+)
 
 var webHandlersRegistered = false
 
@@ -82,7 +86,8 @@ func (s *Server) CheckConfig(config ServerConfig) error {
 
 // Create - create object
 func Create(config ServerConfig) *Server {
-	s := Server{}
+	s := &Server{}
+
 	s.conf.Enabled = config.Enabled
 	s.conf.InterfaceName = config.InterfaceName
 	s.conf.HTTPRegister = config.HTTPRegister
@@ -90,10 +95,23 @@ func Create(config ServerConfig) *Server {
 	s.conf.DBFilePath = filepath.Join(config.WorkDir, dbFilename)
 
 	if !webHandlersRegistered && s.conf.HTTPRegister != nil {
-		webHandlersRegistered = true
+		if runtime.GOOS == "windows" {
+			// Our DHCP server doesn't work on Windows yet, so
+			// signal that to the front with an HTTP 501.
+			//
+			// TODO(a.garipov): This needs refactoring.  We
+			// shouldn't even try and initialize a DHCP server on
+			// Windows, but there are currently too many
+			// interconnected parts--such as HTTP handlers and
+			// frontend--to make that work properly.
+			s.registerNotImplementedHandlers()
+		} else {
 			s.registerHandlers()
 		}
 
+		webHandlersRegistered = true
+	}
+
 	var err4, err6 error
 	v4conf := config.Conf4
 	v4conf.Enabled = s.conf.Enabled
@@ -130,7 +148,7 @@ func Create(config ServerConfig) *Server {
 	// we can't delay database loading until DHCP server is started,
 	//  because we need static leases functionality available beforehand
 	s.dbLoad()
-	return &s
+	return s
 }
 
 // server calls this function after DB is updated

internal/dhcpd/dhcphttp.go

@@ -11,7 +11,6 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/util"
-
 	"github.com/AdguardTeam/golibs/jsonutil"
 	"github.com/AdguardTeam/golibs/log"
 )
@@ -499,11 +498,48 @@ func (s *Server) handleReset(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) registerHandlers() {
-	s.conf.HTTPRegister("GET", "/control/dhcp/status", s.handleDHCPStatus)
+	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/status", s.handleDHCPStatus)
-	s.conf.HTTPRegister("GET", "/control/dhcp/interfaces", s.handleDHCPInterfaces)
+	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/interfaces", s.handleDHCPInterfaces)
-	s.conf.HTTPRegister("POST", "/control/dhcp/set_config", s.handleDHCPSetConfig)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/set_config", s.handleDHCPSetConfig)
-	s.conf.HTTPRegister("POST", "/control/dhcp/find_active_dhcp", s.handleDHCPFindActiveServer)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/find_active_dhcp", s.handleDHCPFindActiveServer)
-	s.conf.HTTPRegister("POST", "/control/dhcp/add_static_lease", s.handleDHCPAddStaticLease)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/add_static_lease", s.handleDHCPAddStaticLease)
-	s.conf.HTTPRegister("POST", "/control/dhcp/remove_static_lease", s.handleDHCPRemoveStaticLease)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/remove_static_lease", s.handleDHCPRemoveStaticLease)
-	s.conf.HTTPRegister("POST", "/control/dhcp/reset", s.handleReset)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", s.handleReset)
+}
+
+// jsonError is a generic JSON error response.
+type jsonError struct {
+	// Message is the error message, an opaque string.
+	Message string `json:"message"`
+}
+
+// notImplemented returns a handler that replies to any request with an HTTP 501
+// Not Implemented status and a JSON error with the provided message msg.
+//
+// TODO(a.garipov): Either take the logger from the server after we've
+// refactored logging or make this not a method of *Server.
+func (s *Server) notImplemented(msg string) (f func(http.ResponseWriter, *http.Request)) {
+	return func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusNotImplemented)
+
+		err := json.NewEncoder(w).Encode(&jsonError{
+			Message: msg,
+		})
+		if err != nil {
+			log.Debug("writing 501 json response: %s", err)
+		}
+	}
+}
+
+func (s *Server) registerNotImplementedHandlers() {
+	h := s.notImplemented("dhcp is not supported on windows")
+
+	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/status", h)
+	s.conf.HTTPRegister(http.MethodGet, "/control/dhcp/interfaces", h)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/set_config", h)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/find_active_dhcp", h)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/add_static_lease", h)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/remove_static_lease", h)
+	s.conf.HTTPRegister(http.MethodPost, "/control/dhcp/reset", h)
 }

internal/dhcpd/dhcphttp_test.go

@@ -0,0 +1,22 @@
+package dhcpd
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestServer_notImplemented(t *testing.T) {
+	s := &Server{}
+	h := s.notImplemented("never!")
+
+	w := httptest.NewRecorder()
+	r, err := http.NewRequest(http.MethodGet, "/unsupported", nil)
+	assert.Nil(t, err)
+
+	h(w, r)
+	assert.Equal(t, http.StatusNotImplemented, w.Code)
+	assert.Equal(t, `{"message":"never!"}`+"\n", w.Body.String())
+}

internal/dhcpd/network_utils.go

@@ -17,7 +17,8 @@ import (
 	"github.com/AdguardTeam/golibs/log"
 )
 
-// Check if network interface has a static IP configured
+// HasStaticIP check if the network interface has a static IP configured
+//
 // Supports: Raspbian.
 func HasStaticIP(ifaceName string) (bool, error) {
 	if runtime.GOOS == "linux" {
@@ -36,7 +37,7 @@ func HasStaticIP(ifaceName string) (bool, error) {
 	return false, fmt.Errorf("cannot check if IP is static: not supported on %s", runtime.GOOS)
 }
 
-// Set a static IP for the specified network interface
+// SetStaticIP sets a static IP for the network interface.
 func SetStaticIP(ifaceName string) error {
 	if runtime.GOOS == "linux" {
 		return setStaticIPDhcpdConf(ifaceName)

internal/home/control.go

@@ -6,6 +6,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"runtime"
 	"strconv"
 	"strings"
 
@@ -46,6 +47,7 @@ func handleStatus(w http.ResponseWriter, r *http.Request) {
 	if Context.dnsServer != nil {
 		Context.dnsServer.WriteDiskConfig(&c)
 	}
+
 	data := map[string]interface{}{
 		"dns_addresses": getDNSAddresses(),
 		"http_port":     config.BindPort,
@@ -56,7 +58,17 @@ func handleStatus(w http.ResponseWriter, r *http.Request) {
 
 		"protection_enabled": c.ProtectionEnabled,
 	}
+
+	if runtime.GOOS == "windows" {
+		// Set the DHCP to false explicitly, because Context.dhcpServer
+		// is probably not nil, despite the fact that there is no
+		// support for DHCP on Windows in AdGuardHome.
+		//
+		// See also the TODO in dhcpd.Create.
+		data["dhcp_available"] = false
+	} else {
 		data["dhcp_available"] = (Context.dhcpServer != nil)
+	}
 
 	jsonVal, err := json.Marshal(data)
 	if err != nil {

internal/home/home.go

@@ -20,18 +20,16 @@ import (
 	"syscall"
 	"time"
 
-	"gopkg.in/natefinch/lumberjack.v2"
-
 	"github.com/AdguardTeam/AdGuardHome/internal/agherr"
-	"github.com/AdguardTeam/AdGuardHome/internal/update"
-	"github.com/AdguardTeam/AdGuardHome/internal/util"
-
 	"github.com/AdguardTeam/AdGuardHome/internal/dhcpd"
 	"github.com/AdguardTeam/AdGuardHome/internal/dnsfilter"
 	"github.com/AdguardTeam/AdGuardHome/internal/dnsforward"
 	"github.com/AdguardTeam/AdGuardHome/internal/querylog"
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
+	"github.com/AdguardTeam/AdGuardHome/internal/update"
+	"github.com/AdguardTeam/AdGuardHome/internal/util"
 	"github.com/AdguardTeam/golibs/log"
+	"gopkg.in/natefinch/lumberjack.v2"
 )
 
 const (
@@ -216,12 +214,12 @@ func run(args options) {
 	config.DHCP.WorkDir = Context.workDir
 	config.DHCP.HTTPRegister = httpRegister
 	config.DHCP.ConfigModified = onConfigModified
-	if runtime.GOOS != "windows" {
+
 	Context.dhcpServer = dhcpd.Create(config.DHCP)
 	if Context.dhcpServer == nil {
-			log.Fatalf("Can't initialize DHCP module")
+		log.Fatalf("can't initialize dhcp module")
-		}
 	}
+
 	Context.autoHosts.Init("")
 
 	Context.updater = update.NewUpdater(update.Config{

internal/util/helpers.go

@@ -1,3 +1,7 @@
+// Package util contains various utilities.
+//
+// TODO(a.garipov): Such packages are widely considered an antipattern.  Remove
+// this when we refactor our project structure.
 package util
 
 import (

openapi/openapi.yaml

@@ -339,6 +339,12 @@ paths:
                         application/json:
                             schema:
                                 $ref: "#/components/schemas/DhcpStatus"
+                "501":
+                    content:
+                        application/json:
+                            schema:
+                                $ref: "#/components/schemas/Error"
+                    description: Not implemented (for example, on Windows).
     /dhcp/set_config:
         post:
             tags:
@@ -353,6 +359,12 @@ paths:
             responses:
                 "200":
                     description: OK
+                "501":
+                    content:
+                        application/json:
+                            schema:
+                                $ref: "#/components/schemas/Error"
+                    description: Not implemented (for example, on Windows).
     /dhcp/find_active_dhcp:
         post:
             tags:
@@ -366,6 +378,12 @@ paths:
                         application/json:
                             schema:
                                 $ref: "#/components/schemas/DhcpSearchResult"
+                "501":
+                    content:
+                        application/json:
+                            schema:
+                                $ref: "#/components/schemas/Error"
+                    description: Not implemented (for example, on Windows).
     /dhcp/add_static_lease:
         post:
             tags:
@@ -377,6 +395,12 @@ paths:
             responses:
                 "200":
                     description: OK
+                "501":
+                    content:
+                        application/json:
+                            schema:
+                                $ref: "#/components/schemas/Error"
+                    description: Not implemented (for example, on Windows).
     /dhcp/remove_static_lease:
         post:
             tags:
@@ -388,6 +412,12 @@ paths:
             responses:
                 "200":
                     description: OK
+                "501":
+                    content:
+                        application/json:
+                            schema:
+                                $ref: "#/components/schemas/Error"
+                    description: Not implemented (for example, on Windows).
     /dhcp/reset:
         post:
             tags:
@@ -397,6 +427,12 @@ paths:
             responses:
                 "200":
                     description: OK
+                "501":
+                    content:
+                        application/json:
+                            schema:
+                                $ref: "#/components/schemas/Error"
+                    description: Not implemented (for example, on Windows).
     /filtering/status:
         get:
             tags:
@@ -1976,3 +2012,10 @@ components:
                 password:
                     type: string
                     description: Password
+        Error:
+            description: A generic JSON error response.
+            properties:
+                message:
+                    type: string
+                    description: The error message, an opaque string.
+            type: object