From ed4be089f6d46d14f4cc6502f50194a0e776a0ca Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Thu, 26 Dec 2024 19:48:52 +0300 Subject: [PATCH] Pull request 385: 418-hosts-files-paths Merge in GO/dnsproxy from 418-hosts-files-paths to master Squashed commit of the following: commit 8f9d0a4fbf9dada88c3259f5a357e6b7bcc29afe Author: Stanislav Chzhen Date: Thu Dec 26 16:16:20 2024 +0300 all: imp code commit 5a4b52be3be0d52802727cc7809bf4bd45b2d933 Author: Stanislav Chzhen Date: Thu Dec 26 15:39:28 2024 +0300 all: imp code commit ccca73513acda479c7591fd6b33f308a032ccf13 Author: Stanislav Chzhen Date: Thu Dec 26 14:41:59 2024 +0300 all: imp docs commit b96522a073b0e44a0ca876a24a4b600926c3a203 Author: Stanislav Chzhen Date: Wed Dec 25 21:06:29 2024 +0300 all: imp code commit ee07edaa41ac32ff3795c47c654b9aaa97c485f8 Author: Stanislav Chzhen Date: Tue Dec 24 19:00:51 2024 +0300 cmd: hosts files paths --- README.md | 2 +- internal/cmd/args.go | 9 ++++---- internal/cmd/proxy.go | 13 ++++++------ internal/handler/default.go | 25 +++++----------------- internal/handler/default_internal_test.go | 26 +++++++++++------------ internal/handler/handler.go | 3 +++ internal/handler/hosts.go | 16 ++++++++------ 7 files changed, 41 insertions(+), 53 deletions(-) create mode 100644 internal/handler/handler.go diff --git a/README.md b/README.md index c29624d07..303abc1d2 100644 --- 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 index 4626fef1a..adeca257e 100644 --- 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 index f272af30b..578ee163c 100644 --- 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 index b1883a8d2..5e3c0948e 100644 --- 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 index 913e6b390..d48856a66 100644 --- 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 new file mode 100644 index 000000000..18d2e2f5e --- /dev/null +++ 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 index 9eb840ab0..34bee3706 100644 --- 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)