amadan
A newline in a run can no longer inject Markdown into an export
Branch task 23 on discussion #11, the half of it that was still open.
The other half — hard_break vanishing in all three renderers — landed
with the typography branch and is on main already; verified rather than
assumed before writing this.
escapeMarkdown decided "the start of a line" by asking whether the byte
index was 0. That is the start of the RUN, not the start of a line, and
a run can contain a literal newline: POST /d/{sqid}/updates accepts Yjs
bytes that never met ProseMirror's schema, and a paste can carry one
too. So everything after an embedded newline was treated as
mid-sentence and written through unescaped:
"intro\n# not a heading" exported as a real heading
"intro\n- not a bullet" exported as a real bullet
"intro\n1. not a list" exported as a real ordered list
An exported document then says something its author did not write,
which is the one failure this module's own rule does not allow. Filed
as "fidelity, not security", and that is right — nothing crosses an
owner boundary — but it is fidelity in the direction that matters,
because the words change rather than disappear.
The escaper now tracks where the current line actually begins, so
"start of the line" and "start of the run" are the same question again
only when they really are. startsWithDigits is asked about the line
rather than the whole prefix for the same reason, which is what keeps
"intro\nversion 1.2" from acquiring an escape it does not need. The
non-escaping cases are asserted alongside the escaping ones: a fix that
escapes its way out of the problem would make every hyphen in every
export unreadable and would have passed a test that only checked the
injections.
Two things found while in the file:
- jsonString in the test helpers claimed to quote "the way a JSON
encoder would" and handled only " and \. A fixture with a literal
newline produced invalid JSON, so the test failed inside Parse with
"invalid character '\n' in string literal" — a fixture bug wearing
a parser bug's costume, and the reason a control character was
untestable here at all. It asks encoding/json now.
- The list and blockquote paths were already correct for a literal
newline, and were uncovered. TestMarkdownKeepsALiteralNewlineInside-
ItsBlock covers them now. It is a different question from the
escaping — the escaper decides what characters MEAN, these decide
what prefix each line CARRIES — so the fix for one does not imply
the other.
Verified: the five injection cases red first and green after; the new
block test confirmed red by removing writeList's continuation indent
("- one\ntwo" instead of "- one\n two"), because a new check never
seen to fail cannot tell "no" from "I can't see it"; all 15 export
tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WxucgadpWLE9XNWvfHBsnb
2 files changed,
+95
−16
internal/export/markdown.go+23 −3internal/export/markdown_test.go+72 −13
CI passed — run details
Intent
Attributed to this commit: 237 g CO₂e (27413747 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on worktree-discussions-triage, never backward from it. A turn that produced more than one commit books all of its tokens to the last one, and a concurrent session on this branch can add to the same figure. Some of it was priced using the shared default rate, because no model-specific factor row exists yet for at least one model involved. The Carbon view has the repo total, which is never derived from this attribution.
diff --git a/internal/export/markdown.go b/internal/export/markdown.go| index a3e2532..83417fa 100644 |
| --- a/internal/export/markdown.go |
| +++ b/internal/export/markdown.go |
| @@ -302,11 +302,30 @@ func codeSpan(s string) string { |
| // over-escaped file renders correctly and reads slightly noisily in |
| // source, while an under-escaped one renders as something the author |
| // did not write. |
| +// "The start of a line" is tracked rather than inferred from index 0. |
| +// |
| +// It used to ask whether i == 0, which is the start of the RUN and not |
| +// the start of a line. A run can contain a literal newline — nothing |
| +// stops one: POST /d/{sqid}/updates accepts Yjs bytes that never met |
| +// ProseMirror's schema, and a paste can carry one — and everything |
| +// after that newline was then treated as mid-sentence. So "#" became a |
| +// heading, "- " became a bullet and "1." became an ordered list, in the |
| +// middle of somebody's paragraph. An exported document said something |
| +// its author did not write, which is the one failure this module is |
| +// not allowed to have. |
| +// |
| +// lineStart is what makes the two questions the same question again. |
| func escapeMarkdown(s string) string { |
| var b strings.Builder |
| b.Grow(len(s)) |
| + lineStart := 0 // byte index in s where the current line begins |
| for i, r := range s { |
| + atLineStart := i == lineStart |
| switch r { |
| + case '\n': |
| + b.WriteRune(r) |
| + lineStart = i + 1 |
| + continue |
| case '\\', '`', '*', '_', '[', ']', '<', '>': |
| b.WriteByte('\\') |
| b.WriteRune(r) |
| @@ -314,14 +333,15 @@ func escapeMarkdown(s string) string { |
| // Only special at the start of a line. Inside a sentence |
| // they are ordinary punctuation, and escaping every hyphen |
| // in a document would be unreadable. |
| - if i == 0 { |
| + if atLineStart { |
| b.WriteByte('\\') |
| } |
| b.WriteRune(r) |
| case '.', ')': |
| // "1." and "1)" start an ordered list, but only after |
| - // digits at the start of a line. |
| - if i > 0 && startsWithDigits(s[:i]) { |
| + // digits at the start of a line — so the digits are counted |
| + // from the line, not from the run. |
| + if !atLineStart && startsWithDigits(s[lineStart:i]) { |
| b.WriteByte('\\') |
| } |
| b.WriteRune(r) |
diff --git a/internal/export/markdown_test.go b/internal/export/markdown_test.go| index eae167f..6c66934 100644 |
| --- a/internal/export/markdown_test.go |
| +++ b/internal/export/markdown_test.go |
| @@ -1,6 +1,7 @@ |
| package export_test |
| import ( |
| + "encoding/json" |
| "strings" |
| "testing" |
| @@ -118,6 +119,32 @@ func TestMarkdownEscapesTextThatWouldBecomeMarkup(t *testing.T) { |
| // A hyphen mid-sentence is ordinary punctuation and must NOT |
| // be escaped — over-escaping every one of them is unreadable. |
| {"well-known", "well-known"}, |
| + |
| + // A LITERAL newline inside one text run starts a new line in |
| + // the output, and everything the cases above assert about the |
| + // start of a line has to hold there too. |
| + // |
| + // escapeMarkdown decided "start of a line" by asking whether |
| + // the index was 0 — true only for the start of the whole run — |
| + // so a "#" or a "- " or a "1." after an embedded newline was |
| + // written through unescaped and became a heading, a bullet or |
| + // an ordered list in the exported file. The document then says |
| + // something its author did not write, which is the one thing |
| + // this module's own rule says it must never do. |
| + // |
| + // Reachable without a hostile client: POST /d/{sqid}/updates |
| + // takes arbitrary Yjs bytes that never met ProseMirror's |
| + // schema, and a paste can carry one too. |
| + {"intro\n# not a heading", "intro\n\\# not a heading"}, |
| + {"intro\n- not a bullet", "intro\n\\- not a bullet"}, |
| + {"intro\n+ not a bullet", "intro\n\\+ not a bullet"}, |
| + {"intro\n1. not a list", "intro\n1\\. not a list"}, |
| + {"intro\n1) not a list", "intro\n1\\) not a list"}, |
| + // And the flip side: the same characters mid-line after a |
| + // newline are still ordinary punctuation, so the fix must not |
| + // escape its way out of the problem. |
| + {"intro\nwell-known", "intro\nwell-known"}, |
| + {"intro\nversion 1.2", "intro\nversion 1.2"}, |
| } |
| for _, c := range cases { |
| js := `{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":` + jsonString(c.text) + `}]}]}` |
| @@ -229,21 +256,19 @@ func TestParseRejectsSomethingThatIsNotADocument(t *testing.T) { |
| // jsonString quotes s the way a JSON encoder would, for building the |
| // fixtures above. |
| +// |
| +// It asks the JSON encoder rather than reimplementing it. The hand- |
| +// rolled version handled only " and \, so a fixture containing a |
| +// literal newline produced invalid JSON and the test failed inside |
| +// Parse with "invalid character '\n' in string literal" — a fixture |
| +// bug wearing the costume of a parser bug, and one that made a control |
| +// character impossible to write a test for at all. |
| func jsonString(s string) string { |
| - var b strings.Builder |
| - b.WriteByte('"') |
| - for _, r := range s { |
| - switch r { |
| - case '"': |
| - b.WriteString(`\"`) |
| - case '\\': |
| - b.WriteString(`\\`) |
| - default: |
| - b.WriteRune(r) |
| - } |
| + b, err := json.Marshal(s) |
| + if err != nil { |
| + panic(err) // a string always marshals |
| } |
| - b.WriteByte('"') |
| - return b.String() |
| + return string(b) |
| } |
| // Shift+Enter's line break, in the export that has two spellings for |
| @@ -331,3 +356,37 @@ func TestMarkdownIndentsTheLineAfterAHardBreak(t *testing.T) { |
| t.Errorf("got:\n%q\nwant:\n%q", got, want) |
| } |
| } |
| + |
| +// A literal newline inside a list item or a blockquote must not walk |
| +// the rest of the line out of the block it is in. |
| +// |
| +// Same defect class as the escaping above and worth its own check, |
| +// because the fix for one does not imply the fix for the other: the |
| +// escaper decides what the characters MEAN, and these two paths decide |
| +// what prefix each line CARRIES. A newline that arrives without its |
| +// indent or its "> " leaves the second half of somebody's sentence |
| +// outside the list. |
| +func TestMarkdownKeepsALiteralNewlineInsideItsBlock(t *testing.T) { |
| + js := `{"type":"doc","content":[ |
| + {"type":"bullet_list","content":[ |
| + {"type":"list_item","content":[{"type":"paragraph","content":[ |
| + {"type":"text","text":"one\ntwo"} |
| + ]}]} |
| + ]}, |
| + {"type":"blockquote","content":[{"type":"paragraph","content":[ |
| + {"type":"text","text":"three\nfour"} |
| + ]}]} |
| + ]}` |
| + got := export.Markdown(mustParse(t, "", js)) |
| + want := strings.Join([]string{ |
| + "- one", |
| + " two", |
| + "", |
| + "> three", |
| + "> four", |
| + "", |
| + }, "\n") |
| + if got != want { |
| + t.Errorf("got:\n%q\nwant:\n%q", got, want) |
| + } |
| +} |