Pull request 385: 418-hosts-files-paths

Merge in GO/dnsproxy from 418-hosts-files-paths to master Squashed commit of the following: commit 8f9d0a4 Author: Stanislav Chzhen <[email protected]> Date: Thu Dec 26 16:16:20 2024 +0300 all: imp code commit 5a4b52b Author: Stanislav Chzhen <[email protected]> Date: Thu Dec 26 15:39:28 2024 +0300 all: imp code commit ccca735 Author: Stanislav Chzhen <[email protected]> Date: Thu Dec 26 14:41:59 2024 +0300 all: imp docs commit b96522a Author: Stanislav Chzhen <[email protected]> Date: Wed Dec 25 21:06:29 2024 +0300 all: imp code commit ee07eda Author: Stanislav Chzhen <[email protected]> Date: Tue Dec 24 19:00:51 2024 +0300 cmd: hosts files paths
AdguardTeam · Dec 26, 2024 · ed4be08 · ed4be08
1 parent d80a8e3
commit ed4be08
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 53 deletions.
diff --git a/README.md b/README.md
@@ -76,7 +76,7 @@ Application Options:
       --private-subnets=           Private subnets to use for reverse DNS lookups of private addresses
       --bogus-nxdomain=            Transform the responses containing at least a single IP that matches specified addresses and CIDRs into
                                    NXDOMAIN.  Can be specified multiple times.
-      --hosts-files=               List of paths to the hosts files relative to the root, can be specified multiple times
+      --hosts-files=               List of paths to the hosts files, can be specified multiple times
       --timeout=                   Timeout for outbound DNS queries to remote upstream servers in a human-readable form (default: 10s)
       --cache-min-ttl=             Minimum TTL value for DNS entries, in seconds. Capped at 3600. Artificially extending TTLs should only be
                                    done with careful consideration.

