From 5aa0ca9319919f35eb2e1dc4c9e6d8f0385c394e Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Fri, 5 Mar 2021 19:20:36 +0300 Subject: [PATCH] Pull request: 2582 invalid hostname vol.2 Merge in DNS/adguard-home from 2582-invalid-hostname-2 to master Updates #2582. Squashed commit of the following: commit 9d3ceb289e3869b2c3d12e91ec104fb25d7931ee Merge: 91c68e46 90054974 Author: Eugene Burkov Date: Fri Mar 5 19:11:49 2021 +0300 Merge branch 'master' into 2582-invalid-hostname-2 commit 91c68e468c5f5b12a2fb509ff391133483c9d915 Author: Eugene Burkov Date: Fri Mar 5 18:28:14 2021 +0300 all: mv trimming from home to dhcpd commit f51faf35288577b6f610f172b26e7ac13aa24f72 Author: Eugene Burkov Date: Fri Mar 5 16:28:00 2021 +0300 home: add more host sanitizings --- internal/dhcpd/v4.go | 12 +++++++++++- internal/home/clients.go | 13 ------------- internal/home/clients_test.go | 26 -------------------------- 3 files changed, 11 insertions(+), 40 deletions(-) diff --git a/internal/dhcpd/v4.go b/internal/dhcpd/v4.go index 7d24699e..38c0a90d 100644 --- a/internal/dhcpd/v4.go +++ b/internal/dhcpd/v4.go @@ -7,6 +7,7 @@ import ( "encoding/binary" "fmt" "net" + "strings" "sync" "time" @@ -463,7 +464,16 @@ func (s *v4Server) processRequest(req, resp *dhcpv4.DHCPv4) (*Lease, bool) { } if lease.Expiry.Unix() != leaseExpireStatic { - lease.Hostname = string(hostname) + // The trimming is required since some devices include trailing + // zero-byte in DHCP option length calculation. + // + // See https://github.com/AdguardTeam/AdGuardHome/issues/2582. + // + // TODO(e.burkov): Remove after the trimming for hostname option + // will be added into github.com/insomniacslk/dhcp module. + hostnameStr := strings.TrimRight(string(hostname), "\x00") + + lease.Hostname = hostnameStr s.commitLease(lease) } else if len(lease.Hostname) != 0 { o := &optFQDN{ diff --git a/internal/home/clients.go b/internal/home/clients.go index 6c314874..5a9e9eac 100644 --- a/internal/home/clients.go +++ b/internal/home/clients.go @@ -586,22 +586,9 @@ func (clients *clientsContainer) SetWhoisInfo(ip string, info [][]string) { log.Debug("clients: set whois info for auto-client with IP %s: %q", ip, info) } -// sanitizeHost proccesses a host to remove some special symbols causing errors. -// Logic may be expanded in the future. -func sanitizeHost(host string) (processed string) { - // cutset brings together all the deprecated sets. - // - // See https://github.com/AdguardTeam/AdGuardHome/issues/2582. - const cutset = "\x00" - - return strings.TrimRight(host, cutset) -} - // AddHost adds a new IP-hostname pairing. The priorities of the sources is // taken into account. ok is true if the pairing was added. func (clients *clientsContainer) AddHost(ip, host string, src clientSource) (ok bool, err error) { - host = sanitizeHost(host) - clients.lock.Lock() ok = clients.addHostLocked(ip, host, src) clients.lock.Unlock() diff --git a/internal/home/clients_test.go b/internal/home/clients_test.go index e6200ddd..a098bf4c 100644 --- a/internal/home/clients_test.go +++ b/internal/home/clients_test.go @@ -290,29 +290,3 @@ func TestClientsCustomUpstream(t *testing.T) { assert.Equal(t, 1, len(config.Upstreams)) assert.Equal(t, 1, len(config.DomainReservedUpstreams)) } - -func TestProcessHost(t *testing.T) { - const ( - name int = iota - host - want - - fieldsNum - ) - - testCases := [][fieldsNum]string{{ - name: "valid", - host: "abc", - want: "abc", - }, { - name: "with_trailing_zero_byte", - host: "abc\x00", - want: "abc", - }} - - for _, tc := range testCases { - t.Run(tc[name], func(t *testing.T) { - assert.Equal(t, tc[want], sanitizeHost(tc[host])) - }) - } -}