mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 22:10:25 +00:00
09ecc5e02f
* fix(diff): version writers never diff text that is not valid UTF-8 (BUG-3839)
go-diff works on runes and slices its input at byte offsets derived from
them, so text that is not valid UTF-8 near an edit either panics in
CreatePatch (slice bounds out of range) or builds a patch that does not
reproduce the old text: "hello world" -> "hello \xff world" reverse-applied
to "hello d", with no error. Any invalid byte does it, lone \xff and \x80 as
well as truncated sequences.
diff.ReversePatch returns (patch, ok), false when either side is not valid
UTF-8, and every version writer stores the full body in that case: the item
edit and the throttled rebase (items.go), the import's attachment remap
(attachments.go) and the legacy document edit (documents.go). The full body
is stored losslessly on SQLite; Postgres refuses invalid UTF-8 in the
column, so there the body write fails first and nothing is versioned. No
existing row is rewritten.
Requests cannot deliver such text (encoding/json coerces it, artifact import
rejects it, the materializer emits U+FFFD for lone surrogates), but SQLite
rows from older releases or direct edits can hold it, and an item like that
500ed on every save.
Tests: diff unit pins for both shapes, a census that production code calls
only ReversePatch, and a store pin per caller on SQLite and Postgres. With
the UTF-8 check removed every SQLite leg panics or stores a wrong version,
and the census names the reintroduced call.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* test(diff): LineCounts, FormatDiffSummary and Unified never panic on invalid UTF-8 (BUG-3839)
Pins the 200k-case fuzz measurement deterministically: the panic shapes as
seeds plus 20k generated pairs from the same seed-1 generator, with an
instrument showing the inputs reach go-diff's PatchApply panic.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* fix(diff): ApplyPatch returns an error where go-diff would panic (BUG-3839)
ApplyPatch refuses a base or a decoded patch text that is not valid UTF-8,
and turns a go-diff PatchApply panic into an error. The second covers valid
text: a well-formed patch applied over a base it was not made from panics in
about 1.5% of measured pairs, and the version walk does that after it skips
a row. Every caller (the item and document walks, the throttled-edit rebase,
the attachment remap, the tie diagnostic) already treats the error as a
patch that failed, so the row reads as patch_failed instead of a 500.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* test(store): the remap pin asserts the row it leaves over an invalid body (BUG-3839)
With ApplyPatch refusing an invalid base, the remap cannot resolve a diff
row over the invalid current body: it remaps the patch text in place, and
the row reads as patch_failed. The pin asserts exactly that; it is red when
the base check is removed (the row is then re-diffed from a fuzzy body).
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* refactor(diff): recover wraps only the go-diff call, and the remap pin is named for patch_failed (BUG-3839)
Lead rulings on BUG-3839 checkpoint 11.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* test(diff): the unchecked-patch census resolves the import name (BUG-3839)
Matches any selector on the file's local name for the diff package (alias,
method value), bare identifiers under a dot import, and skips web,
node_modules, .git and vendor only at the module root. A self-test runs the
matcher over each form.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* test(store): History past a broken row does not panic (BUG-3839)
The main-reproduction pin: a broken newest row, then an older row applied
over a base it was not made from. Red on main (d95f90850, slice bounds
panic), green here.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
* fix(diff): ApplyPatch refuses a result that is not valid UTF-8 (BUG-3839)
Over a base it was not made from, go-diff can succeed and cut a rune in
half: valid text in, "\x80" out (#2200 review r2, hunter). The rebase would
write that body (Postgres refuses it, failing the edit) and History would
show it. Pinned in the diff fuzz leg and in a mangled_result leg of the item
History, document History and throttled-rebase store tests, on both
backends; red when the check is removed. The doc comment now says an empty
patch returns its base unchecked, which is exact.
Claude-Session: https://claude.ai/code/session_01Q9WssCjFuHjSfLABoW4tmS
180 lines
6.5 KiB
Go
180 lines
6.5 KiB
Go
// Package diff provides compact text diffing for version storage.
|
|
// It uses the Myers diff algorithm via go-diff to create and apply patches.
|
|
package diff
|
|
|
|
import (
|
|
"fmt"
|
|
"net/url"
|
|
"strings"
|
|
"unicode/utf8"
|
|
|
|
difflib "github.com/sergi/go-diff/diffmatchpatch"
|
|
)
|
|
|
|
var dmp = difflib.New()
|
|
|
|
// CreatePatch computes a compact patch that transforms oldText into newText.
|
|
// The patch can be applied with ApplyPatch to recover newText from oldText.
|
|
// Both texts must be valid UTF-8 (BUG-3839; see ReversePatch).
|
|
func CreatePatch(oldText, newText string) string {
|
|
diffs := dmp.DiffMain(oldText, newText, true)
|
|
patches := dmp.PatchMake(oldText, diffs)
|
|
return dmp.PatchToText(patches)
|
|
}
|
|
|
|
// ApplyPatch applies a patch to baseText to produce the target text.
|
|
// Returns an error if the patch cannot be applied cleanly, or if the base or
|
|
// the patch's text is not valid UTF-8: go-diff slices such text at offsets it
|
|
// derives from runes, and panics or builds the wrong text (BUG-3839).
|
|
//
|
|
// go-diff also panics on valid text, when a patch is applied over a base it
|
|
// was not made from and the base holds multi-byte runes (about 1.5% of such
|
|
// pairs in the BUG-3839 measurement), or returns text that is not valid
|
|
// UTF-8. The version walk does exactly that after it skips a row, so both
|
|
// are errors here too.
|
|
//
|
|
// An empty patch returns baseText as it is, valid or not: it records a
|
|
// version byte-identical to its base, and go-diff is never reached.
|
|
func ApplyPatch(baseText, patchText string) (string, error) {
|
|
if patchText == "" {
|
|
return baseText, nil
|
|
}
|
|
if !utf8.ValidString(baseText) {
|
|
return "", fmt.Errorf("base text is not valid UTF-8")
|
|
}
|
|
if !patchTextValidUTF8(patchText) {
|
|
return "", fmt.Errorf("patch text is not valid UTF-8")
|
|
}
|
|
patches, err := dmp.PatchFromText(patchText)
|
|
if err != nil {
|
|
return "", fmt.Errorf("parse patch: %w", err)
|
|
}
|
|
result, applied, err := patchApply(patches, baseText)
|
|
if err != nil {
|
|
return "", err
|
|
}
|
|
// Check that all hunks applied
|
|
for i, ok := range applied {
|
|
if !ok {
|
|
return "", fmt.Errorf("patch hunk %d failed to apply", i)
|
|
}
|
|
}
|
|
// Over a base it was not made from, go-diff can also succeed and cut a
|
|
// rune in half: valid text in, invalid text out. No version recorded that.
|
|
if !utf8.ValidString(result) {
|
|
return "", fmt.Errorf("applying the patch produced text that is not valid UTF-8")
|
|
}
|
|
return result, nil
|
|
}
|
|
|
|
// patchTextValidUTF8 reports whether every text line of a patch decodes to
|
|
// valid UTF-8. A line is escaped, so "%FF" is the byte 0xFF once parsed; this
|
|
// decodes each line the way PatchFromText does, whose result is unexported.
|
|
func patchTextValidUTF8(patchText string) bool {
|
|
for _, line := range strings.Split(patchText, "\n") {
|
|
if line == "" {
|
|
continue
|
|
}
|
|
switch line[0] {
|
|
case ' ', '-', '+':
|
|
// PatchFromText ignores the error too: a malformed line decodes
|
|
// to "", which is valid.
|
|
text, _ := url.QueryUnescape(strings.ReplaceAll(line[1:], "+", "%2b"))
|
|
if !utf8.ValidString(text) {
|
|
return false
|
|
}
|
|
}
|
|
}
|
|
return true
|
|
}
|
|
|
|
// patchApply is go-diff's PatchApply with its panic returned as an error.
|
|
func patchApply(patches []difflib.Patch, base string) (result string, applied []bool, err error) {
|
|
defer func() {
|
|
if r := recover(); r != nil {
|
|
result, applied, err = "", nil, fmt.Errorf("go-diff panicked applying the patch: %v", r)
|
|
}
|
|
}()
|
|
result, applied = dmp.PatchApply(patches, base)
|
|
return result, applied, nil
|
|
}
|
|
|
|
// IsDiffSmaller returns true if storing a diff would be meaningfully smaller
|
|
// than storing the full content. Returns false for near-complete rewrites
|
|
// where the diff is >= 80% of the full content size.
|
|
func IsDiffSmaller(patch, fullContent string) bool {
|
|
if len(fullContent) == 0 {
|
|
return false
|
|
}
|
|
return len(patch) < (len(fullContent) * 80 / 100)
|
|
}
|
|
|
|
// CreateReversePatch computes a patch that transforms newText back into oldText.
|
|
// Used for reverse-diff version storage: current doc has latest content,
|
|
// versions store patches to go backwards.
|
|
//
|
|
// Both texts must be valid UTF-8, as for CreatePatch; production code calls
|
|
// ReversePatch, which checks (BUG-3839, TestNoUncheckedPatchCallers).
|
|
func CreateReversePatch(oldText, newText string) string {
|
|
return CreatePatch(newText, oldText)
|
|
}
|
|
|
|
// ReversePatch is CreateReversePatch for text that may not be valid UTF-8
|
|
// (BUG-3839). go-diff works on runes and slices the input at byte offsets it
|
|
// derives from them, so invalid UTF-8 near an edit either panics (slice
|
|
// bounds out of range) or yields a patch that does not reproduce oldText:
|
|
// "hello world" -> "hello \xff world" reverse-applied to "hello d". ok is
|
|
// false when either side is not valid UTF-8, and the caller stores the full
|
|
// body instead. Never sanitize and diff: the version would then record a body
|
|
// it does not hold.
|
|
func ReversePatch(oldText, newText string) (patch string, ok bool) {
|
|
if !utf8.ValidString(oldText) || !utf8.ValidString(newText) {
|
|
return "", false
|
|
}
|
|
return CreatePatch(newText, oldText), true
|
|
}
|
|
|
|
// FormatDiffSummary returns a human-readable summary of changes. Its "lines"
|
|
// are character-diff CHUNKS (a one-character edit reads +1/-1); LineCounts is
|
|
// the line-level count the version rows store (PLAN-2348 U2).
|
|
func FormatDiffSummary(oldText, newText string) string {
|
|
oldLines := strings.Count(oldText, "\n")
|
|
newLines := strings.Count(newText, "\n")
|
|
added := 0
|
|
removed := 0
|
|
diffs := dmp.DiffMain(oldText, newText, false)
|
|
for _, d := range diffs {
|
|
switch d.Type {
|
|
case difflib.DiffInsert:
|
|
added += strings.Count(d.Text, "\n") + 1
|
|
case difflib.DiffDelete:
|
|
removed += strings.Count(d.Text, "\n") + 1
|
|
}
|
|
}
|
|
_ = oldLines
|
|
_ = newLines
|
|
return fmt.Sprintf("+%d/-%d lines", added, removed)
|
|
}
|
|
|
|
// LineCounts returns how many lines a line-level diff of oldText → newText
|
|
// adds and removes (PLAN-2348 U2). It counts the way the web DiffView renders
|
|
// jsdiff's diffLines: each changed run is split on newlines after dropping one
|
|
// trailing newline, so a run "a\nb\n" is two lines. Both sides are Myers
|
|
// line diffs; diffmatchpatch's speed-up heuristics can choose a different,
|
|
// equally valid split on unusual inputs, in which case a count can differ from
|
|
// the rendered diff by the lines those splits trade.
|
|
func LineCounts(oldText, newText string) (added, removed int) {
|
|
a, b, lines := dmp.DiffLinesToChars(oldText, newText)
|
|
diffs := dmp.DiffCharsToLines(dmp.DiffMain(a, b, false), lines)
|
|
for _, d := range diffs {
|
|
n := strings.Count(strings.TrimSuffix(d.Text, "\n"), "\n") + 1
|
|
switch d.Type {
|
|
case difflib.DiffInsert:
|
|
added += n
|
|
case difflib.DiffDelete:
|
|
removed += n
|
|
}
|
|
}
|
|
return added, removed
|
|
}
|