diff --git a/internal/cmd/args.go b/internal/cmd/args.go
@@ -221,11 +221,10 @@ var commandLineOptions = []*commandLineOption{
 		valueType: "subnet",
 	},
 	hostsFilesIdx: {
-		description: "List of paths to the hosts files relative to the root, can be specified " +
-			"multiple times.",
-		long:      "hosts-files",
-		short:     "",
-		valueType: "path",
+		description: "List of paths to the hosts files, can be specified multiple times.",
+		long:        "hosts-files",
+		short:       "",
+		valueType:   "path",
 	},
 	timeoutIdx: {
 		description: "Timeout for outbound DNS queries to remote upstream servers in a " +

diff --git a/internal/cmd/proxy.go b/internal/cmd/proxy.go
@@ -40,17 +40,18 @@ func createProxyConfig(
 		return nil, err
 	}
 
-	reqHdlr, err := handler.NewDefault(&handler.DefaultConfig{
+	hosts, err := handler.ReadHosts(hostsFiles)
+	if err != nil {
+		return nil, fmt.Errorf("reading hosts files: %w", err)
+	}
+
+	reqHdlr := handler.NewDefault(&handler.DefaultConfig{
 		Logger: l.With(slogutil.KeyPrefix, "default_handler"),
 		// TODO(e.burkov):  Use the configured message constructor.
 		MessageConstructor: dnsmsg.DefaultMessageConstructor{},
 		HaltIPv6:           conf.IPv6Disabled,
-		HostsFiles:         hostsFiles,
-		FileSystem:         osutil.RootDirFS(),
+		HostsFiles:         hosts,
 	})
-	if err != nil {
-		return nil, fmt.Errorf("creating default handler: %w", err)
-	}
 
 	proxyConf = &proxy.Config{
 		Logger: l.With(slogutil.KeyPrefix, proxy.LogPrefix),

diff --git a/internal/handler/default.go b/internal/handler/default.go
@@ -1,10 +1,7 @@
-// Package handler provides some customizable DNS request handling logic used in
-// the proxy.
 package handler
 
 import (
 	"context"
-	"io/fs"
 	"log/slog"
 
 	"github.com/AdguardTeam/dnsproxy/proxy"
@@ -16,18 +13,11 @@ type DefaultConfig struct {
 	// MessageConstructor constructs DNS messages.  It must not be nil.
 	MessageConstructor proxy.MessageConstructor
 
-	// FileSystem is the file system for reading files from.  It must not be
-	// nil.
-	FileSystem fs.FS
-
 	// Logger is the logger.  It must not be nil.
 	Logger *slog.Logger
 
-	// HostsFiles is the list of paths to the hosts files.  The hosts files
-	// aren't used if the list is empty.
-	//
-	// TODO(e.burkov):  Consider passing just a [hostsfile.Storage].
-	HostsFiles []string
+	// HostsFiles is the index containing the records of the hosts files.
+	HostsFiles hostsfile.Storage
 
 	// HaltIPv6 halts the processing of AAAA requests and makes the handler
 	// reply with NODATA to them.
@@ -43,12 +33,7 @@ type Default struct {
 }
 
 // NewDefault creates a new [Default] handler.
-func NewDefault(conf *DefaultConfig) (d *Default, err error) {
-	hosts, err := readHosts(conf.FileSystem, conf.HostsFiles)
-	if err != nil {
-		return nil, err
-	}
-
+func NewDefault(conf *DefaultConfig) (d *Default) {
 	mc, ok := conf.MessageConstructor.(messageConstructor)
 	if !ok {
 		mc = defaultConstructor{
@@ -60,8 +45,8 @@ func NewDefault(conf *DefaultConfig) (d *Default, err error) {
 		logger:       conf.Logger,
 		isIPv6Halted: conf.HaltIPv6,
 		messages:     mc,
-		hosts:        hosts,
-	}, nil
+		hosts:        conf.HostsFiles,
+	}
 }
 
 // HandleRequest resolves the DNS request within proxyCtx.  It only calls

diff --git a/internal/handler/default_internal_test.go b/internal/handler/default_internal_test.go
@@ -1,11 +1,11 @@
 package handler
 
 import (
-	"io/fs"
 	"net"
 	"net/netip"
 	"os"
 	"path"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -33,9 +33,6 @@ func TestMain(m *testing.M) {
 // defaultTimeout is a default timeout for tests and contexts.
 const defaultTimeout = 1 * time.Second
 
-// testdata is the file system for test data.
-var testdata fs.FS = os.DirFS("testdata")
-
 func TestDefault_haltAAAA(t *testing.T) {
 	t.Parallel()
 
@@ -52,13 +49,11 @@ func TestDefault_haltAAAA(t *testing.T) {
 	t.Run("disabled", func(t *testing.T) {
 		t.Parallel()
 
-		hdlr, err := NewDefault(&DefaultConfig{
+		hdlr := NewDefault(&DefaultConfig{
 			Logger:             slogutil.NewDiscardLogger(),
 			MessageConstructor: messages,
 			HaltIPv6:           false,
-			FileSystem:         testdata,
 		})
-		require.NoError(t, err)
 
 		ctx := testutil.ContextWithTimeout(t, defaultTimeout)
 
@@ -69,13 +64,11 @@ func TestDefault_haltAAAA(t *testing.T) {
 	t.Run("enabled", func(t *testing.T) {
 		t.Parallel()
 
-		hdlr, err := NewDefault(&DefaultConfig{
+		hdlr := NewDefault(&DefaultConfig{
 			Logger:             slogutil.NewDiscardLogger(),
 			MessageConstructor: messages,
 			HaltIPv6:           true,
-			FileSystem:         testdata,
 		})
-		require.NoError(t, err)
 
 		ctx := testutil.ContextWithTimeout(t, defaultTimeout)
 
@@ -90,14 +83,19 @@ func TestDefault_resolveFromHosts(t *testing.T) {
 	// TODO(e.burkov):  Use the one from [dnsproxytest].
 	messages := dnsmsg.DefaultMessageConstructor{}
 
-	hdlr, err := NewDefault(&DefaultConfig{
+	relPath := path.Join("testdata", t.Name(), "hosts")
+	absPath, err := filepath.Abs(path.Join("testdata", t.Name(), "hosts"))
+	require.NoError(t, err)
+
+	strg, err := ReadHosts([]string{absPath, relPath})
+	require.NoError(t, err)
+
+	hdlr := NewDefault(&DefaultConfig{
 		MessageConstructor: messages,
-		FileSystem:         testdata,
 		Logger:             slogutil.NewDiscardLogger(),
-		HostsFiles:         []string{path.Join(t.Name(), "hosts")},
+		HostsFiles:         strg,
 		HaltIPv6:           true,
 	})
-	require.NoError(t, err)
 
 	const (
 		domainV4 = "ipv4.domain.example"

diff --git a/internal/handler/handler.go b/internal/handler/handler.go
@@ -0,0 +1,3 @@
+// Package handler provides some customizable DNS request handling logic used in
+// the proxy.
+package handler
diff --git a/internal/handler/hosts.go b/internal/handler/hosts.go
@@ -3,8 +3,8 @@ package handler
 import (
 	"context"
 	"fmt"
-	"io/fs"
 	"net/netip"
+	"os"
 	"slices"
 	"strings"
 
@@ -33,15 +33,15 @@ func (emptyStorage) ByName(_ string) (addrs []netip.Addr) {
 	return nil
 }
 
-// readHosts reads the hosts files from the file system and returns a storage
+// ReadHosts reads the hosts files from the file system and returns a storage
 // with parsed records.  strg is always usable even if an error occurred.
-func readHosts(fsys fs.FS, paths []string) (strg hostsfile.Storage, err error) {
+func ReadHosts(paths []string) (strg hostsfile.Storage, err error) {
 	// Don't check the error since it may only appear when any readers used.
 	defaultStrg, _ := hostsfile.NewDefaultStorage()
 
 	var errs []error
 	for _, path := range paths {
-		err = readHostsFile(defaultStrg, fsys, path)
+		err = readHostsFile(defaultStrg, path)
 		if err != nil {
 			// Don't wrap the error since it's informative enough as is.
 			errs = append(errs, err)
@@ -63,13 +63,15 @@ func readHosts(fsys fs.FS, paths []string) (strg hostsfile.Storage, err error) {
 	return defaultStrg, errors.Join(errs...)
 }
 
-// readHostsFile reads the hosts file at path from fsys and parses it into strg.
-func readHostsFile(strg *hostsfile.DefaultStorage, fsys fs.FS, path string) (err error) {
-	f, err := fsys.Open(path)
+// readHostsFile reads the hosts file at path and parses it into strg.
+func readHostsFile(strg *hostsfile.DefaultStorage, path string) (err error) {
+	// #nosec G304 -- Trust the file path from the configuration file.
+	f, err := os.Open(path)
 	if err != nil {
 		// Don't wrap the error since it's informative enough as is.
 		return err
 	}
+
 	defer func() { err = errors.WithDeferred(err, f.Close()) }()
 
 	err = hostsfile.Parse(strg, f, nil)