From c16bf4bd9d3cdceade9ab50403b9b081ebce2c77 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Thu, 30 Jul 2026 10:29:00 +0600 Subject: [PATCH] add shortcodes as the first internal/ext feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A call is `{{< name key="value" >}}` alone on a line, parsed by a goldmark block parser into an AST node and rendered by executing a theme template of that name (ADR-0036). `figure` ships; `include` and `gallery` need the including bundle's directory, which the parser does not carry yet, so they wait. The layering did the design work here. internal/render may not import internal/ext, so render.New takes a callback that receives a Partial and returns Markdown extensions, and cmd/khosra/wire.go holds the only list of enabled features. Empty that list and the engine still builds and serves — which is the property extensions.md says the contract should have. Raw HTML stays disabled. An author's text reaches a page only as arguments that html/template escapes in context, which the real binary shows: a hostile alt becomes <script> and src="javascript:…" becomes #ZgotmplZ. Getting contextual escaping from the standard library rather than writing it is the whole reason a fragment renders this instead of the feature. parseSet became variadic so the fragment set reuses it rather than growing a second copy of the overlay logic; `Partial` takes map[string]string after the advisory correctly flagged `any` as generality nothing had asked for. --- cmd/khosra/main.go | 2 +- cmd/khosra/wire.go | 19 +++ docs/content-model.md | 20 ++- docs/extensions.md | 20 ++- docs/state.md | 16 +- docs/theme-contract.md | 25 +++- internal/ext/shortcodes/doc.go | 7 + internal/ext/shortcodes/shortcodes.go | 161 +++++++++++++++++++++ internal/ext/shortcodes/shortcodes_test.go | 132 +++++++++++++++++ internal/render/chrome_test.go | 4 +- internal/render/render.go | 74 +++++++--- internal/render/render_test.go | 10 +- internal/render/templates/shortcodes.html | 8 + internal/web/web_test.go | 16 +- 14 files changed, 459 insertions(+), 55 deletions(-) create mode 100644 cmd/khosra/wire.go create mode 100644 internal/ext/shortcodes/doc.go create mode 100644 internal/ext/shortcodes/shortcodes.go create mode 100644 internal/ext/shortcodes/shortcodes_test.go create mode 100644 internal/render/templates/shortcodes.html diff --git a/cmd/khosra/main.go b/cmd/khosra/main.go index 7c9ddd3..824cbcf 100644 --- a/cmd/khosra/main.go +++ b/cmd/khosra/main.go @@ -30,7 +30,7 @@ func main() { if err != nil { fatal("cannot read content", err) } - renderer, err := render.New(fsys) + renderer, err := render.New(fsys, extenders) if err != nil { fatal("cannot prepare the theme", err) } diff --git a/cmd/khosra/wire.go b/cmd/khosra/wire.go new file mode 100644 index 0000000..51351fd --- /dev/null +++ b/cmd/khosra/wire.go @@ -0,0 +1,19 @@ +package main + +import ( + "github.com/yuin/goldmark" + + "khosra/internal/ext/shortcodes" + "khosra/internal/render" +) + +// extenders is the only list of features this build includes (extensions.md). Source order is the +// semantics; enabling or disabling one is a one-line diff and a rebuild. +// +// It lives in cmd because nothing below it may know which features exist: internal/render, internal/web +// and internal/content must all build and serve with this list empty (conventions.md layering). +func extenders(partial render.Partial) []goldmark.Extender { + return []goldmark.Extender{ + shortcodes.New(partial), + } +} diff --git a/docs/content-model.md b/docs/content-model.md index f9490a3..9a51535 100644 --- a/docs/content-model.md +++ b/docs/content-model.md @@ -268,11 +268,23 @@ Widow prevention is not implemented: doing it safely needs a transform over the pass over rendered HTML, which cannot tell prose from an escaped code span. It waits for the Stage pipeline. -## Includes and shortcodes `[spec]` +## Shortcodes -Shortcodes are a Stage running on trusted content only (ADR-0003), never on comments. File inclusion -resolves relative to the including bundle and may not escape the site root. Transclusion of another -bundle's body is Arc 4 and needs a cycle guard on the first attempt. +A shortcode is `{{< name key="value" >}}` **alone on a line** — the whole line, or it is prose. Every +argument is `key="value"`; there is one spelling, so nothing is guessed and a malformed call stays visible +as text instead of half-working. + +Arguments are data, never markup: the call renders through a theme template of the same name +(`theme-contract.md`), and raw HTML in a body remains dropped, so the only HTML on a page came from a +template the site owns (ADR-0036). A call naming a shortcode the theme has no template for renders nothing +and logs it — one typo does not take a page down (ADR-0029). + +Shortcodes run on site-root content only (ADR-0003), never on anything untrusted. + +`figure` exists. `include` and `gallery` are `[spec]`: both need the including bundle's directory, which is +context the parser does not carry yet. File inclusion will resolve relative to the including bundle and may +not escape the site root. Transclusion of another bundle's body is Arc 4 and needs a cycle guard on the +first attempt. ## Images `[spec]` diff --git a/docs/extensions.md b/docs/extensions.md index dd7539b..0bb19ba 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -2,9 +2,14 @@ The plugin story, and the gate keeping it from arriving early. -**STATUS: not buildable yet.** A feature is its own directory under `internal/ext//`, called -explicitly from `wire.go` (ADR-0027) — correct and sufficient until the counters say otherwise. This document exists so the eventual shape is known, not -so it can be built now. +**STATUS: one feature exists.** `internal/ext/shortcodes` is the first, listed in `cmd/khosra/wire.go` +(ADR-0027) — one directory, called explicitly, correct and sufficient until the counters say otherwise. The +`Extension` struct below is still unbuilt; this document exists so the eventual shape is known, not so it +can be built now. + +How a feature reaches the engine today: `cmd` builds the list, so nothing under `internal/` knows which +features exist. A feature that must emit markup is handed `render.Partial` and renders through a theme +template, because deciding markup is not a feature's job (ADR-0036). ## The gate @@ -41,9 +46,10 @@ type Extension struct { } ``` -`cmd/khosra/wire.go` holds the only list of enabled extensions. Enabling or disabling one is a -one-line diff and a rebuild. Removing one leaves no trace elsewhere — that property is the test of -whether the contract is right. +`cmd/khosra/wire.go` holds the only list of enabled extensions — it exists now, holding one line. Enabling +or disabling one is a one-line diff and a rebuild. Removing one leaves no trace elsewhere — that property is +the test of whether the contract is right, and it is testable today: empty the list and the engine still +builds and serves, minus that feature. ## Stage phases @@ -54,7 +60,7 @@ wearing a disguise. |---|---|---| | `PhaseLoad` | raw bytes + frontmatter | includes, translation fallback | | `PhaseParse` | the parsed Markdown tree | shortcodes, transclusion, image derivatives | -| `PhaseMarkup` | rendered HTML fragments, code spans skipped | smart quotes, dashes, widows, Bengali numerals | +| `PhaseMarkup` | rendered HTML fragments, code spans skipped | widows. Smart quotes and dashes turned out to be a Markdown parser option, and chrome localisation a template function (ADR-0034) — neither needed a phase | | `PhasePage` | the assembled page object | OpenGraph, JSON-LD, related posts, series nav | | `PhaseOutput` | the final byte stream | minification, dithering, gemtext conversion | diff --git a/docs/state.md b/docs/state.md index 71bda06..ae9fdb2 100644 --- a/docs/state.md +++ b/docs/state.md @@ -11,13 +11,15 @@ If this file disagrees with the code, the code is right and this file is a bug. | `internal/content/doc.go` | package comment | 5 | | `internal/content/content.go` | bundles: `os.Root` open, walk, frontmatter split, key/lang derivation, NFC, tag slugs, permalink building | 352 | | `internal/content/site.go` | the indexed site: lookup with language fallback, aliases, `Query` and `Run`, sections, `Sequence` | 286 | -| `internal/render/render.go` | goldmark with the typographer, per-kind template sets with site override, `Page`/`List`/`Sequence`/`head` | 305 | +| `internal/render/render.go` | goldmark with the typographer, per-kind template sets with site override, the `Partial` seam features render through, `Page`/`List`/`Sequence`/`head` | 345 | | `internal/render/chrome.go` | the engine's own words: phrase table, month names, digits, and the `t`/`num`/`day` template funcs (ADR-0034) | 105 | -| `internal/render/templates/` | reference theme: `base.html`, `page.html`, `list.html`, `theme.css` (ADR-0026) | — | +| `internal/render/templates/` | reference theme: `base.html`, `page.html`, `list.html`, `shortcodes.html`, `theme.css` (ADR-0026) | — | +| `internal/ext/shortcodes/` | first feature: `{{< name key="value" >}}` block parser and node renderer, rendering through a theme fragment (ADR-0036) | 168 | +| `cmd/khosra/wire.go` | the only list of enabled features (`extensions.md`) | 19 | | `internal/web/resolve.go` | URL → (key, lang, page, tag) or a canonical redirect: language prefix, `/en/…` fork guard, pagination, tags, trailing slash | 112 | | `internal/web/web.go` | handler: resolve, look up with fallback, section and tag listings, sequence, `/static/` (misses and refusals alike answer 404), degrade on failure | 152 | | `cmd/khosra/main.go` | flags, wiring, startup — the only place things are assembled | 53 | -| `*_test.go` | table-driven, one file per source file; symlink escape (content and static), canonical paths, language fallback, aliases, pagination, tags, sequences, chrome, typography, 404 | 1172 | +| `*_test.go` | table-driven, one file per source file; symlink escape (content and static), canonical paths, language fallback, aliases, pagination, tags, sequences, chrome, typography, shortcode escaping, 404 | 1288 | Serves a bundle at `/{section}/{slug}/`, a paginated listing per section, tag listings global and section-narrowed, sequence navigation and a series archive on any nested bundle, and `static/` verbatim. @@ -38,12 +40,12 @@ this change*. | Counter | Now | Extraction due at | What it buys | |---|---|---|---| -| Render transforms | 0 | **3** | Stage pipeline (ordered `func(ctx,*Page) error`). Typography is *not* one: it is a goldmark parser option, not a function over a page, so it buys the feature without moving the counter. Shortcodes (queue 12) will be the first real one | +| Render transforms — **page-level only** | 0 | **3** | Stage pipeline (ordered `func(ctx,*Page) error`). Parse-phase work does *not* count and must not: goldmark's extender list is already an ordered pipeline for it, so typography and shortcodes compose there (`cmd/khosra/wire.go`) and a second pipeline beside it would be pure duplication. This counts transforms over the assembled page, which nothing hosts yet — OpenGraph and JSON-LD (queue 15) are the first candidates | | Routing cases | 5 | **2** — done | Resolver at `internal/web/resolve.go`: bundle, language prefix, pagination, tag, section-narrowed tag | | Collection pages | 4 | **1** — done | Query primitive: `content.Query{Section, Tag, Lang}` + `Site.Run`. The fourth — a series archive — resolves through `Site.Sequence` instead: membership is structural and the sort ascends, so it shares the index but not the Query | | Views / output formats | 2 | **2** — due | Two template sets exist (bundle, listing); the View layer is Arc 2's third item | | Effects | 0 | **2** | Effect runner + trigger wiring (change / schedule / demand) | -| Extensions | 0 | **3** | Extension registry + wire file (`extensions.md`) | +| Extensions | 1 | **3** | Extension registry (`extensions.md`). The wire file arrived with the first feature rather than the registry — `cmd/khosra/wire.go`, one line, no struct | | Interface implementations | — | **2** | The interface itself | | Non-stdlib dependencies | 3 direct | budget in `scripts/budgets.env` | — | @@ -58,11 +60,11 @@ with a stated reason. A list nothing drains is a graveyard of known defects. | Item | Why it waits | Trigger to fix | |---|---|---| -| No mechanical check that the counters are *correct* | Accepted at the Arc 1 boundary: the coupling gate makes forgetting them impossible, which is the real failure mode, and checking the values needs code to count | 3rd transform (queue 12) | +| No mechanical check that the counters are *correct* | Accepted at the Arc 1 boundary: the coupling gate makes forgetting them impossible, which is the real failure mode, and checking the values needs code to count | The first page-level transform (queue 15), now that the transform counter means something narrower | | No mechanical gate on the untrusted boundary (ADR-0003) | Scheduled to Arc 3: nothing untrusted is read yet | The comment path — a test that untrusted input reaches no shortcode or template evaluation | | `date` stays in `Extra` after being lifted onto `Bundle.Date`, unlike `title`, `aliases`, `tags` and `order`, which are deleted | Spotted while adding `order`; the theme contract says `Extra` holds what the parser does not name, so one of the two is wrong. Harmless today — a template reading `.Extra.date` gets the raw YAML value | Whatever next reads `Extra` generically: feeds (queue 14) or `check` (17) | | Sequence resolution rescans the index on every bundle request — two passes over every key, each doing a `Lookup` | No cache exists anywhere yet, and a site of this size resolves in microseconds. Measuring first is the rule (queue 16) | The page cache (queue 16), which is the thing that makes the cost visible | -| Raw HTML in Markdown is currently omitted only because goldmark's default omits it | Verified at the Arc 1 boundary, and it is what keeps invariant 2 intact for authored content. `html.WithUnsafe()` is the obvious move when a shortcode needs to emit HTML, and it silently turns authored Markdown into an injection path | Shortcodes (queue 12) — if unsafe rendering is enabled, the trusted/untrusted split must be real code, not a default | +| Raw HTML in Markdown is omitted only because goldmark's default omits it | Shortcodes landed without unsafe mode — a call renders through a template instead (ADR-0036), so the feared trigger came and went. What remains is that nothing *stops* a later change from enabling `html.WithUnsafe()`, which would silently turn authored Markdown into an injection path | Now: a gate rejecting `WithUnsafe` anywhere in the tree, so the rule is mechanism rather than memory | ## Open questions diff --git a/docs/theme-contract.md b/docs/theme-contract.md index 422f5f6..772363a 100644 --- a/docs/theme-contract.md +++ b/docs/theme-contract.md @@ -73,6 +73,23 @@ these functions. An address is not chrome (`content-model.md`). A site root cannot add or override a phrase yet. A theme needing its own words writes them in its own block; site-supplied strings wait for the settings cascade (`ideas/deferred-decisions.md`). +## Shortcode fragments + +`templates/shortcodes.html` holds one named template per shortcode, and that is where a shortcode's markup +lives — the engine parses the call and supplies its arguments, never any HTML (ADR-0036). + +| Shortcode | Template | Receives | +|---|---|---| +| `{{< figure src="…" alt="…" caption="…" >}}` | `figure` | `.src`, `.alt`, `.caption` — every argument as written, escaped on output | + +Arguments arrive as strings and are escaped by `html/template` like any other data, which is what keeps an +author's text out of the markup. A call whose template is missing renders nothing and logs; it never fails +the page. + +The set is overlaid the same way as the page kinds: a site's `templates/shortcodes.html` is parsed after +the embedded one, so redefining `figure` replaces it and any fragment left alone is inherited. Argument +names are contract, added but never renamed. + ## The stability rule Fields and names are **added, never renamed or removed**. Absence is always legal: a template reading a @@ -137,10 +154,10 @@ two drift together. ## Overriding it A site root's `templates/` is parsed **after** the embedded set, and the last definition of a name wins, so -a theme redefines one named block and inherits the document (ADR-0019). Per kind, exactly two files are -overlaid — `base.html` and that kind's block file (`page.html` or `list.html`). Overlaying every site -template into every set would let a listing's `main` leak into bundle pages, which is the collision -per-kind sets exist to prevent. +a theme redefines one named block and inherits the document (ADR-0019). Each set is built from named files +and only those are overlaid — `base.html` plus that kind's block file (`page.html` or `list.html`), and +`shortcodes.html` on its own. Overlaying every site template into every set would let a listing's `main` +leak into bundle pages, which is the collision per-kind sets exist to prevent. `templates/theme.css` in the site root replaces the reference stylesheet entirely; there is no merging. diff --git a/internal/ext/shortcodes/doc.go b/internal/ext/shortcodes/doc.go new file mode 100644 index 0000000..83bc5a9 --- /dev/null +++ b/internal/ext/shortcodes/doc.go @@ -0,0 +1,7 @@ +// Package shortcodes expands `{{< name key="value" >}}` on its own line into a theme fragment. +// +// Contributes: a Markdown block parser and node renderer (PhaseParse). +// Cascade keys: none. +// Contract fields: a template per shortcode name in templates/shortcodes.html, receiving its arguments. +// Not doing: inline shortcodes, file inclusion, galleries — each waits for a second real use. +package shortcodes diff --git a/internal/ext/shortcodes/shortcodes.go b/internal/ext/shortcodes/shortcodes.go new file mode 100644 index 0000000..a4bad95 --- /dev/null +++ b/internal/ext/shortcodes/shortcodes.go @@ -0,0 +1,161 @@ +package shortcodes + +import ( + "log/slog" + "strings" + + "github.com/yuin/goldmark" + "github.com/yuin/goldmark/ast" + "github.com/yuin/goldmark/parser" + "github.com/yuin/goldmark/renderer" + "github.com/yuin/goldmark/text" + "github.com/yuin/goldmark/util" + + "khosra/internal/render" +) + +// open and close delimit a call. Chosen to be something no Markdown construct claims and no author types +// by accident; the syntax is a disk contract, so it does not change (ADR-0036). +const ( + opener = "{{<" + closer = ">}}" +) + +// New returns the Markdown extension, rendering each call through partial. +// +// The feature never writes markup: it hands the call's name and arguments to a theme template of the same +// name and writes whatever comes back (ADR-0036). +func New(partial render.Partial) goldmark.Extender { + return extension{partial: partial} +} + +type extension struct { + partial render.Partial +} + +// Extend registers the block parser and the node renderer. Priorities sit above goldmark's paragraph +// parser so a line that is only a call never becomes a paragraph. +func (e extension) Extend(md goldmark.Markdown) { + md.Parser().AddOptions(parser.WithBlockParsers( + util.Prioritized(blocks{}, 100))) + md.Renderer().AddOptions(renderer.WithNodeRenderers( + util.Prioritized(fragments{partial: e.partial}, 100))) +} + +// kind identifies a parsed call in the tree. +var kind = ast.NewNodeKind("Shortcode") + +// node is one call: everything the renderer needs, and nothing from the source bytes. +type node struct { + ast.BaseBlock + name string + args map[string]string +} + +func (n *node) Kind() ast.NodeKind { return kind } + +func (n *node) Dump(source []byte, level int) { ast.DumpHelper(n, source, level, nil, nil) } + +// blocks parses a line that is nothing but a call. +type blocks struct{} + +func (blocks) Trigger() []byte { return []byte{'{'} } + +func (blocks) Open(parent ast.Node, reader text.Reader, pc parser.Context) (ast.Node, parser.State) { + line, seg := reader.PeekLine() + name, args, ok := parse(string(line)) + if !ok { + return nil, parser.NoChildren + } + reader.Advance(seg.Len() - 1) + return &node{name: name, args: args}, parser.NoChildren +} + +// Continue never runs: a call is one line, closed as soon as it opens. +func (blocks) Continue(n ast.Node, reader text.Reader, pc parser.Context) parser.State { + return parser.Close +} + +func (blocks) Close(n ast.Node, reader text.Reader, pc parser.Context) {} + +func (blocks) CanInterruptParagraph() bool { return true } + +func (blocks) CanAcceptIndentedLine() bool { return false } + +// fragments renders a parsed call through the theme. +type fragments struct { + partial render.Partial +} + +func (f fragments) RegisterFuncs(reg renderer.NodeRendererFuncRegisterer) { + reg.Register(kind, f.render) +} + +// render writes the theme's fragment for this call. +// +// A missing or broken template logs and renders nothing: a shortcode is content decoration, and one typo +// in a bundle must not take a page down (extensions.md rule 5, ADR-0029). +func (f fragments) render(w util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) { + if !entering { + return ast.WalkContinue, nil + } + call := n.(*node) + out, err := f.partial(call.name, call.args) + if err != nil { + slog.Error("skipping shortcode", "name", call.name, "err", err) + return ast.WalkContinue, nil + } + if _, err := w.Write(out); err != nil { + return ast.WalkStop, err + } + return ast.WalkContinue, nil +} + +// parse reads one line as a call, reporting false for anything else. +// +// The whole line must be the call, and every argument is key="value" — one spelling, so there is nothing +// to guess and no half-parsed state. Values are returned raw; escaping is the template's job, which is +// what keeps an author's text out of the markup (ADR-0036). +func parse(line string) (name string, args map[string]string, ok bool) { + body, found := strings.CutPrefix(strings.TrimSpace(line), opener) + if !found { + return "", nil, false + } + body, found = strings.CutSuffix(strings.TrimSpace(body), closer) + if !found { + return "", nil, false + } + body = strings.TrimSpace(body) + name, rest, _ := strings.Cut(body, " ") + if name == "" || strings.ContainsAny(name, `="`) { + return "", nil, false + } + args = map[string]string{} + for rest = strings.TrimSpace(rest); rest != ""; { + key, value, remainder, valid := argument(rest) + if !valid { + return "", nil, false + } + args[key] = value + rest = remainder + } + return name, args, true +} + +// argument reads one key="value" pair and returns what follows it. +func argument(s string) (key, value, rest string, ok bool) { + key, after, found := strings.Cut(s, "=") + key = strings.TrimSpace(key) + if !found || key == "" || strings.ContainsAny(key, `" `) { + return "", "", "", false + } + quoted, found := strings.CutPrefix(after, `"`) + if !found { + return "", "", "", false + } + value, rest, found = strings.Cut(quoted, `"`) + if !found { + return "", "", "", false + } + return key, value, strings.TrimSpace(rest), true +} diff --git a/internal/ext/shortcodes/shortcodes_test.go b/internal/ext/shortcodes/shortcodes_test.go new file mode 100644 index 0000000..c1efcc4 --- /dev/null +++ b/internal/ext/shortcodes/shortcodes_test.go @@ -0,0 +1,132 @@ +package shortcodes + +import ( + "strings" + "testing" + "testing/fstest" + + "github.com/yuin/goldmark" + + "khosra/internal/content" + "khosra/internal/render" +) + +func TestParseAcceptsOnlyAWholeLineCall(t *testing.T) { + name, args, ok := parse(` {{< figure src="a.jpg" alt="A cat" >}} `) + if !ok || name != "figure" { + t.Fatalf("parse gave %q %v ok=%v", name, args, ok) + } + if args["src"] != "a.jpg" || args["alt"] != "A cat" { + t.Errorf("args = %v", args) + } + if _, _, ok := parse(`{{< figure src="a.jpg" >}} and then prose`); ok { + t.Error("a call must be the whole line, so trailing prose is not a call") + } + for _, line := range []string{ + "plain prose", + "{{< figure", // unterminated + `{{< src="a.jpg" >}}`, // no name + `{{< figure src=a.jpg >}}`, // unquoted value + `{{< figure src="unclosed >}}`, // unbalanced quote + "{{<>}}", // empty + } { + if _, _, ok := parse(line); ok { + t.Errorf("parse accepted %q", line) + } + } +} + +// wired builds a real Renderer wired to this extension, the way cmd does. +func wired(t *testing.T, siteFS fstest.MapFS) *render.Renderer { + t.Helper() + var fsys fstest.MapFS + if siteFS != nil { + fsys = siteFS + } + r, err := render.New(fsys, func(p render.Partial) []goldmark.Extender { + return []goldmark.Extender{New(p)} + }) + if err != nil { + t.Fatal(err) + } + return r +} + +func body(t *testing.T, r *render.Renderer, markdown string) string { + t.Helper() + b, err := content.Parse("posts/x.md", []byte("---\ntitle: X\n---\n"+markdown)) + if err != nil { + t.Fatal(err) + } + out, err := r.Bundle(b, "en", nil, nil) + if err != nil { + t.Fatal(err) + } + return string(out) +} + +func TestFigureRendersThroughTheThemeFragment(t *testing.T) { + got := body(t, wired(t, nil), "Before.\n\n{{< figure src=\"cat.jpg\" alt=\"A cat\" caption=\"Sleeping\" >}}\n\nAfter.\n") + for _, want := range []string{ + "
", `A cat`, "
Sleeping
", "
", + } { + if !strings.Contains(got, want) { + t.Errorf("missing %q:\n%s", want, got) + } + } + if strings.Contains(got, "

