diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b2182e7..c3529fc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -215,9 +215,15 @@ automatic** (D-021). `pending` or `failed`; a recipe entered by hand has only `raw_text`. Two items at one position leave recipe order unknown, so such a recipe is 500 rather than shown out of order. `source` is `{site_name}` of the - page the recipe was fetched from (`site_name` null when the page names - none), null for a manual recipe or none; it has no `url`, as the page's - link may carry a personal token. `why_picked` is the reason of the + page the recipe was fetched from, null for a manual recipe or none; it + has no `url`, as the page's link may carry a personal token. + `site_name` is the name the page gives its site (publisher or + `og:site_name`) and null otherwise: a page naming no site is stored + under its host, which may be the pasted link's — a token in a + subdomain — or a redirect's, so any stored name that could be a host + (the link's host, an IP address or a dotted host name, even a real + name like "Food.com") is withheld. The detail screen shows "Recipe + from {site}" only when `site_name` is not null. `why_picked` is the reason of the suggestion that created the dinner — its pending or accepted one, as declining a suggestion archives its dinner — null for one no suggestion created; a dinner with more than one such suggestion is 500, as nothing diff --git a/internal/dinners/detail.go b/internal/dinners/detail.go index 1d7b726..b2673b2 100644 --- a/internal/dinners/detail.go +++ b/internal/dinners/detail.go @@ -5,7 +5,10 @@ import ( "database/sql" "errors" "fmt" + "net" + "net/url" "strconv" + "strings" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" @@ -59,7 +62,9 @@ type Ingredient struct { // link, pasted or offered as a candidate, may carry a personal token, and // no stored link can be shown to be free of one, so like a dinner's // recipe_url and a candidate's page it never leaves the server. SiteName -// is null when the page names no site. +// is the name the page itself gives its site, and null when it gives none +// or the stored name could have come from a link (see pageSiteName), so a +// client shows "Recipe from {site}" only when SiteName is not null. type Source struct { SiteName *string `json:"site_name"` } @@ -140,7 +145,11 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) return fmt.Errorf("recipe %s: %w", recipeID, err) } if !IsManualRecipe(rec.SourceUrl) { - d.Source = &Source{SiteName: nullString(rec.SiteName)} + site, err := pageSiteName(rec.SiteName, rec.SourceUrl) + if err != nil { + return err + } + d.Source = &Source{SiteName: site} } ings, err := q.ListRecipeIngredients(ctx, recipeID) if err != nil { @@ -171,6 +180,35 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) return nil } +// pageSiteName is the stored site name of a recipe fetched from sourceURL +// when it is one the page gave, else nil. A page that names no site is +// stored under recipes.HostSiteName of the URL it was served from: the +// pasted link's host, which may hold a personal token (a private +// subdomain), or that of a page the link redirected to, which is not +// stored. Nothing records which a stored name is, so any name that could +// be a host — the link's own host, an IP address, or a dotted host name — +// is treated as one and withheld. A site that names itself as a domain, +// such as "Food.com", loses its attribution with them: a recipe with no +// attribution is better than one that leaks a link. +func pageSiteName(stored sql.NullString, sourceURL string) (*string, error) { + if !stored.Valid { + return nil, nil + } + link, err := url.Parse(sourceURL) + if err != nil { + // Not wrapped: url.Parse's error quotes the link. + return nil, errors.New("source url does not parse") + } + name := stored.String + if strings.EqualFold(name, recipes.HostSiteName(link)) || net.ParseIP(name) != nil { + return nil, nil + } + if u, err := url.Parse("https://" + name + "/"); err == nil && u.Host == name && strings.Contains(name, ".") { + return nil, nil + } + return &name, nil +} + // duplicatePosition is the error for two recipe items of kind at position. // Position is the only record of recipe order, and the id that breaks the // tie says nothing about it, so a recipe with a duplicate is refused rather diff --git a/internal/dinners/dinners_test.go b/internal/dinners/dinners_test.go index 764f6cb..16cf234 100644 --- a/internal/dinners/dinners_test.go +++ b/internal/dinners/dinners_test.go @@ -1,6 +1,7 @@ package dinners_test import ( + "database/sql" "encoding/json" "errors" "strings" @@ -74,3 +75,46 @@ func TestCreateRegularNeverNamesFromLink(t *testing.T) { } check("duplicate error", dup.Error()) } + +// TestPageSiteNameWithholdsHosts: a stored site name that could be a host — +// the link's own, in any case and without "www.", an IP address or a dotted +// host name — is withheld; a name the page gave, dotless or with spaces, is +// kept. +func TestPageSiteNameWithholdsHosts(t *testing.T) { + t.Parallel() + const link = "https://www.TokenHost/recipes/stew" + for _, tc := range []struct { + stored string + valid bool + want *string + }{ + {"", false, nil}, + {"tokenhost", true, nil}, + {"TOKENHOST", true, nil}, + {"private-token.example.com", true, nil}, + {"Food.com", true, nil}, + {"203.0.113.7", true, nil}, + {"2001:db8::1", true, nil}, + {"Delish", true, ptr("Delish")}, + {"Bon Appétit", true, ptr("Bon Appétit")}, + {"Mr. Food Test Kitchen", true, ptr("Mr. Food Test Kitchen")}, + } { + got, err := dinners.PageSiteName(sql.NullString{String: tc.stored, Valid: tc.valid}, link) + if err != nil { + t.Fatalf("PageSiteName(%q): %v", tc.stored, err) + } + if (got == nil) != (tc.want == nil) || (got != nil && *got != *tc.want) { + t.Errorf("PageSiteName(%q) = %v, want %v", tc.stored, deref(got), deref(tc.want)) + } + } + if _, err := dinners.PageSiteName(sql.NullString{String: "Delish", Valid: true}, "https://secret-token\x7f.example/%zz"); err == nil || strings.Contains(err.Error(), "secret-token") { + t.Errorf("PageSiteName of an unparseable link: err = %v, want one not quoting the link", err) + } +} + +func deref(s *string) any { + if s == nil { + return nil + } + return *s +} diff --git a/internal/dinners/export_test.go b/internal/dinners/export_test.go index b143035..711568f 100644 --- a/internal/dinners/export_test.go +++ b/internal/dinners/export_test.go @@ -71,3 +71,6 @@ func (s *Service) RunGroundJob(ctx context.Context, p GroundJob) (string, error) r, err := s.runGroundJob(ctx, p, nil) return r.GroundingStatus, err } + +// PageSiteName is pageSiteName. +var PageSiteName = pageSiteName diff --git a/internal/recipes/extract.go b/internal/recipes/extract.go index 2ce6069..9de3383 100644 --- a/internal/recipes/extract.go +++ b/internal/recipes/extract.go @@ -29,7 +29,7 @@ type Recipe struct { // Title is the recipe's name. Title string // SiteName is the publisher's name, else the page's og:site_name, else - // its host without a leading "www.". + // HostSiteName of the page's URL. SiteName string // ImageURL is the largest image, absolute; empty when the recipe has // none. @@ -232,7 +232,13 @@ func siteName(publisher any, g graph, ogSiteName string, base *url.URL) (string, if ogSiteName != "" { return ogSiteName, nil } - return strings.TrimPrefix(base.Hostname(), "www."), nil + return HostSiteName(base), nil +} + +// HostSiteName is the site name of a page that names none: its host +// without a leading "www.". +func HostSiteName(u *url.URL) string { + return strings.TrimPrefix(u.Hostname(), "www.") } // imageURL picks the largest of the recipe's images by width × height; diff --git a/internal/server/dinnerdetail_test.go b/internal/server/dinnerdetail_test.go index 95c635a..f60ddaf 100644 --- a/internal/server/dinnerdetail_test.go +++ b/internal/server/dinnerdetail_test.go @@ -265,6 +265,80 @@ func TestGetDinnerSourceHidesTokenURL(t *testing.T) { } } +// TestGetDinnerSiteNameNeverFromLink: a page that names no site is stored +// under its host, so a recipe grounded in a pasted link whose host carries a +// token has site_name null and nothing from the link in its detail; a page +// that names its site keeps that name, even on a token-bearing host; and a +// stored name shaped like another host, as a redirect leaves, is withheld. +func TestGetDinnerSiteNameNeverFromLink(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t) + const ( + bareToken = "private-token-abc123" + namedToken = "named-token-def456" + bare = "https://" + bareToken + ".example.com/recipe" + named = "https://www." + namedToken + ".example.com/recipe" + ) + e.pages.Bodies[bare] = recipestest.RecipePage("Chicken Curry", "", "https://a.example/curry.jpg", 40) + e.pages.Bodies[named] = recipestest.RecipePage("Weeknight Curry", "Curry House", "https://b.example/curry.jpg", 25) + e.images.JPEGs["https://a.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 1) + e.images.JPEGs["https://b.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 2) + ground := func(dish, page string) (id, recipeID string) { + t.Helper() + id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) + if rec, _ := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"recipe_url":"`+page+`"}`); rec.Code != http.StatusOK { + t.Fatalf("POST ground %s = %d: %s", page, rec.Code, rec.Body) + } + if err := e.st.Reader.QueryRow(`SELECT recipe_id FROM dinners WHERE id = ?`, id).Scan(&recipeID); err != nil { + t.Fatal(err) + } + return id, recipeID + } + storedSite := func(recipeID string) string { + t.Helper() + var site string + if err := e.st.Reader.QueryRow(`SELECT site_name FROM recipes WHERE id = ?`, recipeID).Scan(&site); err != nil { + t.Fatal(err) + } + return site + } + check := func(what, id string, leaks []string, want any) { + t.Helper() + rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "") + if rec.Code != http.StatusOK { + t.Fatalf("GET %s = %d: %s", what, rec.Code, rec.Body) + } + for _, leak := range leaks { + if strings.Contains(strings.ToLower(rec.Body.String()), leak) { + t.Errorf("detail of %s contains %q: %s", what, leak, rec.Body) + } + } + raw, _ := getDetail(t, e.h, id) + if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": want}) { + t.Errorf("source of %s = %v, want only site_name %v", what, raw["source"], want) + } + } + + bareID, bareRecipe := ground("Chicken curry", bare) + if got := storedSite(bareRecipe); got != bareToken+".example.com" { + t.Fatalf("stored site_name = %q, want the link's host", got) + } + check("a page naming no site", bareID, []string{bare, bareToken, "example.com", `"url"`}, nil) + + namedID, namedRecipe := ground("Weeknight curry", named) + if got := storedSite(namedRecipe); got != "Curry House" { + t.Fatalf("stored site_name = %q, want the page's", got) + } + check("a page naming its site", namedID, []string{named, namedToken, "example.com", `"url"`}, "Curry House") + + const redirectHost = "redirect-token-ghi789.example.net" + if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, redirectHost, namedRecipe); err != nil { + t.Fatal(err) + } + check("a page named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken}, nil) +} + // TestGetDinnerWhyPickedIsCreatingSuggestion: why_picked is the reason of // the suggestion that created the dinner, never that of an earlier // suggestion row for it that was declined. diff --git a/internal/server/dinners.go b/internal/server/dinners.go index cb09073..5f0d54e 100644 --- a/internal/server/dinners.go +++ b/internal/server/dinners.go @@ -72,7 +72,11 @@ func getDinners(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc { // getDinner answers GET /dinners/{id} with the dinner, its recipe and the // reason it was suggested. An archived dinner is 404, like an unknown one, -// not a 200 with its status: it has no detail to show. +// not a 200 with its status: it has no detail to show. source never has the +// recipe page's link, and source.site_name is null unless the page itself +// named its site — a name that could be a host, the link's or a redirect's, +// is withheld (dinners.Source) — so the detail screen shows "Recipe from +// {site}" only when site_name is not null. func getDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { d, err := svc.Get(r.Context(), chi.URLParam(r, "id"))