diff --git a/CHANGELOG.md b/CHANGELOG.md index cee3d83..9a638e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,14 @@ All notable changes to kage are recorded here. The format follows directly on its article and keeps that article's title metadata ([#62](https://github.com/tamnd/kage/issues/62)). - Non-UTF-8 `` and Content-Type charset declarations are rewritten to `utf-8`, matching the encoding kage writes to disk ([#16](https://github.com/tamnd/kage/issues/16)). +- Relative links on redirected pages resolve against the browser's final URL + and the document's first ``, while the page remains saved under + the URL that was originally discovered. Consumed `href` attributes are + removed from every `` so they cannot re-root rewritten links when the + saved page opens, while `target` behavior is preserved. + After a cross-host redirect, + relative references resolve to the other host and remain absolute when that + host is outside the crawl scope, so those resources are not localised. - `--resume` picks an interrupted crawl back up instead of doing nothing ([#36](https://github.com/tamnd/kage/issues/36)). `state.json` persisted only the visited set, and the frontier was rebuilt purely by re-rendering pages and following their links, which resume exists to avoid. So a resumed run found its seed already visited, `enqueuePage` turned it down, nothing was queued, and the run printed `pages 0` and exited successfully with most of the site still missing. diff --git a/clone/cloner.go b/clone/cloner.go index f597ec4..bc2ad7b 100644 --- a/clone/cloner.go +++ b/clone/cloner.go @@ -19,6 +19,7 @@ import ( "github.com/tamnd/kage/sanitize" "github.com/tamnd/kage/urlx" "golang.org/x/net/html" + "golang.org/x/net/html/atom" "golang.org/x/time/rate" ) @@ -315,6 +316,13 @@ func (c *Cloner) processPage(ctx context.Context, j pageItem) { return } + // Resolve references against the post-redirect URL (and any ), + // but keep writing the page under the discovered URL so existing offline + // links that pointed at /old still resolve. Cross-host redirects leave the + // resolve base as the final location for relative refs; scope checks still + // use that absolute URL. + resolveBase := pageResolveBase(j.u, res.FinalURL, root) + localFile := urlx.LocalPath(c.seedHost, j.u, urlx.Page, c.cfg.Reserved) fileDir := urlx.Dir(localFile) @@ -337,7 +345,7 @@ func (c *Cloner) processPage(ctx context.Context, j pageItem) { } } - asset.RewriteHTML(root, j.u, sink) + asset.RewriteHTML(root, resolveBase, sink) sanitize.CleanTree(root, sanitize.Options{ KeepNoscript: c.cfg.KeepNoscript, MobileReadable: c.cfg.MobileReadable, @@ -367,6 +375,55 @@ func (c *Cloner) waitForCrawlDelay(ctx context.Context) bool { return c.crawlLimiter.Wait(ctx) == nil } +// pageResolveBase picks the URL against which relative references on a rendered +// page should resolve. Preference order: +// 1. A document (the live page's own base); +// 2. The browser's final URL after redirects; +// 3. The URL that was enqueued. +// +// The page is still written under the enqueued URL so offline links discovered +// as /old keep working when the server redirected /old → /new. +func pageResolveBase(enqueued *url.URL, finalURL string, root *html.Node) *url.URL { + base := enqueued + if finalURL != "" { + if u, err := url.Parse(finalURL); err == nil && u.Scheme != "" && u.Host != "" { + // Drop fragment; keep query/path as the browser shows them. + u.Fragment = "" + base = u + } + } + if href := documentBaseHref(root); href != "" { + if u, err := urlx.Normalize(base, href); err == nil { + return u + } + } + return base +} + +// documentBaseHref returns the first in document order, or "". +func documentBaseHref(root *html.Node) string { + var found string + var walk func(*html.Node) + walk = func(n *html.Node) { + if found != "" || n == nil { + return + } + if n.Type == html.ElementNode && n.DataAtom == atom.Base { + for _, a := range n.Attr { + if strings.EqualFold(a.Key, "href") && strings.TrimSpace(a.Val) != "" { + found = strings.TrimSpace(a.Val) + return + } + } + } + for c := n.FirstChild; c != nil && found == ""; c = c.NextSibling { + walk(c) + } + } + walk(root) + return found +} + // processAsset downloads one asset, rewriting CSS references on the way, and // writes it to its deterministic local path. func (c *Cloner) processAsset(ctx context.Context, j assetItem) { diff --git a/clone/resolve_test.go b/clone/resolve_test.go new file mode 100644 index 0000000..42d59dc --- /dev/null +++ b/clone/resolve_test.go @@ -0,0 +1,45 @@ +package clone + +import ( + "net/url" + "strings" + "testing" + + "golang.org/x/net/html" +) + +func TestPageResolveBaseUsesFinalURL(t *testing.T) { + enqueued, _ := url.Parse("https://ex.com/old") + root, err := html.Parse(strings.NewReader(`n`)) + if err != nil { + t.Fatal(err) + } + base := pageResolveBase(enqueued, "https://ex.com/new/", root) + if base.String() != "https://ex.com/new/" { + t.Fatalf("resolve base = %q, want final URL", base) + } +} + +func TestPageResolveBasePrefersDocumentBase(t *testing.T) { + enqueued, _ := url.Parse("https://ex.com/page") + root, err := html.Parse(strings.NewReader( + ``)) + if err != nil { + t.Fatal(err) + } + base := pageResolveBase(enqueued, "https://ex.com/page", root) + if base.String() != "https://ex.com/dir/" { + t.Fatalf("resolve base = %q, want document ", base) + } +} + +func TestDocumentBaseHref(t *testing.T) { + root, err := html.Parse(strings.NewReader( + ``)) + if err != nil { + t.Fatal(err) + } + if got := documentBaseHref(root); got != "/subdir/" { + t.Fatalf("documentBaseHref = %q, want first base", got) + } +} diff --git a/docs/content/reference/release-notes.md b/docs/content/reference/release-notes.md index f408934..a2976e3 100644 --- a/docs/content/reference/release-notes.md +++ b/docs/content/reference/release-notes.md @@ -18,6 +18,11 @@ The authoritative, commit-level history lives in [`CHANGELOG.md`](https://github single-page archives still open directly on their article ([#62](https://github.com/tamnd/kage/issues/62)). - **Saved pages declare their real encoding.** Non-UTF-8 charset metadata is rewritten to UTF-8, matching the bytes kage writes to disk ([#16](https://github.com/tamnd/kage/issues/16)). +- **Redirects resolve correctly.** Relative links use the post-redirect URL and + the document's first ``. After rewriting, every base `href` is + removed so it cannot affect the saved page, while a base `target` is + preserved. On a cross-host redirect, references to an out-of-scope + destination remain absolute rather than being localised. ## v0.3.11 diff --git a/sanitize/sanitize.go b/sanitize/sanitize.go index de6adb7..3cf81d1 100644 --- a/sanitize/sanitize.go +++ b/sanitize/sanitize.go @@ -49,6 +49,7 @@ type Report struct { MetaRefreshRemoved int DeadLinksRemoved int CondCommentsRemoved int + BaseHrefsRemoved int CharsetAdded bool CharsetRewritten bool } @@ -110,6 +111,15 @@ func clean(n *html.Node, opts Options, rep *Report) { } if c.Type == html.ElementNode { switch c.DataAtom { + case atom.Base: + // Link rewriting has already consumed the document base. Remove every + // href so a later base cannot become active when an earlier one is + // removed, but preserve target because it controls browsing contexts. + rep.BaseHrefsRemoved += stripBaseHrefs(c) + if len(c.Attr) == 0 { + n.RemoveChild(c) + continue + } case atom.Script: n.RemoveChild(c) rep.ScriptsRemoved++ @@ -143,6 +153,20 @@ func clean(n *html.Node, opts Options, rep *Report) { } } +func stripBaseHrefs(n *html.Node) int { + kept := n.Attr[:0] + removed := 0 + for _, a := range n.Attr { + if strings.EqualFold(a.Key, "href") { + removed++ + continue + } + kept = append(kept, a) + } + n.Attr = kept + return removed +} + // stripHandlers removes every on* event-handler attribute from n. func stripHandlers(n *html.Node, rep *Report) { kept := n.Attr[:0] diff --git a/sanitize/sanitize_test.go b/sanitize/sanitize_test.go index d618eaf..2e41479 100644 --- a/sanitize/sanitize_test.go +++ b/sanitize/sanitize_test.go @@ -167,6 +167,43 @@ func TestKeepMetaRefreshPlain(t *testing.T) { } } +func TestBaseHrefRemovedAfterRewrite(t *testing.T) { + cases := []struct { + name string + bases string + wantBase bool + wantTarget bool + wantRemoved int + }{ + {name: "href only", bases: ``, wantRemoved: 1}, + {name: "target only", bases: ``, wantBase: true, wantTarget: true}, + {name: "href and target", bases: ``, wantBase: true, wantTarget: true, wantRemoved: 1}, + {name: "all hrefs", bases: ``, wantBase: true, wantTarget: true, wantRemoved: 2}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + in := `` + tc.bases + `saved` + out, rep, err := Strip([]byte(in), Options{}) + if err != nil { + t.Fatal(err) + } + s := strings.ToLower(string(out)) + if got := strings.Contains(s, ". The saved file must gain one so a reader does not fall back