crawl: stop letting a watch widen on-demand reading, and honour Crawl-delay
The on-demand crawler was built over allow_hosts plus every watched host. webfetch reads a non-empty allow list as these and nothing else, so a config with one watch and no allow_hosts at all silently narrowed on-demand reading to the watched site. Every other url he pasted came back as a flat refusal with nothing in the log to explain it. The two crawlers now take two host lists from one crawlHosts helper. Crawl-delay was parsed into Rules and never read. The only pacing was the fetcher's flat one request per host per second, which cannot express what a site asked for, and deploy/README claimed the field was honoured. Page now waits it out between the robots fetch and the page fetch, and a delay longer than the turn fails the read instead of hanging it. A robots.txt that failed was treated as no rules, so a site whose server was having a bad minute became a site with no restrictions. A 5xx now refuses the crawl. A 404 still means unrestricted, which is what the standard says. The refusal check matched substrings of webfetch's message text from a package that cannot import webfetch, so a reworded error would have silently turned into a robots verdict. internal/crawl now exports ErrFetchRefused and ErrFetchStatus and the adapter in cmd/mavend maps the webfetch sentinels onto them. Robots group selection picks the longest matching agent prefix instead of the first one in file order. queryWeb passed a claim it could not serve when no crawler was configured, so an unconfigured deployment answered a web question with an apology instead of falling through to the model. Found in review of #67.
This commit is contained in:
kami
@@ -940,8 +940,13 @@ type CrawlConfig struct {
|
||||
Interval Duration `json:"interval,omitempty"`
|
||||
|
||||
// AllowHosts — when set, the ONLY hosts the crawler may reach (subdomains
|
||||
// included). Watched pages' own hosts are added automatically. Setting this
|
||||
// is how "she may read the arch wiki and nothing else" is expressed.
|
||||
// included). Setting this is how "she may read the arch wiki and nothing
|
||||
// else" is expressed.
|
||||
//
|
||||
// A watched page's own host is reachable by the scheduled crawler whether
|
||||
// or not it is listed here, because configuring a watch is already saying
|
||||
// she may read it. That does NOT extend to on-demand reading: a watch is
|
||||
// not an allowlist entry for pages he pastes.
|
||||
AllowHosts []string `json:"allow_hosts,omitempty"`
|
||||
|
||||
// DenyHosts — never reachable, checked first. Private addresses do not need
|
||||
|
||||
+125
-45
@@ -1,13 +1,21 @@
|
||||
// Package crawl reads a web page: fetch, robots check, HTML to text.
|
||||
//
|
||||
// It is the LAST place Maven looks for an answer, and that ordering is the whole
|
||||
// design. "Never phones home" is deprecated, but what replaced it puts local
|
||||
// sources first: the resident model, then his own memory, then the Kiwix ZIMs on
|
||||
// the box (internal/kiwix), and only then the network. A local read costs
|
||||
// nothing and leaks nothing; a fetch costs a round-trip and puts a URL in
|
||||
// someone's access log. So this package exists to be the fallback, not the
|
||||
// front door — see the querySources chain in cmd/mavend/actions_query.go for
|
||||
// where it actually sits.
|
||||
// It is the last LOCAL-FIRST step, and the ordering is the whole design.
|
||||
// "Never phones home" is deprecated, but what replaced it puts local sources
|
||||
// first. Where this actually sits in querySources (cmd/mavend/actions_query.go):
|
||||
// after his memory and after his notes, and BEFORE the model answers from what
|
||||
// it remembers. Not after the model, which is what this comment used to claim.
|
||||
//
|
||||
// That position is deliberate. A fetch only happens where he named a URL out
|
||||
// loud, and a named URL is an instruction, not a guess; letting a 1.7B answer
|
||||
// about a page it cannot read is how a small model invents contents. Kiwix
|
||||
// (internal/kiwix) is not in the chain yet, so nothing here describes it.
|
||||
//
|
||||
// A page's text is written by whoever owns the page. It reaches PhraseQuery as
|
||||
// context beside his question, and for a watch it becomes a note. It cannot
|
||||
// reach a tool or an act — the query path executes nothing — but it can steer
|
||||
// what she says, which is the same trust level as a mail body and lower than
|
||||
// anything he said himself.
|
||||
//
|
||||
// What never leaves the box: his notes, his facts, the persona block, the
|
||||
// conversation history. Only the URL is requested and, for the on-demand path,
|
||||
@@ -27,6 +35,7 @@ import (
|
||||
"fmt"
|
||||
"net/url"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -34,6 +43,18 @@ import (
|
||||
var (
|
||||
ErrRobots = errors.New("crawl: robots.txt disallows this path")
|
||||
ErrNotHTML = errors.New("crawl: response is not html or text")
|
||||
|
||||
// ErrFetchRefused — the fetcher would not go: a denied host, a private
|
||||
// address, a scheme that is not http(s). The adapter that owns both
|
||||
// packages (cmd/mavend/crawls.go) maps webfetch's sentinels onto this one,
|
||||
// so this package tells "off limits" from "no robots.txt here" without
|
||||
// importing webfetch and without matching on message text.
|
||||
ErrFetchRefused = errors.New("crawl: the fetcher refused this url")
|
||||
|
||||
// ErrFetchStatus — the server answered, badly (5xx, and anything else
|
||||
// non-2xx). Separate from ErrFetchRefused because robots treats them
|
||||
// differently: a broken server is not permission to crawl.
|
||||
ErrFetchStatus = errors.New("crawl: the server answered with an error status")
|
||||
)
|
||||
|
||||
// Fetcher is the guarded HTTP door (internal/webfetch adapted by the daemon). An
|
||||
@@ -69,8 +90,20 @@ type Crawler struct {
|
||||
fetch Fetcher
|
||||
cfg Config
|
||||
robots *robotsCache
|
||||
|
||||
// mu guards last, the per-host time of the previous fetch. It is what makes
|
||||
// Crawl-delay real: the fetcher's own limiter is a flat one request per
|
||||
// host per second and knows nothing about what a site asked for.
|
||||
mu sync.Mutex
|
||||
last map[string]time.Time
|
||||
}
|
||||
|
||||
// robotsTimeout — the robots fetch gets its own, shorter deadline. It shares the
|
||||
// caller's budget with the page fetch (30s for the on-demand path, against a 20s
|
||||
// default fetch timeout each), so a slow robots.txt used to eat the page's half
|
||||
// and he heard "не получилось прочитать страницу" about a site that was fine.
|
||||
const robotsTimeout = 8 * time.Second
|
||||
|
||||
// New builds a crawler. Returns nil when there is no fetcher, which is how the
|
||||
// daemon expresses "crawling is off unless configured".
|
||||
func New(fetch Fetcher, cfg Config) *Crawler {
|
||||
@@ -89,7 +122,7 @@ func New(fetch Fetcher, cfg Config) *Crawler {
|
||||
if cfg.Now == nil {
|
||||
cfg.Now = time.Now
|
||||
}
|
||||
return &Crawler{fetch: fetch, cfg: cfg, robots: newRobotsCache(cfg.RobotsTTL)}
|
||||
return &Crawler{fetch: fetch, cfg: cfg, robots: newRobotsCache(cfg.RobotsTTL), last: map[string]time.Time{}}
|
||||
}
|
||||
|
||||
// Page fetches rawURL and returns its text. It checks robots.txt first and
|
||||
@@ -99,14 +132,22 @@ func (c *Crawler) Page(ctx context.Context, rawURL string) (Page, error) {
|
||||
if err != nil {
|
||||
return Page{}, fmt.Errorf("crawl: bad url %q: %w", rawURL, err)
|
||||
}
|
||||
ok, err := c.allowed(ctx, u)
|
||||
rules, err := c.rulesFor(ctx, u)
|
||||
if err != nil {
|
||||
return Page{}, err
|
||||
}
|
||||
if !ok {
|
||||
path := u.EscapedPath()
|
||||
if u.RawQuery != "" {
|
||||
path += "?" + u.RawQuery
|
||||
}
|
||||
if !rules.Allowed(path) {
|
||||
return Page{}, fmt.Errorf("%w: %s", ErrRobots, u.Path)
|
||||
}
|
||||
if err := c.waitCrawlDelay(ctx, u.Host, rules.Delay); err != nil {
|
||||
return Page{}, err
|
||||
}
|
||||
resp, err := c.fetch.Get(ctx, u.String())
|
||||
c.markFetched(u.Host)
|
||||
if err != nil {
|
||||
return Page{}, err
|
||||
}
|
||||
@@ -120,49 +161,88 @@ func (c *Crawler) Page(ctx context.Context, rawURL string) (Page, error) {
|
||||
return Extract(resp.URL, resp.Body, c.cfg.MaxRunes), nil
|
||||
}
|
||||
|
||||
// allowed consults robots.txt for u's host, reading it at most once per TTL.
|
||||
// rulesFor consults robots.txt for u's host, reading it at most once per TTL.
|
||||
//
|
||||
// A robots.txt that cannot be fetched (404, a timeout, a blocked host) means
|
||||
// allow, per the standard. The one thing that is NOT fail-open is an explicit
|
||||
// Disallow.
|
||||
func (c *Crawler) allowed(ctx context.Context, u *url.URL) (bool, error) {
|
||||
// A robots.txt that is not there (404) means allow, per the standard. What does
|
||||
// NOT mean allow: a server that answered with an error. The standard asks for
|
||||
// the opposite there, and "the site is broken, so crawl it" is the wrong way to
|
||||
// resolve an unknown.
|
||||
func (c *Crawler) rulesFor(ctx context.Context, u *url.URL) (Rules, error) {
|
||||
host := u.Host
|
||||
now := c.cfg.Now()
|
||||
rules, ok := c.robots.get(host, now)
|
||||
if !ok {
|
||||
robotsURL := u.Scheme + "://" + host + "/robots.txt"
|
||||
resp, err := c.fetch.Get(ctx, robotsURL)
|
||||
switch {
|
||||
case err != nil:
|
||||
// Note what is NOT swallowed: a refusal from the guarded fetcher.
|
||||
// If webfetch says this host is denied or private, the page fetch
|
||||
// would fail the same way, and reporting the real reason beats
|
||||
// reporting a robots verdict we never got.
|
||||
if isFatalFetchError(err) {
|
||||
return false, err
|
||||
}
|
||||
rules = Rules{}
|
||||
default:
|
||||
rules = ParseRobots(string(resp.Body), c.cfg.UserAgent)
|
||||
}
|
||||
c.robots.put(host, rules, now)
|
||||
if ok {
|
||||
return rules, nil
|
||||
}
|
||||
path := u.EscapedPath()
|
||||
if u.RawQuery != "" {
|
||||
path += "?" + u.RawQuery
|
||||
robotsURL := u.Scheme + "://" + host + "/robots.txt"
|
||||
rctx, cancel := context.WithTimeout(ctx, robotsTimeout)
|
||||
defer cancel()
|
||||
resp, err := c.fetch.Get(rctx, robotsURL)
|
||||
c.markFetched(host)
|
||||
switch {
|
||||
case err == nil:
|
||||
rules = ParseRobots(string(resp.Body), c.cfg.UserAgent)
|
||||
case errors.Is(err, ErrFetchRefused):
|
||||
// Not swallowed: if the fetcher says this host is denied or private,
|
||||
// the page fetch would fail the same way, and the real reason beats a
|
||||
// robots verdict we never got.
|
||||
return Rules{}, err
|
||||
case errors.Is(err, ErrFetchStatus) && isServerError(err):
|
||||
return Rules{}, fmt.Errorf("%w: robots.txt at %s could not be read", ErrFetchStatus, host)
|
||||
default:
|
||||
rules = Rules{}
|
||||
}
|
||||
return rules.Allowed(path), nil
|
||||
c.robots.put(host, rules, now)
|
||||
return rules, nil
|
||||
}
|
||||
|
||||
// isFatalFetchError — a fetch failure that means "this host is off limits"
|
||||
// rather than "there is no robots.txt here". The sentinel set is webfetch's, but
|
||||
// this package must not import it (the interface exists precisely so it does
|
||||
// not), so the check is on the message. Ugly and honest: the alternative is a
|
||||
// dependency inversion for two strings.
|
||||
func isFatalFetchError(err error) bool {
|
||||
// waitCrawlDelay honours a Crawl-delay the site asked for. The fetcher's flat
|
||||
// one-per-host-per-second is the floor and cannot express "30 seconds"; without
|
||||
// this, deploy/README's claim that Crawl-delay is honoured was false.
|
||||
//
|
||||
// It waits on ctx, so a delay longer than the caller's budget fails the read
|
||||
// rather than blocking a turn. That is the honest outcome: a site that wants a
|
||||
// minute between requests is not a site to answer a voice question from.
|
||||
func (c *Crawler) waitCrawlDelay(ctx context.Context, host string, delay time.Duration) error {
|
||||
if delay <= 0 {
|
||||
return nil
|
||||
}
|
||||
c.mu.Lock()
|
||||
last, ok := c.last[host]
|
||||
c.mu.Unlock()
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
wait := delay - c.cfg.Now().Sub(last)
|
||||
if wait <= 0 {
|
||||
return nil
|
||||
}
|
||||
t := time.NewTimer(wait)
|
||||
defer t.Stop()
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return fmt.Errorf("crawl: %s asks for %s between requests, longer than this read has: %w", host, delay, ctx.Err())
|
||||
case <-t.C:
|
||||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
func (c *Crawler) markFetched(host string) {
|
||||
c.mu.Lock()
|
||||
c.last[host] = c.cfg.Now()
|
||||
c.mu.Unlock()
|
||||
}
|
||||
|
||||
// isServerError — a 5xx rather than any other non-2xx. The adapter formats the
|
||||
// status into the message, which is the only place it survives.
|
||||
func isServerError(err error) bool {
|
||||
s := err.Error()
|
||||
return strings.Contains(s, "not allowed") || strings.Contains(s, "private address") ||
|
||||
strings.Contains(s, "only http and https")
|
||||
for _, code := range []string{" 50", " 51", " 52", " 53"} {
|
||||
if strings.Contains(s, code) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// Hash is the dedup key for a crawl result: the sha256 of the extracted text,
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
package crawl
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// timedFetcher records when each request was made, so a test can assert a wait
|
||||
// actually happened rather than that a field was parsed.
|
||||
type timedFetcher struct {
|
||||
pages map[string]Response
|
||||
errs map[string]error
|
||||
at []time.Time
|
||||
urls []string
|
||||
}
|
||||
|
||||
func (f *timedFetcher) Get(_ context.Context, u string) (*Response, error) {
|
||||
f.at = append(f.at, time.Now())
|
||||
f.urls = append(f.urls, u)
|
||||
if err, ok := f.errs[u]; ok {
|
||||
return nil, err
|
||||
}
|
||||
r, ok := f.pages[u]
|
||||
if !ok {
|
||||
return nil, errors.New("http 404: no such page")
|
||||
}
|
||||
if r.URL == "" {
|
||||
r.URL = u
|
||||
}
|
||||
if r.ContentType == "" {
|
||||
r.ContentType = "text/html"
|
||||
}
|
||||
return &r, nil
|
||||
}
|
||||
|
||||
func TestPage_HonoursCrawlDelay(t *testing.T) {
|
||||
// deploy/README says Crawl-delay is honoured. It was parsed into Rules and
|
||||
// never read: the only pacing was the fetcher's flat one request per host
|
||||
// per second, which cannot express what a site asked for.
|
||||
const delay = 120 * time.Millisecond
|
||||
f := &timedFetcher{pages: map[string]Response{
|
||||
"https://example.org/robots.txt": {Body: []byte(fmt.Sprintf("User-agent: *\nCrawl-delay: %.3f\n", delay.Seconds())), ContentType: "text/plain"},
|
||||
"https://example.org/a": {Body: []byte("<html><body>a</body></html>")},
|
||||
}}
|
||||
c := New(f, Config{UserAgent: "Maven/1.0"})
|
||||
if _, err := c.Page(context.Background(), "https://example.org/a"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(f.at) != 2 {
|
||||
t.Fatalf("requests = %v; want robots.txt then the page", f.urls)
|
||||
}
|
||||
if gap := f.at[1].Sub(f.at[0]); gap < delay {
|
||||
t.Errorf("the page was fetched %s after robots.txt; the site asked for %s", gap, delay)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPage_ACrawlDelayLongerThanTheTurnFailsInsteadOfBlocking(t *testing.T) {
|
||||
f := &timedFetcher{pages: map[string]Response{
|
||||
"https://example.org/robots.txt": {Body: []byte("User-agent: *\nCrawl-delay: 30\n"), ContentType: "text/plain"},
|
||||
"https://example.org/a": {Body: []byte("<html><body>a</body></html>")},
|
||||
}}
|
||||
c := New(f, Config{UserAgent: "Maven/1.0"})
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond)
|
||||
defer cancel()
|
||||
if _, err := c.Page(ctx, "https://example.org/a"); err == nil {
|
||||
t.Fatal("a 30-second Crawl-delay was ignored inside a turn that cannot wait that long")
|
||||
}
|
||||
if len(f.urls) != 1 {
|
||||
t.Errorf("requests = %v; the page must not be fetched before the wait it refused", f.urls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPage_ABrokenRobotsServerIsNotPermissionToCrawl(t *testing.T) {
|
||||
// A 404 means unrestricted, per the standard. A 500 does not: the standard
|
||||
// asks for the opposite, and "the site is broken, so read it" is the wrong
|
||||
// way to resolve an unknown.
|
||||
f := &timedFetcher{
|
||||
pages: map[string]Response{"https://example.org/a": {Body: []byte("<html><body>a</body></html>")}},
|
||||
errs: map[string]error{"https://example.org/robots.txt": fmt.Errorf("%w: 503", ErrFetchStatus)},
|
||||
}
|
||||
c := New(f, Config{UserAgent: "Maven/1.0"})
|
||||
if _, err := c.Page(context.Background(), "https://example.org/a"); !errors.Is(err, ErrFetchStatus) {
|
||||
t.Fatalf("Page over a 503 robots.txt = %v; want a refusal", err)
|
||||
}
|
||||
if len(f.urls) != 1 {
|
||||
t.Errorf("requests = %v; the page was read anyway", f.urls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPage_AFetcherRefusalIsReportedAsItself(t *testing.T) {
|
||||
// The old check matched three substrings of webfetch's message from a
|
||||
// package that cannot import webfetch. The sentinel is mapped by the
|
||||
// adapter that owns both (cmd/mavend/crawls.go).
|
||||
f := &timedFetcher{errs: map[string]error{
|
||||
"https://example.org/robots.txt": fmt.Errorf("%w: host is not allowed", ErrFetchRefused),
|
||||
}}
|
||||
c := New(f, Config{UserAgent: "Maven/1.0"})
|
||||
if _, err := c.Page(context.Background(), "https://example.org/a"); !errors.Is(err, ErrFetchRefused) {
|
||||
t.Fatalf("Page = %v; want the fetcher's own refusal, not a robots verdict", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseRobots_MostSpecificAgentWinsRegardlessOfOrder(t *testing.T) {
|
||||
body := "User-agent: maven\nDisallow: /private\n\nUser-agent: mav\nDisallow: /\n"
|
||||
r := ParseRobots(body, "maven/1.0")
|
||||
if !r.Allowed("/public") {
|
||||
t.Error("the shorter agent group won by file order; the longer prefix is the more specific match")
|
||||
}
|
||||
if r.Allowed("/private") {
|
||||
t.Error("the group that names us was not applied")
|
||||
}
|
||||
}
|
||||
@@ -27,7 +27,8 @@ type Rules struct {
|
||||
disallow []string
|
||||
// Delay is Crawl-delay in seconds when the group named one, 0 otherwise.
|
||||
// The fetcher's own per-host rate limit is the floor; this can only make
|
||||
// Maven slower, never faster.
|
||||
// Maven slower, never faster. Enforced in Crawler.waitCrawlDelay — the
|
||||
// fetcher's limiter is flat and cannot express what a site asked for.
|
||||
Delay time.Duration
|
||||
}
|
||||
|
||||
@@ -104,7 +105,11 @@ func ParseRobots(body string, agent string) Rules {
|
||||
}
|
||||
}
|
||||
|
||||
// Most specific wins, and specificity is the LENGTH of the matching agent
|
||||
// string, not the file order. Two groups naming "mav" and "maven" used to be
|
||||
// resolved by whichever came last in the file.
|
||||
var star, exact *group
|
||||
best := 0
|
||||
for i := range groups {
|
||||
for _, a := range groups[i].agents {
|
||||
if a == "*" && star == nil {
|
||||
@@ -112,8 +117,8 @@ func ParseRobots(body string, agent string) Rules {
|
||||
}
|
||||
// A robots.txt names "maven", we send "Maven/1.0 (…)": match on
|
||||
// prefix, which is how every crawler reads this field.
|
||||
if a != "*" && a != "" && strings.HasPrefix(agent, a) {
|
||||
exact = &groups[i]
|
||||
if a != "*" && a != "" && strings.HasPrefix(agent, a) && len(a) > best {
|
||||
best, exact = len(a), &groups[i]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user