diff --git a/plugin/hosts/hostsfile.go b/plugin/hosts/hostsfile.go index 2835cf243..8954fb0c2 100644 --- a/plugin/hosts/hostsfile.go +++ b/plugin/hosts/hostsfile.go @@ -12,6 +12,7 @@ import ( "io" "net" "os" + "strconv" "strings" "sync" "time" @@ -161,7 +162,7 @@ func (h *Hostsfile) initInline(inline []string) { return } - h.inline = h.parse(strings.NewReader(strings.Join(inline, "\n"))) + h.inline = h.parseSource(strings.NewReader(strings.Join(inline, "\n")), "Inline hosts entries") } // maxFieldSize bounds the memory used while assembling a single field that @@ -170,13 +171,19 @@ func (h *Hostsfile) initInline(inline []string) { const maxFieldSize = 1024 // Parse reads the hostsfile and populates the byName and addr maps. +func (h *Hostsfile) parse(r io.Reader) *Map { + return h.parseSource(r, "Hosts file "+strconv.Quote(h.path)) +} + +// parseSource is parse with an explicit source name, so that entries inlined in +// the Corefile are not reported as coming from the hosts file. // // 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 { +func (h *Hostsfile) parseSource(r io.Reader, src string) *Map { hmap := newMap() - p := lineParser{h: h, hmap: hmap} + p := lineParser{h: h, hmap: hmap, src: src, line: 1} reader := bufio.NewReader(r) for { @@ -200,10 +207,12 @@ func (h *Hostsfile) parse(r io.Reader) *Map { type lineParser struct { h *Hostsfile hmap *Map + src string // where the entries came from, for diagnostics 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 + line int // 1-based number of the line being parsed, for diagnostics comment bool // the rest of this line is a comment addr net.IP // address of the current line, nil if unusable family int @@ -222,6 +231,7 @@ func (p *lineParser) feed(chunk []byte, last bool) { } if last { p.index, p.comment, p.addr = 0, false, nil + p.line++ } } @@ -279,6 +289,8 @@ func (p *lineParser) emit() { if p.index == 1 { // The first field is the address; without it the line is unusable. if oversized { + log.Errorf("%s, line %d: address longer than %d bytes, dropping the line", + p.src, p.line, maxFieldSize) return } p.addr = parseIP(string(field)) @@ -292,7 +304,12 @@ func (p *lineParser) emit() { } return } - if p.addr == nil || oversized { + if p.addr == nil { + return + } + if oversized { + log.Errorf("%s, line %d: name longer than %d bytes, dropping the name", + p.src, p.line, maxFieldSize) return } p.addName(string(field)) diff --git a/plugin/hosts/hostsfile_test.go b/plugin/hosts/hostsfile_test.go index 30e412313..2bae28661 100644 --- a/plugin/hosts/hostsfile_test.go +++ b/plugin/hosts/hostsfile_test.go @@ -5,7 +5,10 @@ package hosts import ( + "bytes" "fmt" + "io" + golog "log" "net" "os" "reflect" @@ -386,3 +389,129 @@ func TestParseLongLineWithComment(t *testing.T) { t.Errorf("LookupStaticHostV4(after.example.org.) = %v, want [127.0.0.3]", addrs) } } + +func TestParseLogsOversizedField(t *testing.T) { + // #8496 established that a hosts file entry must never be dropped without a + // trace: "the hosts file simply looked shorter than it is, with nothing in + // the log". A field over maxFieldSize is dropped, and when it is the address + // the whole line goes with it, so both cases have to be reported. + var logBuf bytes.Buffer + golog.SetOutput(&logBuf) + defer golog.SetOutput(io.Discard) + + long := strings.Repeat("a", maxFieldSize+1) + h := &Hostsfile{ + Origins: []string{"."}, + hmap: newMap(), + inline: newMap(), + options: newOptions(), + path: "/tmp/hosts.test", + } + h.hmap = h.parse(strings.NewReader( + "127.0.0.1 before.example.org\n" + + "127.0.0.2 " + long + ".example.org\n" + + long + " orphan.example.org\n" + + "127.0.0.4 after.example.org\n")) + + // Controls: the entries around the dropped fields are still parsed. + for _, tc := range []struct{ name, addr string }{ + {"before.example.org.", "127.0.0.1"}, + {"after.example.org.", "127.0.0.4"}, + } { + 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) + } + } + if addrs := h.LookupStaticHostV4("orphan.example.org."); len(addrs) != 0 { + t.Errorf("LookupStaticHostV4(orphan.example.org.) = %v, want []", addrs) + } + + got := logBuf.String() + for _, want := range []string{ + `[ERROR] plugin/hosts: Hosts file "/tmp/hosts.test", line 2:`, + `[ERROR] plugin/hosts: Hosts file "/tmp/hosts.test", line 3:`, + "dropping the line", + "dropping the name", + } { + if !strings.Contains(got, want) { + t.Errorf("Expected log to contain %q, got %q", want, got) + } + } + // One report per dropped field, not one per read: the field on line 3 is + // assembled across several calls to append. + if n := strings.Count(got, "[ERROR] plugin/hosts:"); n != 2 { + t.Errorf("Expected exactly 2 reports, got %d in %q", n, got) + } +} + +func TestParseLogsOversizedFieldSpanningReads(t *testing.T) { + // The line number must survive a field that crosses the read buffer: feed + // runs once per chunk, but only the last chunk of a line ends it. A field + // this long is also reported once, not once per read. + var logBuf bytes.Buffer + golog.SetOutput(&logBuf) + defer golog.SetOutput(io.Discard) + + long := strings.Repeat("a", 5<<20) + h := &Hostsfile{ + Origins: []string{"."}, + hmap: newMap(), + inline: newMap(), + options: newOptions(), + path: "/tmp/hosts.test", + } + h.hmap = h.parse(strings.NewReader( + "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, addr string }{ + {"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) + } + } + + got := logBuf.String() + if want := fmt.Sprintf("line 2: name longer than %d bytes", maxFieldSize); !strings.Contains(got, want) { + t.Errorf("Expected log to contain %q, got %q", want, got) + } + if n := strings.Count(got, "[ERROR] plugin/hosts:"); n != 1 { + t.Errorf("Expected exactly 1 report, got %d in %q", n, got) + } +} + +func TestInitInlineReportsItsOwnSource(t *testing.T) { + // Inline entries are parsed with the same parser but do not come from the + // hosts file, so naming the file in a diagnostic would send the operator to + // the wrong place, at a line number that does not exist there. + var logBuf bytes.Buffer + golog.SetOutput(&logBuf) + defer golog.SetOutput(io.Discard) + + h := &Hostsfile{ + Origins: []string{"."}, + hmap: newMap(), + inline: newMap(), + options: newOptions(), + path: "/tmp/hosts.test", + } + h.initInline([]string{ + "127.0.0.1 first.example.org", + strings.Repeat("a", maxFieldSize+1) + " orphan.example.org", + }) + + if addrs := h.inline.name4["first.example.org."]; len(addrs) != 1 { + t.Errorf("inline name4[first.example.org.] = %v, want one address", addrs) + } + + got := logBuf.String() + if want := "Inline hosts entries, line 2: address longer"; !strings.Contains(got, want) { + t.Errorf("Expected log to contain %q, got %q", want, got) + } + if strings.Contains(got, "/tmp/hosts.test") { + t.Errorf("Inline entries reported as coming from the hosts file: %q", got) + } +}