webfetch checks the status before it reads the body (V-581)
A non-2xx reply was read in full first and only then rejected. Two costs followed. A 500 with a large error page pulled up to MaxBytes off the wire for nothing. An error page over the cap returned ErrTooLarge, which names the size and hides the status the server actually sent. The status is a typed error now. webfetch.StatusError carries the code and unwraps to ErrStatus, so errors.Is keeps working and errors.As reads the number. crawl.StatusError is the same shape on the other side of the seam, and cmd/mavend/crawls.go carries the code across. That removes the string grep in crawl.isServerError, which decided whether a failed robots.txt blocks a crawl by looking for " 50" in an error message it did not own. A reworded error would have turned a 503 robots.txt into permission to crawl. It reads the code now. Two comments corrected. webfetch.HostMatches said the crawler calls it and nothing outside the package does. rss.PlainText said the crawler's extractor goes through it and crawl/extract.go has its own pass. The rss poller parses the feed straight off the byte slice instead of copying a document that can run to a megabyte through a string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user