Merge the web sources sweep (#243 )

Three of the four packages were already clean on the brief's priorities. The brief predicted missing timeouts and unbounded reads; websearch already had a status check, a deferred close, a 4 MiB limit, an 8s total and a 1.5s connect cap on a cloned transport. The one bug with reach was a string grep across a package boundary. crawl.isServerError decided whether a failed robots.txt blocks a crawl by scanning err.Error() for " 50", " 51", " 52" and " 53", in a message built two packages away. Rewording that message would silently turn a 503 robots.txt into permission to crawl, which the surrounding comment says must never happen. Both packages now carry a typed StatusError that unwraps to the existing sentinel, so errors.Is is unchanged, and isServerError reads a number. webfetch checked the status after reading the body, the same shape the weather sweep found. A 500 pulled its error page up to MaxBytes off the wire, and an error page over the cap returned ErrTooLarge, naming the size and hiding the status. rss.Parse copied a feed document that can reach a megabyte through strings.NewReader(string(...)). Two comments claimed callers that do not exist. The privacy invariant holds across all four. None of them can read the store. rss.Ranker is the one seam that could carry notes outward, it is nil in the daemon, and its doc states the constraint. Every regex here is over structured input. (V-581)
webfetch checks the status before it reads the body (V-581)
2026-08-06 03:13:46 +04:00 · 2026-08-06 03:12:59 +04:00
7 changed files with 71 additions and 17 deletions
@@ -173,6 +173,13 @@ func (a *crawlFetcher) Get(ctx context.Context, u string) (*crawl.Response, erro
 		case errors.Is(err, webfetch.ErrBlocked), errors.Is(err, webfetch.ErrPrivate), errors.Is(err, webfetch.ErrScheme):
 			return nil, fmt.Errorf("%w: %v", crawl.ErrFetchRefused, err)
 		case errors.Is(err, webfetch.ErrStatus):
+			// Carry the code across the seam. The crawler needs to tell a 5xx
+			// from a 404 to decide what a failed robots.txt means, and it must
+			// not learn that by reading this sentence.
+			var se *webfetch.StatusError
+			if errors.As(err, &se) {
+				return nil, &crawl.StatusError{Code: se.Code}
+			}
 			return nil, fmt.Errorf("%w: %v", crawl.ErrFetchStatus, err)
 		}
 		return nil, err
@@ -57,6 +57,16 @@ var (
 	ErrFetchStatus = errors.New("crawl: the server answered with an error status")
 )

+// StatusError is ErrFetchStatus with the code the server actually sent. The
+// adapter builds it; isServerError reads Code rather than the message, so a
+// reworded error can no longer turn a 503 robots.txt into permission to crawl.
+type StatusError struct{ Code int }
+
+func (e *StatusError) Error() string {
+	return fmt.Sprintf("crawl: the server answered with status %d", e.Code)
+}
+func (e *StatusError) Unwrap() error { return ErrFetchStatus }
+
 // Fetcher is the guarded HTTP door (internal/webfetch adapted by the daemon). An
 // interface so this package constructs no http.Client of its own and can be
 // tested without a network.
@@ -233,16 +243,15 @@ func (c *Crawler) markFetched(host string) {
 	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.
+// isServerError — a 5xx rather than any other non-2xx. A status the adapter
+// could not recover reads as 0 and is not a server error, which keeps the
+// standard's "404 means allow" as the default for an unknown.
 func isServerError(err error) bool {
-	s := err.Error()
-	for _, code := range []string{" 50", " 51", " 52", " 53"} {
-		if strings.Contains(s, code) {
-			return true
-		}
+	var se *StatusError
+	if !errors.As(err, &se) {
+		return false
 	}
-	return false
+	return se.Code >= 500 && se.Code <= 599
 }

 // Hash is the dedup key for a crawl result: the sha256 of the extracted text,
@@ -79,7 +79,7 @@ func TestPage_ABrokenRobotsServerIsNotPermissionToCrawl(t *testing.T) {
 	// 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)},
+		errs:  map[string]error{"https://example.org/robots.txt": &StatusError{Code: 503}},
 	}
 	c := New(f, Config{UserAgent: "Maven/1.0"})
 	if _, err := c.Page(context.Background(), "https://example.org/a"); !errors.Is(err, ErrFetchStatus) {
@@ -173,8 +173,9 @@ var (
 )

 // PlainText strips markup and decodes entities — feed summaries are HTML, and
-// what reaches a note (and possibly the TTS) must be text. Exported because the
-// crawler's extractor needs exactly this on a bigger input.
+// what reaches a note (and possibly the TTS) must be text. Exported so a caller
+// holding raw feed markup can reduce it the same way; crawl/extract.go does the
+// bigger job on a whole document and does not go through here.
 func PlainText(s string) string {
 	s = scriptRE.ReplaceAllString(s, " ")
 	s = tagRE.ReplaceAllString(s, " ")
@@ -1,6 +1,7 @@
 package rss

 import (
+	"bytes"
 	"context"
 	"fmt"
 	"log"
@@ -164,7 +165,9 @@ func (p *Poller) PollFeed(ctx context.Context, f FeedConfig, now time.Time) (int
 	if err != nil {
 		return 0, err
 	}
-	feed, err := Parse(strings.NewReader(string(body.Bytes)))
+	// bytes.NewReader and not strings.NewReader(string(…)): the latter copied a
+	// feed document that can run to a megabyte, for nothing.
+	feed, err := Parse(bytes.NewReader(body.Bytes))
 	if err != nil {
 		return 0, err
 	}
@@ -61,6 +61,14 @@ var (
 	ErrStatus    = errors.New("webfetch: non-2xx status")
 )

+// StatusError is a non-2xx reply, carrying the code. It unwraps to ErrStatus,
+// so errors.Is keeps working, and it exists so a caller can tell a 404 from a
+// 503 with errors.As instead of grepping the message for digits.
+type StatusError struct{ Code int }
+
+func (e *StatusError) Error() string { return fmt.Sprintf("webfetch: non-2xx status: %d", e.Code) }
+func (e *StatusError) Unwrap() error { return ErrStatus }
+
 // Config are the limits. Every zero value means "the default above", so
 // Config{} is safe; the only field that changes behaviour by being empty is
 // AllowHosts (empty ⇒ any public host that is not denied).
@@ -232,6 +240,13 @@ func (f *Fetcher) do(ctx context.Context, method, rawURL string, body []byte, hd
 	}
 	defer resp.Body.Close()

+	// Status first, body second. A server that answered 500 has no body worth
+	// reading, and reading it anyway cost up to MaxBytes off the wire and
+	// reported an oversized error page as ErrTooLarge, which names the wrong
+	// cause. The body is closed either way by the defer above.
+	if resp.StatusCode < 200 || resp.StatusCode > 299 {
+		return nil, &StatusError{Code: resp.StatusCode}
+	}
 	respBody, err := io.ReadAll(io.LimitReader(resp.Body, f.cfg.MaxBytes+1))
 	if err != nil {
 		return nil, err
@@ -239,9 +254,6 @@ func (f *Fetcher) do(ctx context.Context, method, rawURL string, body []byte, hd
 	if int64(len(respBody)) > f.cfg.MaxBytes {
 		return nil, fmt.Errorf("%w (%d bytes)", ErrTooLarge, f.cfg.MaxBytes)
 	}
-	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		return nil, fmt.Errorf("%w: %d", ErrStatus, resp.StatusCode)
-	}
 	out := &Response{
 		URL:         resp.Request.URL.String(),
 		Status:      resp.StatusCode,
@@ -307,8 +319,9 @@ func (f *Fetcher) waitTurn(ctx context.Context, host string) error {
 }

 // HostMatches reports whether host equals one of pats or is a subdomain of one.
-// Exported because the crawler applies the same rule to links it decides not to
-// follow, before it ever builds a request.
+// Exported so anything that wants to apply the same allow/deny rule to a link
+// before building a request reads it from here rather than reimplementing it.
+// Nothing outside this package calls it today.
 func HostMatches(host string, pats []string) bool {
 	host = strings.ToLower(strings.TrimSuffix(host, "."))
 	for _, p := range pats {
@@ -1,6 +1,7 @@
 package webfetch

 import (
+	"bytes"
 	"context"
 	"errors"
 	"io"
@@ -212,6 +213,26 @@ func TestNon2xxIsAnError(t *testing.T) {
 	}
 }

+func TestNon2xxCarriesTheCodeAndBeatsTheSizeCap(t *testing.T) {
+	// A big error page used to be read in full and reported as ErrTooLarge,
+	// which names the size and hides the 503. The status is checked first now,
+	// and the code survives for a caller that has to tell 5xx from 404.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusServiceUnavailable)
+		w.Write(bytes.Repeat([]byte("x"), 5000))
+	}))
+	defer srv.Close()
+	f := testFetcher(t, Config{MaxBytes: 100})
+	_, err := f.Get(context.Background(), srv.URL)
+	if !errors.Is(err, ErrStatus) || errors.Is(err, ErrTooLarge) {
+		t.Fatalf("error = %v, want ErrStatus and not ErrTooLarge", err)
+	}
+	var se *StatusError
+	if !errors.As(err, &se) || se.Code != http.StatusServiceUnavailable {
+		t.Fatalf("error = %v, want a StatusError carrying 503", err)
+	}
+}
+
 func TestUserAgentIsSent(t *testing.T) {
 	got := make(chan string, 1)
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {