plugin/hosts: parse hosts files with bufio.Reader (#8516)

plugin/hosts/hostsfile.go

@@ -164,77 +164,176 @@ func (h *Hostsfile) initInline(inline []string) {
 	h.inline = h.parse(strings.NewReader(strings.Join(inline, "\n")))
 }
 
-// maxLineSize is the largest hosts file line we are willing to parse. A line
+// maxFieldSize bounds the memory used while assembling a single field that
-// can legitimately be long when many names share a single address, so this is
+// spans several reads. A DNS name is at most 255 octets, so a longer field can
-// well above bufio.Scanner's 64KiB default, but still bounded.
+// never yield a usable entry and is discarded instead of being buffered.
-const maxLineSize = 1024 * 1024
+const maxFieldSize = 1024
 
 // Parse reads the hostsfile and populates the byName and addr maps.
+//
+// Lines are read with a bufio.Reader and parsed field by field as the data
+// arrives, so a line of any length is handled with a fixed amount of memory
+// and never aborts the parse of the entries that follow it.
 func (h *Hostsfile) parse(r io.Reader) *Map {
 	hmap := newMap()
+	p := lineParser{h: h, hmap: hmap}
 
-	scanner := bufio.NewScanner(r)
+	reader := bufio.NewReader(r)
-	// The scanner grows its buffer as needed; only raise the limit at which it
+	for {
-	// gives up, otherwise a single long line aborts the scan and every entry
+		chunk, err := reader.ReadSlice('\n')
-	// after it is dropped.
+		// The slice returned by ReadSlice is only valid until the next read,
-	scanner.Buffer(nil, maxLineSize)
+		// so feed consumes it before looping. ErrBufferFull means the line
-	for scanner.Scan() {
+		// continues in the next chunk.
-		line := scanner.Bytes()
+		p.feed(chunk, err != bufio.ErrBufferFull)
-		if i := bytes.Index(line, []byte{'#'}); i >= 0 {
+		if err == nil || err == bufio.ErrBufferFull {
+			continue
+		}
+		if err != io.EOF {
+			log.Errorf("Failed to parse hosts file %q: %v", h.path, err)
+		}
+		return hmap
+	}
+}
+
+// lineParser turns a stream of chunks into hosts file entries. It keeps only
+// the current field, so its memory use does not grow with the line length.
+type lineParser struct {
+	h    *Hostsfile
+	hmap *Map
+
+	field     []byte // the field being assembled, possibly spanning chunks
+	oversized bool   // the current field exceeded maxFieldSize and is dropped
+	index     int    // number of fields already seen on this line
+	comment   bool   // the rest of this line is a comment
+	addr      net.IP // address of the current line, nil if unusable
+	family    int
+}
+
+// feed consumes one chunk of the current line. last reports whether the chunk
+// ends the line.
+func (p *lineParser) feed(chunk []byte, last bool) {
+	if !p.comment {
+		if i := bytes.IndexByte(chunk, '#'); i >= 0 {
 			// Discard comments.
-			line = line[0:i]
+			chunk = chunk[:i]
-		}
+			p.comment = true
-		f := bytes.Fields(line)
-		if len(f) < 2 {
-			continue
-		}
-		addr := parseIP(string(f[0]))
-		if addr == nil {
-			continue
 		}
+		p.scan(chunk, last || p.comment)
+	}
+	if last {
+		p.index, p.comment, p.addr = 0, false, nil
+	}
+}
 
-		var family int
+// scan splits a chunk into fields. A field at the end of the chunk is only
-		if addr.To4() != nil {
+// complete if terminal is set, otherwise it continues in the next chunk.
-			family = 1
+func (p *lineParser) scan(b []byte, terminal bool) {
+	for len(b) > 0 {
+		i := 0
+		for i < len(b) && isSpace(b[i]) {
+			i++
+		}
+		if i > 0 {
+			// Whitespace terminates the field before it.
+			p.emit()
+			b = b[i:]
+			continue
+		}
+		j := 0
+		for j < len(b) && !isSpace(b[j]) {
+			j++
+		}
+		p.append(b[:j])
+		b = b[j:]
+	}
+	if terminal {
+		p.emit()
+	}
+}
+
+// append extends the current field, dropping it once it grows beyond any
+// length a DNS name can have.
+func (p *lineParser) append(b []byte) {
+	if p.oversized {
+		return
+	}
+	if len(p.field)+len(b) > maxFieldSize {
+		p.oversized = true
+		p.field = p.field[:0]
+		return
+	}
+	p.field = append(p.field, b...)
+}
+
+// emit handles a completed field. It is a no-op when no field is pending.
+func (p *lineParser) emit() {
+	if len(p.field) == 0 && !p.oversized {
+		return
+	}
+	// field aliases p.field's storage, which is reused by the next append; it
+	// is only read below, before any further append happens.
+	field, oversized := p.field, p.oversized
+	p.field, p.oversized = p.field[:0], false
+	p.index++
+
+	if p.index == 1 {
+		// The first field is the address; without it the line is unusable.
+		if oversized {
+			return
+		}
+		p.addr = parseIP(string(field))
+		if p.addr == nil {
+			return
+		}
+		if p.addr.To4() != nil {
+			p.family = 1
 		} else {
-			family = 2
+			p.family = 2
 		}
+		return
+	}
+	if p.addr == nil || oversized {
+		return
+	}
+	p.addName(string(field))
+}
 
-		for i := 1; i < len(f); i++ {
+func (p *lineParser) addName(field string) {
-			name := plugin.Name(string(f[i])).Normalize()
+	name := plugin.Name(field).Normalize()
-			if !plugin.Zones(h.Origins).Contains(name) {
+	if !plugin.Zones(p.h.Origins).Contains(name) {
-				// name is not in Origins
+		// name is not in Origins
-				continue
+		return
-			}
+	}
-			if isWildcardName(name) {
+	if isWildcardName(name) {
-				switch family {
+		switch p.family {
-				case 1:
+		case 1:
-					hmap.wildName4[name] = append(hmap.wildName4[name], addr)
+			p.hmap.wildName4[name] = append(p.hmap.wildName4[name], p.addr)
-				case 2:
+		case 2:
-					hmap.wildName6[name] = append(hmap.wildName6[name], addr)
+			p.hmap.wildName6[name] = append(p.hmap.wildName6[name], p.addr)
-				}
-				continue
-			}
-			switch family {
-			case 1:
-				hmap.name4[name] = append(hmap.name4[name], addr)
-			case 2:
-				hmap.name6[name] = append(hmap.name6[name], addr)
-			default:
-				continue
-			}
-			if !h.options.autoReverse {
-				continue
-			}
-			hmap.addr[addr.String()] = append(hmap.addr[addr.String()], name)
 		}
+		return
 	}
-	if err := scanner.Err(); err != nil {
+	switch p.family {
-		// Entries after the failing line have not been read.
+	case 1:
-		log.Errorf("Failed to parse hosts file %q: %v", h.path, err)
+		p.hmap.name4[name] = append(p.hmap.name4[name], p.addr)
+	case 2:
+		p.hmap.name6[name] = append(p.hmap.name6[name], p.addr)
+	default:
+		return
 	}
+	if !p.h.options.autoReverse {
+		return
+	}
+	key := p.addr.String()
+	p.hmap.addr[key] = append(p.hmap.addr[key], name)
+}
 
-	return hmap
+func isSpace(c byte) bool {
+	switch c {
+	case ' ', '\t', '\n', '\v', '\f', '\r':
+		return true
+	}
+	return false
 }
 
 func (h *Hostsfile) lookupStaticHostLocked(m, wild map[string][]net.IP, host string) []net.IP {

plugin/hosts/hostsfile_test.go

@@ -5,6 +5,7 @@
 package hosts
 
 import (
+	"fmt"
 	"net"
 	"os"
 	"reflect"
@@ -306,3 +307,82 @@ func TestParseLineLongerThanDefaultScanBuffer(t *testing.T) {
 		t.Errorf("LookupStaticHostV4(after.example.org.) = %v, want [127.0.0.3]", addrs)
 	}
 }
+
+func TestParseVeryLongLine(t *testing.T) {
+	// A line of several megabytes must be parsed without an arbitrary cutoff,
+	// and must not hide the entries that follow it.
+	var sb strings.Builder
+	sb.WriteString("127.0.0.1 before.example.org\n")
+	sb.WriteString("127.0.0.2")
+	const names = 200000
+	for i := range names {
+		fmt.Fprintf(&sb, " n%d.example.org", i)
+	}
+	sb.WriteString("\n127.0.0.3 after.example.org\n")
+	if sb.Len() < 3<<20 {
+		t.Fatalf("test line is %d bytes, want at least 3 MiB", sb.Len())
+	}
+	h := testHostsfile(sb.String())
+
+	for _, tc := range []struct {
+		name string
+		addr string
+	}{
+		{"before.example.org.", "127.0.0.1"},
+		{"n0.example.org.", "127.0.0.2"},
+		{"n199999.example.org.", "127.0.0.2"},
+		{"after.example.org.", "127.0.0.3"},
+	} {
+		if addrs := h.LookupStaticHostV4(tc.name); len(addrs) != 1 || addrs[0].String() != tc.addr {
+			t.Errorf("LookupStaticHostV4(%s) = %v, want [%s]", tc.name, addrs, tc.addr)
+		}
+	}
+}
+
+func TestParseLongFieldSpanningReads(t *testing.T) {
+	// A single field longer than the read buffer is not a usable name, but it
+	// must not corrupt the fields around it or the rest of the file.
+	long := strings.Repeat("a", 5<<20)
+	h := testHostsfile("127.0.0.1 before.example.org\n" +
+		"127.0.0.2 " + long + ".example.org one.example.org\n" +
+		"127.0.0.3 after.example.org\n")
+
+	for _, tc := range []struct {
+		name string
+		addr string
+	}{
+		{"before.example.org.", "127.0.0.1"},
+		{"one.example.org.", "127.0.0.2"},
+		{"after.example.org.", "127.0.0.3"},
+	} {
+		if addrs := h.LookupStaticHostV4(tc.name); len(addrs) != 1 || addrs[0].String() != tc.addr {
+			t.Errorf("LookupStaticHostV4(%s) = %v, want [%s]", tc.name, addrs, tc.addr)
+		}
+	}
+}
+
+func TestParseNoTrailingNewline(t *testing.T) {
+	h := testHostsfile("127.0.0.1 first.example.org\n127.0.0.2 last.example.org")
+
+	if addrs := h.LookupStaticHostV4("last.example.org."); len(addrs) != 1 || addrs[0].String() != "127.0.0.2" {
+		t.Errorf("LookupStaticHostV4(last.example.org.) = %v, want [127.0.0.2]", addrs)
+	}
+}
+
+func TestParseLongLineWithComment(t *testing.T) {
+	// A comment that starts beyond the read buffer must still be discarded,
+	// and must not swallow the following line.
+	long := strings.Repeat(" padding.invalid", 1<<16)
+	h := testHostsfile("127.0.0.1 one.example.org" + long + " # two.example.org\n" +
+		"127.0.0.3 after.example.org\n")
+
+	if addrs := h.LookupStaticHostV4("one.example.org."); len(addrs) != 1 || addrs[0].String() != "127.0.0.1" {
+		t.Errorf("LookupStaticHostV4(one.example.org.) = %v, want [127.0.0.1]", addrs)
+	}
+	if addrs := h.LookupStaticHostV4("two.example.org."); len(addrs) != 0 {
+		t.Errorf("LookupStaticHostV4(two.example.org.) = %v, want []", addrs)
+	}
+	if addrs := h.LookupStaticHostV4("after.example.org."); len(addrs) != 1 || addrs[0].String() != "127.0.0.3" {
+		t.Errorf("LookupStaticHostV4(after.example.org.) = %v, want [127.0.0.3]", addrs)
+	}
+}