Reject malformed search bodies and report unranked candidates
CI / ci (push) Successful in 1m35s
CI / ci (pull_request) Successful in 1m35s

SearXNG now requires a results array and Brave requires "type": "search"
with a results array in any web object, so `{}` is ErrSearchResponse
instead of an empty search. Rank no longer drops failed candidates: it
returns the ranked ones with ErrCandidatesUnranked joined with each
unranked candidate's URL and reason, or ErrNoUsableRecipe when none rank.
Adds an HTTPS ranking test through the SSRF-safe Fetcher.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-05 13:27:07 -04:00
co-authored by Claude Opus 5.5
parent 26928b538b
commit 01523f634b
5 changed files with 177 additions and 22 deletions
+11 -5
View File
@@ -248,15 +248,21 @@ automatic** (D-021).
key decrypted from `recipe_search_settings.api_key_ct` key decrypted from `recipe_search_settings.api_key_ct`
(`recipes.NewSearchProvider`). Redirects are not followed; 401/403 is (`recipes.NewSearchProvider`). Redirects are not followed; 401/403 is
`ErrSearchRefused`, 429 `ErrSearchRateLimited`, other non-2xx a `ErrSearchRefused`, 429 `ErrSearchRateLimited`, other non-2xx a
`SearchStatusError`, and a body that is not the provider's JSON, over `SearchStatusError`, and a body that is not the provider's JSON (SearXNG
1 MB or with a result lacking an absolute http(s) URL `ErrSearchResponse`. without a `results` array; Brave without `"type": "search"` or with a
`web` object lacking `results` — Brave omits `web` when nothing matched),
over 1 MB or with a result lacking an absolute http(s) URL
`ErrSearchResponse`.
- Ranking (`recipes.Rank`) fetches and extracts the top 5 candidates - Ranking (`recipes.Rank`) fetches and extracts the top 5 candidates
concurrently, rejects any whose recipe lacks ingredients or an image, and concurrently, rejects any whose recipe lacks ingredients or an image, and
orders the rest by the token similarity of the recipe's title to the dish orders the rest by the token similarity of the recipe's title to the dish
name (`harness.Similarity`, 0–1) plus a quarter for each of ingredients, name (`harness.Similarity`, 0–1) plus a quarter for each of ingredients,
image, steps and total minutes; ties keep search order. When every image, steps and total minutes; ties keep search order. No unranked
candidate is rejected the error is `ErrNoUsableRecipe` joined with each candidate is dropped silently: when some are ranked, they are returned
reason. with `ErrCandidatesUnranked`; when none are, the error is
`ErrNoUsableRecipe`. Either is joined with each unranked candidate's URL
and reason (fetch failure, no valid recipe, missing ingredients or
image), so the caller decides per item what a failure means.
- Photos are cached locally (`photos`) and processed with - Photos are cached locally (`photos`) and processed with
`golang.org/x/image`. `golang.org/x/image`.
+20 -6
View File
@@ -20,9 +20,13 @@ var (
ErrNoIngredients = errors.New("recipes: recipe has no ingredients") ErrNoIngredients = errors.New("recipes: recipe has no ingredients")
// ErrNoImage rejects a candidate whose recipe has no image. // ErrNoImage rejects a candidate whose recipe has no image.
ErrNoImage = errors.New("recipes: recipe has no image") ErrNoImage = errors.New("recipes: recipe has no image")
// ErrNoUsableRecipe is returned when Rank rejects every candidate; it // ErrNoUsableRecipe is returned when Rank ranks no candidate; it is
// wraps each candidate's reason. // joined with each candidate's reason.
ErrNoUsableRecipe = errors.New("recipes: no candidate holds a usable recipe") ErrNoUsableRecipe = errors.New("recipes: no candidate holds a usable recipe")
// ErrCandidatesUnranked is returned, alongside the ranked candidates,
// when Rank ranks some candidates but not all; it is joined with each
// unranked candidate's reason.
ErrCandidatesUnranked = errors.New("recipes: some candidates were not ranked")
) )
// PageFetcher fetches a candidate's page; *Fetcher satisfies it. // PageFetcher fetches a candidate's page; *Fetcher satisfies it.
@@ -42,8 +46,15 @@ type Ranked struct {
// Rank fetches and extracts the first RankDepth candidates concurrently and // Rank fetches and extracts the first RankDepth candidates concurrently and
// returns those whose recipe has ingredients and an image, highest Score // returns those whose recipe has ingredients and an image, highest Score
// first; equal scores keep search order. When every candidate is rejected // first; equal scores keep search order.
// the error is ErrNoUsableRecipe joined with each rejection's reason. //
// Every candidate that is not ranked — its fetch failed, its page holds no
// valid recipe, or the recipe lacks ingredients or an image — is reported,
// never dropped silently: when some are ranked the error is
// ErrCandidatesUnranked, returned together with the ranked ones; when none
// are it is ErrNoUsableRecipe with no results. Either is joined with each
// unranked candidate's URL and reason. A cancelled ctx returns only its
// error.
func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([]Ranked, error) { func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([]Ranked, error) {
if len(cands) > RankDepth { if len(cands) > RankDepth {
cands = cands[:RankDepth] cands = cands[:RankDepth]
@@ -68,7 +79,7 @@ func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([
} }
var ranked []Ranked var ranked []Ranked
reasons := []error{ErrNoUsableRecipe} var reasons []error
for i, o := range outcomes { for i, o := range outcomes {
if o.err != nil { if o.err != nil {
reasons = append(reasons, fmt.Errorf("%s: %w", cands[i].URL, o.err)) reasons = append(reasons, fmt.Errorf("%s: %w", cands[i].URL, o.err))
@@ -77,9 +88,12 @@ func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([
ranked = append(ranked, o.ranked) ranked = append(ranked, o.ranked)
} }
if len(ranked) == 0 { if len(ranked) == 0 {
return nil, errors.Join(reasons...) return nil, errors.Join(append([]error{ErrNoUsableRecipe}, reasons...)...)
} }
sort.SliceStable(ranked, func(a, b int) bool { return ranked[a].Score > ranked[b].Score }) sort.SliceStable(ranked, func(a, b int) bool { return ranked[a].Score > ranked[b].Score })
if len(reasons) > 0 {
return ranked, errors.Join(append([]error{ErrCandidatesUnranked}, reasons...)...)
}
return ranked, nil return ranked, nil
} }
+98 -2
View File
@@ -4,6 +4,8 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"net/http"
"strings"
"sync" "sync"
"testing" "testing"
) )
@@ -66,8 +68,11 @@ func TestRankOrder(t *testing.T) {
{URL: "https://f.example/toodeep"}, {URL: "https://f.example/toodeep"},
} }
got, err := Rank(context.Background(), f, "Chicken curry", cands) got, err := Rank(context.Background(), f, "Chicken curry", cands)
if err != nil { if !errors.Is(err, ErrCandidatesUnranked) || !errors.Is(err, ErrNoImage) || !errors.Is(err, ErrNoIngredients) {
t.Fatal(err) t.Errorf("err = %v, want ErrCandidatesUnranked with ErrNoImage and ErrNoIngredients", err)
}
if errors.Is(err, ErrNoUsableRecipe) {
t.Errorf("err = %v, want not ErrNoUsableRecipe when some are ranked", err)
} }
want := []struct { want := []struct {
url string url string
@@ -119,6 +124,29 @@ func TestRankTiesKeepSearchOrder(t *testing.T) {
} }
} }
// TestRankReportsFetchFailure checks a failed fetch is reported with the
// candidates that were ranked, never swallowed.
func TestRankReportsFetchFailure(t *testing.T) {
f := &fakeFetcher{pages: map[string][]byte{
"https://a.example/1": recipePage("Beef Stew", true, true, true, true),
}}
cands := []Candidate{{URL: "https://a.example/1"}, {URL: "https://b.example/missing"}}
got, err := Rank(context.Background(), f, "beef stew", cands)
if len(got) != 1 || got[0].Candidate.URL != "https://a.example/1" {
t.Errorf("ranked = %+v, want only https://a.example/1", got)
}
if !errors.Is(err, ErrCandidatesUnranked) {
t.Errorf("err = %v, want ErrCandidatesUnranked", err)
}
var se *StatusError
if !errors.As(err, &se) || se.Status != 404 || se.URL != "https://b.example/missing" {
t.Errorf("err = %v, want the 404 StatusError for https://b.example/missing", err)
}
if err != nil && !strings.Contains(err.Error(), "https://b.example/missing") {
t.Errorf("err = %v, want it to name the failed candidate", err)
}
}
func TestRankAllRejected(t *testing.T) { func TestRankAllRejected(t *testing.T) {
f := &fakeFetcher{pages: map[string][]byte{ f := &fakeFetcher{pages: map[string][]byte{
"https://a.example/noimg": recipePage("Chicken Curry", true, false, true, true), "https://a.example/noimg": recipePage("Chicken Curry", true, false, true, true),
@@ -157,3 +185,71 @@ func TestRankContextCancelled(t *testing.T) {
t.Errorf("err = %v, want context.Canceled", err) t.Errorf("err = %v, want context.Canceled", err)
} }
} }
// TestRankOverHTTP ranks fixture candidates served over HTTPS through the
// SSRF-safe Fetcher: every host, a subdomain the test server's certificate
// covers, resolves to a public test address the test
// dialer routes to one httptest server, which serves pages by host and path.
func TestRankOverHTTP(t *testing.T) {
t.Parallel()
pages := map[string][]byte{
"best.example.com/curry": recipePage("Chicken Curry", true, true, true, true),
"easy.example.com/curry": recipePage("Easy Chicken Curry", true, true, true, false),
"thai.example.com/curry": recipePage("Thai Green Curry", true, true, true, true),
"noimg.example.com/curry": recipePage("Chicken Curry", true, false, true, true),
}
env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
body, ok := pages[r.Host+r.URL.Path]
if !ok {
http.Error(w, "broken", http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "text/html")
w.Write(body)
}), map[string][][]string{
"best.example.com": {{publicIP}},
"easy.example.com": {{publicIP}},
"thai.example.com": {{publicIP}},
"noimg.example.com": {{publicIP}},
"broken.example.com": {{publicIP}},
})
cands := []Candidate{
{URL: "https://thai.example.com/curry"},
{URL: "https://broken.example.com/curry"},
{URL: "https://easy.example.com/curry"},
{URL: "https://noimg.example.com/curry"},
{URL: "https://best.example.com/curry"},
}
got, err := Rank(t.Context(), env.f, "Chicken curry", cands)
want := []struct {
url string
score float64
}{
{"https://best.example.com/curry", 2},
{"https://easy.example.com/curry", 1.75},
{"https://thai.example.com/curry", 1.25},
}
if len(got) != len(want) {
t.Fatalf("got %d ranked, want %d: %+v (err %v)", len(got), len(want), got, err)
}
for i, w := range want {
if got[i].Candidate.URL != w.url || got[i].Score != w.score {
t.Errorf("rank %d = %s (%.2f), want %s (%.2f)", i, got[i].Candidate.URL, got[i].Score, w.url, w.score)
}
}
if got[0].Recipe.Title != "Chicken Curry" || got[0].Recipe.ImageURL != "https://img.example.com/dish.jpg" {
t.Errorf("top recipe = %+v", got[0].Recipe)
}
if !errors.Is(err, ErrCandidatesUnranked) || !errors.Is(err, ErrNoImage) {
t.Errorf("err = %v, want ErrCandidatesUnranked with ErrNoImage", err)
}
var se *StatusError
if !errors.As(err, &se) || se.Status != http.StatusInternalServerError {
t.Errorf("err = %v, want the 500 StatusError for broken.example.com", err)
}
if n := len(env.dials()); n != len(cands) {
t.Errorf("dialed %d times, want %d", n, len(cands))
}
}
+27 -7
View File
@@ -36,8 +36,10 @@ var (
// ErrSearchRateLimited is returned for HTTP 429. // ErrSearchRateLimited is returned for HTTP 429.
ErrSearchRateLimited = errors.New("recipes: search provider rate limit reached") ErrSearchRateLimited = errors.New("recipes: search provider rate limit reached")
// ErrSearchResponse is returned for a response body that is not the // ErrSearchResponse is returned for a response body that is not the
// provider's JSON shape, is over MaxSearchBodyBytes, or holds a result // provider's JSON shape — SearXNG without a results array, Brave without
// without an absolute http(s) URL. // "type": "search" or with a web object lacking a results array — is
// over MaxSearchBodyBytes, or holds a result without an absolute http(s)
// URL.
ErrSearchResponse = errors.New("recipes: malformed search response") ErrSearchResponse = errors.New("recipes: malformed search response")
) )
@@ -126,8 +128,10 @@ func (s *SearXNG) Search(ctx context.Context, query string, n int) ([]Candidate,
if err != nil { if err != nil {
return nil, fmt.Errorf("recipes: build SearXNG request: %w", err) return nil, fmt.Errorf("recipes: build SearXNG request: %w", err)
} }
// Results is a pointer so a body without the array is told apart from
// an empty search.
var body struct { var body struct {
Results []struct { Results *[]struct {
URL string `json:"url"` URL string `json:"url"`
Title string `json:"title"` Title string `json:"title"`
Content string `json:"content"` Content string `json:"content"`
@@ -136,8 +140,11 @@ func (s *SearXNG) Search(ctx context.Context, query string, n int) ([]Candidate,
if err := doSearch(s.client, req, "SearXNG", &body); err != nil { if err := doSearch(s.client, req, "SearXNG", &body); err != nil {
return nil, err return nil, err
} }
if body.Results == nil {
return nil, fmt.Errorf("%w: SearXNG body has no results array", ErrSearchResponse)
}
var out []Candidate var out []Candidate
for _, r := range body.Results { for _, r := range *body.Results {
out = append(out, Candidate{URL: r.URL, Title: r.Title, Snippet: r.Content}) out = append(out, Candidate{URL: r.URL, Title: r.Title, Snippet: r.Content})
} }
return candidates("SearXNG", out, n) return candidates("SearXNG", out, n)
@@ -177,9 +184,13 @@ func (b *Brave) Search(ctx context.Context, query string, n int) ([]Candidate, e
return nil, fmt.Errorf("recipes: build Brave request: %w", err) return nil, fmt.Errorf("recipes: build Brave request: %w", err)
} }
req.Header.Set("X-Subscription-Token", b.key) req.Header.Set("X-Subscription-Token", b.key)
// Brave leaves out "web" when nothing matched, so a search response is
// recognised by its "type"; a web object present must hold the results
// array.
var body struct { var body struct {
Web struct { Type string `json:"type"`
Results []struct { Web *struct {
Results *[]struct {
URL string `json:"url"` URL string `json:"url"`
Title string `json:"title"` Title string `json:"title"`
Description string `json:"description"` Description string `json:"description"`
@@ -189,8 +200,17 @@ func (b *Brave) Search(ctx context.Context, query string, n int) ([]Candidate, e
if err := doSearch(b.client, req, "Brave", &body); err != nil { if err := doSearch(b.client, req, "Brave", &body); err != nil {
return nil, err return nil, err
} }
if body.Type != "search" {
return nil, fmt.Errorf("%w: Brave body type is %q, not \"search\"", ErrSearchResponse, body.Type)
}
if body.Web == nil {
return nil, nil
}
if body.Web.Results == nil {
return nil, fmt.Errorf("%w: Brave web object has no results array", ErrSearchResponse)
}
var out []Candidate var out []Candidate
for _, r := range body.Web.Results { for _, r := range *body.Web.Results {
out = append(out, Candidate{URL: r.URL, Title: r.Title, Snippet: r.Description}) out = append(out, Candidate{URL: r.URL, Title: r.Title, Snippet: r.Description})
} }
return candidates("Brave", out, n) return candidates("Brave", out, n)
+21 -2
View File
@@ -166,8 +166,11 @@ func TestSearchErrors(t *testing.T) {
return errors.As(err, &se) && se.Status == http.StatusFound return errors.As(err, &se) && se.Status == http.StatusFound
}}, }},
{"not JSON", http.StatusOK, `<html>`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }}, {"not JSON", http.StatusOK, `<html>`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }},
{"result without URL", http.StatusOK, `{"results":[{"title":"x"}],"web":{"results":[{"title":"x"}]}}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }}, {"result without URL", http.StatusOK, `{"type":"search","results":[{"title":"x"}],"web":{"results":[{"title":"x"}]}}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }},
{"relative URL", http.StatusOK, `{"results":[{"url":"/x"}],"web":{"results":[{"url":"/x"}]}}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }}, {"relative URL", http.StatusOK, `{"type":"search","results":[{"url":"/x"}],"web":{"results":[{"url":"/x"}]}}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }},
{"empty object", http.StatusOK, `{}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }},
{"null results", http.StatusOK, `{"type":"search","results":null,"web":{"results":null}}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }},
{"results not an array", http.StatusOK, `{"type":"search","results":{},"web":{"results":{}}}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }},
{"body too large", http.StatusOK, `{"results":[],"pad":"` + strings.Repeat("x", MaxSearchBodyBytes) + `"}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }}, {"body too large", http.StatusOK, `{"results":[],"pad":"` + strings.Repeat("x", MaxSearchBodyBytes) + `"}`, func(err error) bool { return errors.Is(err, ErrSearchResponse) }},
} }
for _, tc := range tests { for _, tc := range tests {
@@ -189,6 +192,22 @@ func TestSearchErrors(t *testing.T) {
} }
} }
// TestSearchNoMatches checks a well-formed response with nothing found is an
// empty search, not an error.
func TestSearchNoMatches(t *testing.T) {
providers := map[string]SearchProvider{
"SearXNG": newSearXNGServer(t, http.StatusOK, `{"query":"x recipe","results":[]}`, nil),
"Brave without web": newBraveServer(t, "secret-key", http.StatusOK, `{"type":"search","query":{"original":"x recipe"}}`, nil),
"Brave empty web": newBraveServer(t, "secret-key", http.StatusOK, `{"type":"search","web":{"type":"search","results":[]}}`, nil),
}
for name, p := range providers {
got, err := p.Search(context.Background(), "x", 3)
if err != nil || len(got) != 0 {
t.Errorf("%s: got %v, %v; want no candidates and no error", name, got, err)
}
}
}
func TestSearchContextCancelled(t *testing.T) { func TestSearchContextCancelled(t *testing.T) {
s := newSearXNGServer(t, http.StatusOK, searxngBody, nil) s := newSearXNGServer(t, http.StatusOK, searxngBody, nil)
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())