fix: the crawler and the Chrome guard keep to the site's port
Fetcher.Allowed and Key, the rendered comparison and the Chrome request guard accept an address only on the site's effective port (explicit, else 443 for https and 80 for http), so page code cannot make headless Chrome reach other services on the site host's address and a link to another port is not reported as a page of the site.
This commit is contained in:
@@ -731,7 +731,7 @@ func checkRenderedPage(res *Result, p *Page, rec *Record) []Finding {
|
||||
if p.Rendered == nil || res.fetcher == nil {
|
||||
return nil
|
||||
}
|
||||
out := compareRendered(p.Facts, p.Rendered, res.fetcher.Hosts())
|
||||
out := compareRendered(p.Facts, p.Rendered, res.fetcher.Key)
|
||||
for i := range out {
|
||||
out[i].URL, out[i].Locale, out[i].Record = p.Key, locale(rec), rec
|
||||
}
|
||||
|
||||
@@ -16,32 +16,21 @@ const (
|
||||
)
|
||||
|
||||
// compareRendered compares a page's raw HTML with its JavaScript-rendered
|
||||
// DOM (D-70, P33-55; pure): same-host links, an H1 or a substantial amount
|
||||
// DOM (D-70, P33-55; pure): same-site links, an H1 or a substantial amount
|
||||
// of text that appear only after JavaScript are one content_js finding
|
||||
// with details.check rendered and details.what naming them. hosts are the
|
||||
// site host and its www twin. An identical rendering finds nothing.
|
||||
func compareRendered(raw, rendered *PageFacts, hosts []string) []Finding {
|
||||
// with details.check rendered and details.what naming them. key is the
|
||||
// crawl's address of a link (Fetcher.Key: "" off the site, another port
|
||||
// included). An identical rendering finds nothing.
|
||||
func compareRendered(raw, rendered *PageFacts, key func(*url.URL) string) []Finding {
|
||||
if raw == nil || rendered == nil {
|
||||
return nil
|
||||
}
|
||||
same := func(u *url.URL) (string, bool) {
|
||||
if u == nil {
|
||||
if u == nil || key == nil {
|
||||
return "", false
|
||||
}
|
||||
host := strings.ToLower(u.Hostname())
|
||||
for _, h := range hosts {
|
||||
if host == h {
|
||||
p := u.EscapedPath()
|
||||
if p == "" {
|
||||
p = "/"
|
||||
}
|
||||
if u.RawQuery != "" {
|
||||
p += "?" + u.RawQuery
|
||||
}
|
||||
return p, true
|
||||
}
|
||||
}
|
||||
return "", false
|
||||
k := key(u)
|
||||
return k, k != ""
|
||||
}
|
||||
before := map[string]bool{}
|
||||
for _, l := range raw.Links {
|
||||
|
||||
@@ -18,7 +18,15 @@ func linkFacts(words int, h1 string, hrefs ...string) *PageFacts {
|
||||
return f
|
||||
}
|
||||
|
||||
var siteHosts = []string{"example.test", "www.example.test"}
|
||||
// siteKey is the crawl key of a link on https://example.test (its www
|
||||
// twin too, port 443 only).
|
||||
func siteKey(u *url.URL) string {
|
||||
f, err := NewFetcher("https://example.test", nil)
|
||||
if err != nil {
|
||||
panic(err)
|
||||
}
|
||||
return f.Key(u)
|
||||
}
|
||||
|
||||
// TestCompareRendered pins the rendered comparison (P33-55): a same-host
|
||||
// link (the www twin too), an H1 and growth of at least 50 words and 20 %
|
||||
@@ -28,7 +36,7 @@ var siteHosts = []string{"example.test", "www.example.test"}
|
||||
func TestCompareRendered(t *testing.T) {
|
||||
raw := linkFacts(100, "", "https://example.test/a")
|
||||
rendered := linkFacts(160, "Grzyby", "https://example.test/a", "https://www.example.test/b?x=1", "https://other.test/c", "https://example.test/b?x=1")
|
||||
got := compareRendered(raw, rendered, siteHosts)
|
||||
got := compareRendered(raw, rendered, siteKey)
|
||||
if len(got) != 1 || got[0].Kind != KindContentJS || got[0].Check != checkRendered {
|
||||
t.Fatalf("findings = %+v", got)
|
||||
}
|
||||
@@ -47,14 +55,14 @@ func TestCompareRendered(t *testing.T) {
|
||||
"50 words under 20 %": {linkFacts(300, "T"), linkFacts(350, "T")},
|
||||
"H1 already there": {linkFacts(10, "Raw"), linkFacts(10, "Rendered")},
|
||||
} {
|
||||
if got := compareRendered(pair[0], pair[1], siteHosts); got != nil {
|
||||
if got := compareRendered(pair[0], pair[1], siteKey); got != nil {
|
||||
t.Errorf("%s: %+v", name, got)
|
||||
}
|
||||
}
|
||||
if got := compareRendered(linkFacts(0, "T"), linkFacts(50, "T"), siteHosts); len(got) != 1 {
|
||||
if got := compareRendered(linkFacts(0, "T"), linkFacts(50, "T"), siteKey); len(got) != 1 {
|
||||
t.Fatalf("50 words from an empty page: %+v", got)
|
||||
}
|
||||
if compareRendered(nil, rendered, siteHosts) != nil || compareRendered(raw, nil, siteHosts) != nil {
|
||||
if compareRendered(nil, rendered, siteKey) != nil || compareRendered(raw, nil, siteKey) != nil {
|
||||
t.Fatal("a missing side compares to nothing")
|
||||
}
|
||||
}
|
||||
@@ -66,7 +74,7 @@ func TestCompareRenderedCap(t *testing.T) {
|
||||
for _, p := range []string{"", "/1", "/2", "/3", "/4", "/5", "/6"} {
|
||||
hrefs = append(hrefs, "https://example.test"+p)
|
||||
}
|
||||
got := compareRendered(linkFacts(10, "T"), linkFacts(10, "T", hrefs...), siteHosts)
|
||||
got := compareRendered(linkFacts(10, "T"), linkFacts(10, "T", hrefs...), siteKey)
|
||||
links := got[0].Details["links"].([]string)
|
||||
if len(links) != maxJSOnlyURLs || links[0] != "/" {
|
||||
t.Fatalf("links = %v", links)
|
||||
|
||||
@@ -65,10 +65,29 @@ type Fetcher struct {
|
||||
client *fetchguard.Client
|
||||
site *url.URL
|
||||
hosts []string
|
||||
port string
|
||||
trusted bool
|
||||
ua string
|
||||
}
|
||||
|
||||
// effectivePort is u's port: the explicit one, else 443 for https and 80
|
||||
// for http ("" for another scheme without a port).
|
||||
func effectivePort(u *url.URL) string {
|
||||
if u == nil {
|
||||
return ""
|
||||
}
|
||||
if p := u.Port(); p != "" {
|
||||
return p
|
||||
}
|
||||
switch strings.ToLower(u.Scheme) {
|
||||
case "https":
|
||||
return "443"
|
||||
case "http":
|
||||
return "80"
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// SiteHosts returns the host of site and its www twin.
|
||||
func SiteHosts(host string) []string {
|
||||
host = strings.ToLower(strings.TrimSpace(host))
|
||||
@@ -92,7 +111,7 @@ func NewFetcher(site string, cfg *compass.Config) (*Fetcher, error) {
|
||||
if err != nil || u.Host == "" || (u.Scheme != "https" && u.Scheme != "http") {
|
||||
return nil, ErrNoSiteURL
|
||||
}
|
||||
f := &Fetcher{site: u, hosts: SiteHosts(u.Hostname()), trusted: Trusted(cfg), ua: UserAgent(u.Scheme + "://" + u.Host)}
|
||||
f := &Fetcher{site: u, hosts: SiteHosts(u.Hostname()), port: effectivePort(u), trusted: Trusted(cfg), ua: UserAgent(u.Scheme + "://" + u.Host)}
|
||||
policy := fetchguard.Policy{Mode: fetchguard.AllowHostsMode, AllowHosts: f.hosts, MaxBytes: MaxBody, Timeout: FetchTimeout}
|
||||
if f.trusted {
|
||||
policy.Mode = fetchguard.TrustedMode
|
||||
@@ -116,13 +135,21 @@ func (f *Fetcher) Site() *url.URL { return f.site }
|
||||
// UA returns the crawler's User-Agent.
|
||||
func (f *Fetcher) UA() string { return f.ua }
|
||||
|
||||
// Port returns the site's effective port (explicit, else 443 for https
|
||||
// and 80 for http).
|
||||
func (f *Fetcher) Port() string { return f.port }
|
||||
|
||||
// Allowed reports whether u may be fetched: https (http only in trusted
|
||||
// mode) on the site host or its www twin, without credentials. In trusted
|
||||
// mode the host list is checked here, since fetchguard checks none then.
|
||||
// mode) on the site host or its www twin and on the site's effective port,
|
||||
// without credentials. In trusted mode the host list is checked here,
|
||||
// since fetchguard checks none then.
|
||||
func (f *Fetcher) Allowed(u *url.URL) bool {
|
||||
if u == nil || u.User != nil {
|
||||
return false
|
||||
}
|
||||
if effectivePort(u) != f.port {
|
||||
return false
|
||||
}
|
||||
switch strings.ToLower(u.Scheme) {
|
||||
case "https":
|
||||
case "http":
|
||||
@@ -142,7 +169,7 @@ func (f *Fetcher) Allowed(u *url.URL) bool {
|
||||
}
|
||||
|
||||
// Key is the address of u on the site: its path with the query, or ""
|
||||
// when u is not on the site.
|
||||
// when u is not on the site (another host, scheme or port).
|
||||
func (f *Fetcher) Key(u *url.URL) string {
|
||||
if !f.Allowed(u) {
|
||||
return ""
|
||||
|
||||
51
classes/crawl/fetch_test.go
Normal file
51
classes/crawl/fetch_test.go
Normal file
@@ -0,0 +1,51 @@
|
||||
package crawl
|
||||
|
||||
import (
|
||||
"net/url"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestFetcherPortAndKey pins L3 WR-03 (and WR-04, P33-61): the fetcher
|
||||
// accepts an address only on the site's effective port, so a link to
|
||||
// another port is neither fetched nor reported as a page of the site.
|
||||
func TestFetcherPortAndKey(t *testing.T) {
|
||||
f, err := NewFetcher("https://site.test", nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if f.Port() != "443" {
|
||||
t.Fatalf("Port = %q", f.Port())
|
||||
}
|
||||
parse := func(raw string) *url.URL {
|
||||
u, err := url.Parse(raw)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return u
|
||||
}
|
||||
for raw, want := range map[string]string{
|
||||
"https://site.test:8443/x": "",
|
||||
"https://site.test:443/x": "/x",
|
||||
"https://www.site.test/x": "/x",
|
||||
"https://site.test:80/x": "",
|
||||
"http://site.test/x": "",
|
||||
"https://other.test/x": "",
|
||||
"https://site.test": "/",
|
||||
"https://site.test/a?b=1&c=": "/a?b=1&c=",
|
||||
} {
|
||||
u := parse(raw)
|
||||
if got := f.Key(u); got != want {
|
||||
t.Errorf("Key(%s) = %q, want %q", raw, got, want)
|
||||
}
|
||||
if f.Allowed(u) != (want != "") {
|
||||
t.Errorf("Allowed(%s) = %v", raw, f.Allowed(u))
|
||||
}
|
||||
}
|
||||
local, err := NewFetcher("http://localhost:3000", nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if local.Port() != "3000" || effectivePort(parse("http://x")) != "80" || effectivePort(parse("ftp://x")) != "" || effectivePort(nil) != "" {
|
||||
t.Fatal("effectivePort is wrong")
|
||||
}
|
||||
}
|
||||
@@ -103,7 +103,7 @@ func NewRenderer(ctx context.Context, cfg *compass.Config, site *url.URL, truste
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
r, err := newChromeRenderer(path, hosts, trusted, ua)
|
||||
r, err := newChromeRenderer(path, hosts, effectivePort(site), trusted, ua)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -161,9 +161,11 @@ func resolverRules(hosts map[string][]netip.Addr) string {
|
||||
}
|
||||
|
||||
// allowRequest reports whether the headless browser may load rawURL: https
|
||||
// (http only in trusted mode) to one of hosts, or a data: or blob: URL.
|
||||
// Every other request, third-party scripts included, is failed.
|
||||
func allowRequest(rawURL string, hosts []string, trusted bool) bool {
|
||||
// (http only in trusted mode) to one of hosts on the site's effective port
|
||||
// (port; the request's explicit port, else 443 for https and 80 for http),
|
||||
// or a data: or blob: URL. Every other request, third-party scripts and
|
||||
// other services on the site host's address included, is failed.
|
||||
func allowRequest(rawURL string, hosts []string, port string, trusted bool) bool {
|
||||
u, err := url.Parse(rawURL)
|
||||
if err != nil {
|
||||
return false
|
||||
@@ -179,7 +181,7 @@ func allowRequest(rawURL string, hosts []string, trusted bool) bool {
|
||||
default:
|
||||
return false
|
||||
}
|
||||
if u.User != nil {
|
||||
if u.User != nil || effectivePort(u) != port {
|
||||
return false
|
||||
}
|
||||
host := strings.ToLower(u.Hostname())
|
||||
|
||||
@@ -29,7 +29,7 @@ import (
|
||||
// fetchguard.IsPrivateAddr; every other host resolves to NOTFOUND;
|
||||
// - every request is paused through the Fetch domain and continued only
|
||||
// when allowRequest passes (https, http only in trusted mode, to an
|
||||
// allowed host, or data: and blob:); the others fail with
|
||||
// allowed host on the site's port, or data: and blob:); the others fail with
|
||||
// BlockedByClient, so third-party scripts are never loaded;
|
||||
// - a fresh temporary profile per crawl, removed by Close; no cookies,
|
||||
// credentials or extensions; downloads denied;
|
||||
@@ -43,6 +43,7 @@ import (
|
||||
// added that weakens Chrome's sandbox.
|
||||
type chromeRenderer struct {
|
||||
allowed []string
|
||||
port string
|
||||
trusted bool
|
||||
profile string
|
||||
|
||||
@@ -75,13 +76,13 @@ func seconds(d time.Duration) string {
|
||||
}
|
||||
|
||||
// newChromeRenderer starts Chrome at path for the hosts and their checked
|
||||
// addresses.
|
||||
func newChromeRenderer(path string, hosts map[string][]netip.Addr, trusted bool, ua string) (*chromeRenderer, error) {
|
||||
// addresses, on the site's effective port.
|
||||
func newChromeRenderer(path string, hosts map[string][]netip.Addr, port string, trusted bool, ua string) (*chromeRenderer, error) {
|
||||
profile, err := os.MkdirTemp("", "summercms-seo-chrome-")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("the Chrome profile cannot be created: %w", err)
|
||||
}
|
||||
r := &chromeRenderer{trusted: trusted, profile: profile}
|
||||
r := &chromeRenderer{port: port, trusted: trusted, profile: profile}
|
||||
for h := range hosts {
|
||||
r.allowed = append(r.allowed, h)
|
||||
}
|
||||
@@ -117,7 +118,7 @@ func (r *chromeRenderer) Render(ctx context.Context, pageURL string) (string, er
|
||||
if closed {
|
||||
return "", fmt.Errorf("Chrome is closed")
|
||||
}
|
||||
if !allowRequest(pageURL, r.allowed, r.trusted) {
|
||||
if !allowRequest(pageURL, r.allowed, r.port, r.trusted) {
|
||||
return "", fmt.Errorf("the address %s is outside the site", pageURL)
|
||||
}
|
||||
// One deadline bounds opening the tab and rendering the page; the tab
|
||||
@@ -206,7 +207,7 @@ func (r *chromeRenderer) decide(ctx context.Context, target cdp.Session, ev fetc
|
||||
if ev.Request != nil {
|
||||
u = ev.Request.URL
|
||||
}
|
||||
if allowRequest(u, r.allowed, r.trusted) {
|
||||
if allowRequest(u, r.allowed, r.port, r.trusted) {
|
||||
_, _ = cdp.Call(ctx, target, fetch.ContinueRequest, fetch.ContinueRequestParams{RequestID: ev.RequestID})
|
||||
return
|
||||
}
|
||||
|
||||
@@ -14,8 +14,9 @@ import (
|
||||
)
|
||||
|
||||
// TestAllowRequest pins what the headless browser may load: https on the
|
||||
// site host and its www twin, data: and blob:; http only in trusted mode;
|
||||
// other hosts, credentials in the URL and every other scheme refused.
|
||||
// site host and its www twin on the site's port, data: and blob:; http
|
||||
// only in trusted mode, also on the site's port; other hosts and ports,
|
||||
// credentials in the URL and every other scheme refused (L3 WR-03).
|
||||
func TestAllowRequest(t *testing.T) {
|
||||
hosts := []string{"example.test", "www.example.test"}
|
||||
for _, tc := range []struct {
|
||||
@@ -25,11 +26,13 @@ func TestAllowRequest(t *testing.T) {
|
||||
}{
|
||||
{"https://example.test/a.js", false, true},
|
||||
{"https://WWW.example.test/b.css", false, true},
|
||||
{"https://example.test:8443/x", false, true},
|
||||
{"https://example.test:8443/x", false, false},
|
||||
{"https://example.test:443/x", false, true},
|
||||
{"https://example.test:80/x", false, false},
|
||||
{"data:image/png;base64,AAAA", false, true},
|
||||
{"blob:https://example.test/uuid", false, true},
|
||||
{"http://example.test/a.js", false, false},
|
||||
{"http://example.test/a.js", true, true},
|
||||
{"http://example.test/a.js", true, false}, // trusted, but port 80 is not the site's 443
|
||||
{"https://cdn.other.test/lib.js", false, false},
|
||||
{"https://example.test.evil.test/x", false, false},
|
||||
{"https://user:pw@example.test/x", false, false},
|
||||
@@ -38,11 +41,19 @@ func TestAllowRequest(t *testing.T) {
|
||||
{"javascript:alert(1)", true, false},
|
||||
{"://bad", false, false},
|
||||
} {
|
||||
if got := allowRequest(tc.url, hosts, tc.trusted); got != tc.want {
|
||||
if got := allowRequest(tc.url, hosts, "443", tc.trusted); got != tc.want {
|
||||
t.Errorf("allowRequest(%q, trusted=%v) = %v, want %v", tc.url, tc.trusted, got, tc.want)
|
||||
}
|
||||
}
|
||||
if !allowRequest("https://[::1]/x", []string{"[::1]"}, false) {
|
||||
// A trusted http site on its own port: that port only.
|
||||
if !allowRequest("http://example.test:3000/x", hosts, "3000", true) || allowRequest("http://example.test:3001/x", hosts, "3000", true) ||
|
||||
allowRequest("http://example.test/x", hosts, "3000", true) {
|
||||
t.Error("trusted http keeps to the site's port")
|
||||
}
|
||||
if !allowRequest("data:text/plain,x", hosts, "3000", false) || !allowRequest("blob:https://example.test/uuid", hosts, "3000", false) {
|
||||
t.Error("data: and blob: are allowed on any port")
|
||||
}
|
||||
if !allowRequest("https://[::1]/x", []string{"[::1]"}, "443", false) {
|
||||
t.Fatal("a bracketed IPv6 host must match")
|
||||
}
|
||||
}
|
||||
@@ -138,6 +149,7 @@ func TestRenderTabTimeout(t *testing.T) {
|
||||
t.Cleanup(func() { close(hung) })
|
||||
r := &chromeRenderer{
|
||||
allowed: []string{"example.test"},
|
||||
port: "443",
|
||||
browserCtx: context.Background(),
|
||||
newTab: func(parent context.Context) (context.Context, context.CancelFunc) {
|
||||
return context.WithCancel(parent)
|
||||
|
||||
Reference in New Issue
Block a user