") || strings.Contains(got, "{{<") { + t.Errorf("a call on its own line is a block, not paragraph text:\n%s", got) + } +} + +func TestAnAuthorsArgumentCannotBecomeMarkup(t *testing.T) { + // The security property of ADR-0036, on a call that really parses: output is a template's, so hostile + // argument text arrives as escaped data in whichever context it lands in. + got := body(t, wired(t, nil), `{{< figure src="ok.jpg" alt="" >}}`+"\n") + if strings.Contains(got, "\n") + if strings.Contains(got, "onerror") && !strings.Contains(got, """) { + t.Errorf("a malformed call must stay escaped text, not markup:\n%s", got) + } + if !strings.Contains(got, "raw HTML omitted") { + t.Errorf("authored raw HTML must still be dropped:\n%s", got) + } +} + +func TestAnUnknownShortcodeDegradesToNothing(t *testing.T) { + got := body(t, wired(t, nil), "{{< nosuchthing key=\"v\" >}}\n\nStill here.\n") + if !strings.Contains(got, "Still here.") { + t.Errorf("the rest of the page must survive:\n%s", got) + } + if strings.Contains(got, "nosuchthing") { + t.Errorf("a missing fragment renders nothing, not its own name:\n%s", got) + } +} + +func TestASiteRedefinesOneFragment(t *testing.T) { + site := fstest.MapFS{ + "templates/shortcodes.html": {Data: []byte(`{{define "figure"}}
{{.src}}
{{end}}`)}, + } + got := body(t, wired(t, site), "{{< figure src=\"cat.jpg\" >}}\n") + if !strings.Contains(got, `
cat.jpg
`) { + t.Errorf("the site's fragment should win:\n%s", got) + } + if strings.Contains(got, "
") { + t.Error("the embedded fragment should have been replaced, not appended") + } +} diff --git a/internal/render/chrome_test.go b/internal/render/chrome_test.go index 78f30d2..a1456f4 100644 --- a/internal/render/chrome_test.go +++ b/internal/render/chrome_test.go @@ -61,7 +61,7 @@ func TestDatesReadInTheirOwnScript(t *testing.T) { } func TestTypographerSmoothsProseAndLeavesCodeAlone(t *testing.T) { - r, err := New(nil) + r, err := New(nil, nil) if err != nil { t.Fatal(err) } @@ -86,7 +86,7 @@ func TestTypographerSmoothsProseAndLeavesCodeAlone(t *testing.T) { } func TestMachineReadableOutputStaysASCII(t *testing.T) { - r, err := New(nil) + r, err := New(nil, nil) if err != nil { t.Fatal(err) } diff --git a/internal/render/render.go b/internal/render/render.go index a8f580e..e079f64 100644 --- a/internal/render/render.go +++ b/internal/render/render.go @@ -107,55 +107,95 @@ type Alternate struct { type Renderer struct { // Two sets, not one: base plus the block that kind of page defines. A single set would have two // definitions of "main" fighting, which is why per-type sets are the shape (ADR-0019). - page *template.Template - list *template.Template - md goldmark.Markdown - style template.CSS + page *template.Template + list *template.Template + // partials are named fragments a feature renders through, so no feature decides markup (ADR-0036). + partials *template.Template + md goldmark.Markdown + style template.CSS } +// Partial renders a named fragment with the arguments a feature parsed. A feature under internal/ext is +// handed one of these at wiring time, because markup belongs to the theme and a feature must not write any +// (ADR-0036). +// +// Arguments are strings because that is what a shortcode call carries. A feature needing richer data is +// the reason to widen this, not a reason to have made it `any` in advance. +type Partial func(name string, args map[string]string) ([]byte, error) + // New parses the theme and prepares the Markdown converter. // // siteFS may be nil, in which case only the embedded reference theme is used. A malformed template is a // startup failure rather than a request-time one, so this returns an error the caller treats as fatal. -func New(siteFS fs.FS) (*Renderer, error) { - page, err := parseSet(siteFS, "templates/page.html") +// +// extend is the seam features plug into: it receives the renderer's Partial and returns the Markdown +// extensions to enable. A callback rather than a parameter of feature types, because internal/render must +// not import internal/ext — only cmd knows which features a build includes (conventions.md, ADR-0036). It +// may be nil. +func New(siteFS fs.FS, extend func(Partial) []goldmark.Extender) (*Renderer, error) { + page, err := parseSet(siteFS, "templates/base.html", "templates/page.html") if err != nil { return nil, fmt.Errorf("bundle templates: %w", err) } - list, err := parseSet(siteFS, "templates/list.html") + list, err := parseSet(siteFS, "templates/base.html", "templates/list.html") if err != nil { return nil, fmt.Errorf("listing templates: %w", err) } + partials, err := parseSet(siteFS, "templates/shortcodes.html") + if err != nil { + return nil, fmt.Errorf("partial templates: %w", err) + } css, err := readStyle(siteFS) if err != nil { return nil, err } + r := &Renderer{page: page, list: list, partials: partials, style: css} // The typographer smooths quotes, dashes and ellipses in authored prose and leaves code spans alone, // because it works on the parsed tree rather than the text. That is the only change the engine makes to // an author's words (ADR-0034), and it is a parser option rather than a render transform, so it does // not move the transforms counter. - md := goldmark.New(goldmark.WithExtensions(extension.Typographer)) - return &Renderer{page: page, list: list, md: md, style: css}, nil + // + // Raw HTML stays disabled — goldmark's default — so the only HTML a page carries comes from a template + // (ADR-0036, invariant 2). Nothing here may enable html.WithUnsafe. + extensions := []goldmark.Extender{extension.Typographer} + if extend != nil { + extensions = append(extensions, extend(r.Partial)...) + } + r.md = goldmark.New(goldmark.WithExtensions(extensions...)) + return r, nil } -// parseSet builds one kind of page: the embedded base and block, then the site's versions of exactly -// those two files parsed after them. +// Partial renders one named fragment. A missing template is an error the caller degrades on, never a +// failed request (extensions.md rule 5). +func (r *Renderer) Partial(name string, args map[string]string) ([]byte, error) { + if r.partials.Lookup(name) == nil { + return nil, fmt.Errorf("no template named %q", name) + } + var out bytes.Buffer + if err := r.partials.ExecuteTemplate(&out, name, args); err != nil { + return nil, fmt.Errorf("partial %s: %w", name, err) + } + return out.Bytes(), nil +} + +// parseSet builds one set from the named embedded templates, then the site's versions of exactly those +// files parsed after them. // // Parse order is the whole mechanism — the last definition of a name wins — so a site redefines one -// named block and inherits the rest (ADR-0019). Only the same two names are overlaid: overlaying every -// site template into every set would let a listing's "main" leak into bundle pages, which is the -// collision per-kind sets exist to prevent. -func parseSet(siteFS fs.FS, kind string) (*template.Template, error) { +// named block and inherits the rest (ADR-0019). Only the files this set is built from are overlaid: +// overlaying every site template into every set would let a listing's "main" leak into bundle pages, +// which is the collision per-kind sets exist to prevent. +func parseSet(siteFS fs.FS, names ...string) (*template.Template, error) { // Funcs are attached before anything is parsed, so the chrome helpers are available to a site // override's blocks as well as the embedded ones (ADR-0034). - set, err := template.New("theme").Funcs(funcs).ParseFS(themeFS, "templates/base.html", kind) + set, err := template.New("theme").Funcs(funcs).ParseFS(themeFS, names...) if err != nil { return nil, fmt.Errorf("parse embedded: %w", err) } if siteFS == nil { return set, nil } - for _, name := range []string{"templates/base.html", kind} { + for _, name := range names { if _, err := fs.Stat(siteFS, name); err != nil { continue } diff --git a/internal/render/render_test.go b/internal/render/render_test.go index fef5600..593f3fc 100644 --- a/internal/render/render_test.go +++ b/internal/render/render_test.go @@ -9,7 +9,7 @@ import ( ) func TestBundleRendersMarkdownIntoTheTheme(t *testing.T) { - r, err := New(nil) + r, err := New(nil, nil) if err != nil { t.Fatal(err) } @@ -33,7 +33,7 @@ func TestBundleRendersMarkdownIntoTheTheme(t *testing.T) { } func TestBundleWithoutTitleFallsBackToKey(t *testing.T) { - r, err := New(nil) + r, err := New(nil, nil) if err != nil { t.Fatal(err) } @@ -54,7 +54,7 @@ func TestSiteOverridesOneBlockAndInheritsTheRest(t *testing.T) { siteFS := fstest.MapFS{ "templates/page.html": {Data: []byte(`{{define "main"}}
{{.Title}}
{{end}}`)}, } - r, err := New(siteFS) + r, err := New(siteFS, nil) if err != nil { t.Fatal(err) } @@ -82,7 +82,7 @@ func TestAListingOverrideDoesNotLeakIntoBundlePages(t *testing.T) { siteFS := fstest.MapFS{ "templates/list.html": {Data: []byte(`{{define "main"}}LISTING ONLY{{end}}`)}, } - r, err := New(siteFS) + r, err := New(siteFS, nil) if err != nil { t.Fatal(err) } @@ -101,7 +101,7 @@ func TestAListingOverrideDoesNotLeakIntoBundlePages(t *testing.T) { func TestSiteStylesheetReplacesTheReferenceOne(t *testing.T) { siteFS := fstest.MapFS{"templates/theme.css": {Data: []byte("body{color:rebeccapurple}")}} - r, err := New(siteFS) + r, err := New(siteFS, nil) if err != nil { t.Fatal(err) } diff --git a/internal/render/templates/shortcodes.html b/internal/render/templates/shortcodes.html new file mode 100644 index 0000000..6ba5188 --- /dev/null +++ b/internal/render/templates/shortcodes.html @@ -0,0 +1,8 @@ +{{define "figure" -}} +
+{{.alt}} +{{- if .caption}} +
{{.caption}}
+{{- end}} +
+{{- end}} diff --git a/internal/web/web_test.go b/internal/web/web_test.go index 78818a4..a789a8f 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -24,7 +24,7 @@ func testHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - r, err := render.New(nil) + r, err := render.New(nil, nil) if err != nil { t.Fatal(err) } @@ -72,7 +72,7 @@ func multilingualHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - r, err := render.New(nil) + r, err := render.New(nil, nil) if err != nil { t.Fatal(err) } @@ -124,7 +124,7 @@ func aliasHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - r, err := render.New(nil) + r, err := render.New(nil, nil) if err != nil { t.Fatal(err) } @@ -171,7 +171,7 @@ func listingHandler(t *testing.T, n int) http.Handler { if err != nil { t.Fatal(err) } - r, err := render.New(nil) + r, err := render.New(nil, nil) if err != nil { t.Fatal(err) } @@ -239,7 +239,7 @@ func TestStaticFilesAreServedAndDirectoriesAreNot(t *testing.T) { if err != nil { t.Fatal(err) } - r, err := render.New(fsys) + r, err := render.New(fsys, nil) if err != nil { t.Fatal(err) } @@ -280,7 +280,7 @@ func TestAStaticPathThatEscapesTheRootIs404(t *testing.T) { if err != nil { t.Fatal(err) } - r, err := render.New(fsys) + r, err := render.New(fsys, nil) if err != nil { t.Fatal(err) } @@ -313,7 +313,7 @@ func seriesHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - r, err := render.New(nil) + r, err := render.New(nil, nil) if err != nil { t.Fatal(err) } @@ -403,7 +403,7 @@ func tagHandler(t *testing.T) http.Handler { if err != nil { t.Fatal(err) } - r, err := render.New(nil) + r, err := render.New(nil, nil) if err != nil { t.Fatal(err) }