mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
fix(store): the migrate-to-pg preflight refuses a stored value that is not valid UTF-8, and scan-nul / repair-nul report and repair it (BUG-3222) (#1559)
* fix(store): the migrate-to-pg preflight refuses a stored value that is not valid UTF-8, and scan-nul / repair-nul report and repair it (BUG-3222) PostgreSQL refuses a text or jsonb value holding invalid UTF-8 (SQLSTATE 22021 under a UTF8 database); SQLite stores it silently. A legacy row of that class failed `pad db migrate-to-pg` partway through the copy, leaving the destination half-populated, and the NUL preflight never looked for it. The destination cast on NUL suspects caught a sliver of the class by coincidence (a value that also mentioned the escape), which BUG-3220 is about to delete. The NUL census now reads every protected column in full as a BLOB and reports invalid UTF-8 as a violation kind (merged with a NUL finding on the same row), so scan-nul lists it, repair-nul replaces each invalid sequence with U+FFFD, and the preflight refuses on it through the path it already had (migrated tables only; nothing written first). Measured on a copy of a 306 MB live database: scan-nul 0.51 s -> 1.52-1.62 s, peak RSS +10 MB. * fix(store): a value with both defects on an incomplete-key row is counted once (merged by rowid), and the JSON and SQLSTATE wording is exact (BUG-3222 codex r1) * test(store): no protected table may declare a column that shadows SQLite's rowid, which the NUL scan merges on (BUG-3222 codex r2)
This commit is contained in:
+4
-4
@@ -1106,7 +1106,7 @@ func preflightNULForMigration(src *store.Store, dst *store.Store, fromPath strin
|
||||
|
||||
if n := len(elsewhere); n > 0 {
|
||||
fmt.Fprintf(os.Stderr,
|
||||
"\nNOTE: %d value(s) carrying a NUL are in tables this migration does not copy\n"+
|
||||
"\nNOTE: %d value(s) carrying a NUL or invalid UTF-8 are in tables this migration does not copy\n"+
|
||||
"(users, platform settings, auth data). They do not block it, and\n"+
|
||||
"'%s' will repair them:\n\n", n, repairNULCommandHint)
|
||||
for _, v := range elsewhere {
|
||||
@@ -1131,8 +1131,8 @@ func preflightNULForMigration(src *store.Store, dst *store.Store, fromPath strin
|
||||
// two differ.
|
||||
fmt.Fprintf(os.Stderr, " %s (destination refused it; no layer of ours sees this one)\n", sus)
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "\nEach carries a NUL, which PostgreSQL refuses in a text or jsonb value —\n"+
|
||||
"SQLSTATE 22021 and 22P05. Migrating risks failing partway through the copy, after\n"+
|
||||
fmt.Fprintf(os.Stderr, "\nEach carries a NUL or invalid UTF-8. PostgreSQL refuses a NUL in text (SQLSTATE 22021),\n"+
|
||||
"a NUL escape in jsonb (22P05) and invalid UTF-8 (22021). Migrating risks failing partway through the copy, after\n"+
|
||||
"some workspaces have already moved, so it is refused up front.\n\n"+
|
||||
"Nothing has been migrated. Repair them first:\n\n %s\n\n"+
|
||||
"then re-run this command. To see the same list without migrating: pad db scan-nul\n",
|
||||
@@ -1146,7 +1146,7 @@ func preflightNULForMigration(src *store.Store, dst *store.Store, fromPath strin
|
||||
// running the command against a real Postgres, not by a test: the tests
|
||||
// asserted the message CONTAINED "nothing was migrated" and never read the
|
||||
// number.
|
||||
return fmt.Errorf("%d stored value(s) carry a NUL; nothing was migrated", total)
|
||||
return fmt.Errorf("%d stored value(s) carry a NUL or invalid UTF-8; nothing was migrated", total)
|
||||
}
|
||||
|
||||
// suspectFailure pairs a suspect with the destination's complaint.
|
||||
|
||||
+24
-12
@@ -42,15 +42,23 @@ func dbScanNULCmd() *cobra.Command {
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Use: "scan-nul",
|
||||
Short: "Report stored values carrying a NUL (read-only)",
|
||||
Short: "Report stored values PostgreSQL refuses: a NUL, or invalid UTF-8 (read-only)",
|
||||
Long: `Counts and locates every stored value that violates Pad's NUL invariant:
|
||||
a real NUL byte in any protected column, or a JSON escape in a JSON column
|
||||
that a JSON parser would decode to one.
|
||||
that a JSON parser would decode to one. It also reports every value that is
|
||||
not valid UTF-8. That is not a NUL, but PostgreSQL refuses it too (SQLSTATE
|
||||
22021 under a UTF8 database), so it breaks a migration in the same way.
|
||||
|
||||
Finding invalid UTF-8 means reading every stored value in full, since SQLite
|
||||
has no way to test for it in a query; on a large database that takes about as
|
||||
long as reading the database once.
|
||||
|
||||
Such rows can only have been written by a binary older than the enforcement
|
||||
that now refuses them. They are not cosmetic: their workspace exports fine and
|
||||
re-imports with a 400, and 'pad db migrate-to-pg' fails partway through the
|
||||
copy against PostgreSQL's jsonb parser.
|
||||
copy against PostgreSQL. A SQL_ASCII PostgreSQL would accept the invalid-UTF-8
|
||||
bytes; they are reported anyway, because nothing on the Pad side can tell which
|
||||
encoding a later migration will target.
|
||||
|
||||
The scan itself only reads. Opening the database does apply any pending schema
|
||||
migrations, exactly as starting the server does — pass --from to inspect a
|
||||
@@ -90,10 +98,13 @@ func dbRepairNULCmd() *cobra.Command {
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Use: "repair-nul",
|
||||
Short: "Replace stored NULs with U+FFFD (rewrites user content)",
|
||||
Long: `Rewrites every stored value 'pad db scan-nul' reports, replacing each NUL
|
||||
with U+FFFD (the Unicode replacement character) and leaving the rest of the
|
||||
value byte for byte as it was.
|
||||
Short: "Replace stored NULs and invalid UTF-8 with U+FFFD (rewrites user content)",
|
||||
Long: `Rewrites every stored value 'pad db scan-nul' reports, replacing each NUL,
|
||||
and each invalid UTF-8 byte sequence, with U+FFFD (the Unicode replacement
|
||||
character) and leaving the rest of the value byte for byte as it was. In a valid
|
||||
JSON document an invalid byte can only sit inside a string, so the document
|
||||
stays valid JSON with that string's bad bytes replaced; a value that was not
|
||||
valid JSON to begin with is not made valid by this.
|
||||
|
||||
THIS CHANGES USER CONTENT. It is a separate command, and never part of a
|
||||
migration, for that reason: a migration that rewrote stored text would decide
|
||||
@@ -151,8 +162,8 @@ row.`,
|
||||
}
|
||||
|
||||
if !force {
|
||||
fmt.Fprintf(os.Stderr, "\nThis will rewrite the %d value(s) above, replacing each NUL with "+
|
||||
"U+FFFD, and inspect %d suspect value(s) — rewriting only those that hide a NUL behind "+
|
||||
fmt.Fprintf(os.Stderr, "\nThis will rewrite the %d value(s) above, replacing each NUL and each "+
|
||||
"invalid UTF-8 byte sequence with U+FFFD, and inspect %d suspect value(s) — rewriting only those that hide a NUL behind "+
|
||||
"a repeated key.\n", scan.Total(), len(scan.Suspects))
|
||||
fmt.Fprintf(os.Stderr, "Run with --force to skip this confirmation, or press Ctrl+C to abort.\n")
|
||||
fmt.Fprintf(os.Stderr, "Continue? [y/N] ")
|
||||
@@ -245,7 +256,8 @@ func resolveNULToolsTarget(fromPath *string) (proceed bool, err error) {
|
||||
if os.Getenv("PAD_DB_DRIVER") == "postgres" {
|
||||
fmt.Fprintln(os.Stderr,
|
||||
"This deployment is PostgreSQL, which refuses these values natively (SQLSTATE 22021 for a\n"+
|
||||
"NUL in text, 22P05 for the escape reaching jsonb), so no stored row can carry one.\n"+
|
||||
"NUL in text or invalid UTF-8, 22P05 for the escape reaching jsonb), so no stored row can\n"+
|
||||
"carry one.\n"+
|
||||
"Nothing to scan or repair.")
|
||||
return false, nil
|
||||
}
|
||||
@@ -294,12 +306,12 @@ func printNULScanReport(w io.Writer, report *store.NULScanReport, dbPath string)
|
||||
}
|
||||
|
||||
if report.Total() == 0 {
|
||||
fmt.Fprintln(w, "No values carrying a NUL were found.")
|
||||
fmt.Fprintln(w, "No values carrying a NUL or invalid UTF-8 were found.")
|
||||
printNULSuspects(w, report)
|
||||
return
|
||||
}
|
||||
|
||||
fmt.Fprintf(w, "\nFound %d value(s) carrying a NUL:\n\n", report.Total())
|
||||
fmt.Fprintf(w, "\nFound %d value(s) carrying a NUL or invalid UTF-8:\n\n", report.Total())
|
||||
|
||||
byColumn := report.ByColumn()
|
||||
for _, key := range sortedCountKeys(byColumn) {
|
||||
|
||||
@@ -656,3 +656,79 @@ func plantActivitySuspect(t *testing.T, dbPath, value string) {
|
||||
t.Fatalf("plant: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPreflightRefusesInvalidUTF8 is BUG-3222: PostgreSQL refuses a value that
|
||||
// is not valid UTF-8 (SQLSTATE 22021), so the preflight refuses on one in a
|
||||
// table the migration copies, and reports without blocking one in a table it
|
||||
// does not.
|
||||
//
|
||||
// The last leg is the case BUG-3220 must keep refused after deleting the NUL
|
||||
// suspect path: an invalid byte in a JSON value that ALSO carries the escape
|
||||
// text as a harmless literal. Before BUG-3222 only the destination cast on
|
||||
// suspects caught it, by coincidence.
|
||||
func TestPreflightRefusesInvalidUTF8(t *testing.T) {
|
||||
bad := string([]byte{0x80})
|
||||
backslash := textguard.EscNUL[:1]
|
||||
|
||||
newSource := func(t *testing.T) (*store.Store, string, string) {
|
||||
t.Helper()
|
||||
dbPath := filepath.Join(t.TempDir(), "src.db")
|
||||
s, err := store.New(dbPath)
|
||||
if err != nil {
|
||||
t.Fatalf("open source: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { s.Close() })
|
||||
ws, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "UTF8"})
|
||||
if err != nil {
|
||||
t.Fatalf("create workspace: %v", err)
|
||||
}
|
||||
return s, dbPath, ws.ID
|
||||
}
|
||||
|
||||
t.Run("control: valid multibyte text passes", func(t *testing.T) {
|
||||
s, path, wsID := newSource(t)
|
||||
plantNULInWorkspaceName(t, path, wsID, "é"+string(rune(0x4E2D))+string(rune(0x1F600)))
|
||||
if err := preflightNULForMigration(s, nil, path); err != nil {
|
||||
t.Fatalf("the preflight refused valid UTF-8: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("invalid UTF-8 in a migrated text column refuses", func(t *testing.T) {
|
||||
s, path, wsID := newSource(t)
|
||||
plantNULInWorkspaceName(t, path, wsID, "bad"+bad+"name")
|
||||
err := preflightNULForMigration(s, nil, path)
|
||||
if err == nil || !strings.HasPrefix(err.Error(), "1 stored value") || !strings.Contains(err.Error(), "nothing was migrated") {
|
||||
t.Fatalf("want a refusal of exactly one value, got %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("invalid UTF-8 in a table the migration does not copy is not blocking", func(t *testing.T) {
|
||||
s, path, _ := newSource(t)
|
||||
plantPlatformSetting(t, path, "branding", "site"+bad+"name")
|
||||
scan, err := s.ScanNUL()
|
||||
if err != nil {
|
||||
t.Fatalf("scan: %v", err)
|
||||
}
|
||||
if scan.Total() != 1 {
|
||||
t.Fatalf("the scan should report the row; got %v", scan.Violations)
|
||||
}
|
||||
if err := preflightNULForMigration(s, nil, path); err != nil {
|
||||
t.Errorf("the preflight blocked on a table it does not copy: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("the BUG-3220 sliver: invalid byte plus the escape as a literal, in JSON", func(t *testing.T) {
|
||||
s, path, wsID := newSource(t)
|
||||
plantWorkspaceSettings(t, path, wsID, `{"a":"`+bad+`","b":"x`+backslash+textguard.EscNUL+`y"}`)
|
||||
scan, err := s.ScanNUL()
|
||||
if err != nil {
|
||||
t.Fatalf("scan: %v", err)
|
||||
}
|
||||
if scan.Total() != 1 || !scan.Violations[0].InvalidUTF8 {
|
||||
t.Fatalf("want one invalid-UTF-8 violation, got %v", scan.Violations)
|
||||
}
|
||||
if err := preflightNULForMigration(s, nil, path); err == nil {
|
||||
t.Fatal("the preflight let the sliver through")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
+9
-5
@@ -349,17 +349,21 @@ commands below are for.
|
||||
|
||||
### Finding and repairing affected rows
|
||||
|
||||
`pad db scan-nul` reports every stored value carrying a NUL — which table and
|
||||
column, which row, and which workspace — and changes nothing:
|
||||
`pad db scan-nul` reports every stored value carrying a NUL, and every value
|
||||
that is not valid UTF-8 — which table and column, which row, and which
|
||||
workspace — and changes nothing. Invalid UTF-8 is not a NUL, but PostgreSQL
|
||||
refuses it too (SQLSTATE 22021 under a UTF8 database) and SQLite stores it
|
||||
silently, so it breaks a migration in the same way. Finding it means reading
|
||||
every stored value in full; on a 306 MB database that added about a second.
|
||||
|
||||
```bash
|
||||
pad db scan-nul # the live database
|
||||
pad db scan-nul --from /backups/pad-20260901.db # or a backup file
|
||||
```
|
||||
|
||||
`pad db repair-nul` rewrites those values, replacing each NUL with U+FFFD (the
|
||||
Unicode replacement character) and leaving the rest of the value byte for byte
|
||||
as it was. **It changes stored content**, which is why it is a separate command
|
||||
`pad db repair-nul` rewrites those values, replacing each NUL and each invalid
|
||||
UTF-8 byte sequence with U+FFFD (the Unicode replacement character) and leaving
|
||||
the rest of the value byte for byte as it was. **It changes stored content**, which is why it is a separate command
|
||||
and never part of a migration — running a schema upgrade should not rewrite
|
||||
your text on your behalf. Run the scan first; it is the dry run. Running the
|
||||
repair twice is safe.
|
||||
|
||||
@@ -4,6 +4,8 @@ import (
|
||||
"database/sql"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/textguard"
|
||||
)
|
||||
@@ -208,11 +210,24 @@ func (s *Store) repairOneNUL(v NULViolation) (bool, error) {
|
||||
}
|
||||
|
||||
isJSON := nulColumnIsJSON(v.Table, v.Column)
|
||||
if !textguard.ParameterRefused(value, isJSON) {
|
||||
invalidUTF8 := !utf8.ValidString(value)
|
||||
if !invalidUTF8 && !textguard.ParameterRefused(value, isJSON) {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
repaired := textguard.Repair(value, isJSON)
|
||||
// Invalid UTF-8 first (BUG-3222): each invalid byte sequence becomes
|
||||
// U+FFFD, the same replacement the NUL repair uses. In a VALID JSON
|
||||
// document an invalid byte can only sit inside a string, so the result is
|
||||
// the same document with that string's bad bytes replaced
|
||||
// (TestRepairInvalidUTF8KeepsJSONValid). A value that was already MALFORMED
|
||||
// JSON stays malformed: PostgreSQL refuses it for that reason too (22P02),
|
||||
// which is outside this census and not made worse by the repair (codex r1).
|
||||
// Then the NUL repair, on the now-valid text.
|
||||
repaired := value
|
||||
if invalidUTF8 {
|
||||
repaired = strings.ToValidUTF8(repaired, textguard.Replacement)
|
||||
}
|
||||
repaired = textguard.Repair(repaired, isJSON)
|
||||
if repaired == value {
|
||||
// textguard.Repair is required to change any refused value; a no-op
|
||||
// here would mean the predicate and the repair disagree, and looping
|
||||
|
||||
@@ -55,6 +55,16 @@ type NULViolation struct {
|
||||
// pass for each.
|
||||
RawNUL bool
|
||||
EscapedNUL bool
|
||||
// InvalidUTF8 marks a value that is not valid UTF-8 (BUG-3222). It is
|
||||
// not a NUL defect, but PostgreSQL refuses it too (SQLSTATE 22021 under
|
||||
// a UTF8 database), and a migration that meets one fails
|
||||
// partway through the copy exactly as a NUL does. So it rides the same
|
||||
// census, repair and preflight rather than a second command pair.
|
||||
InvalidUTF8 bool
|
||||
// rowid is the row's SQLite rowid, internal to the scan: it identifies the
|
||||
// row even when KeyIncomplete does not let Key do so, which is what lets
|
||||
// the NUL and invalid-UTF-8 findings for one value merge (BUG-3222).
|
||||
rowid int64
|
||||
// KeyIncomplete marks a row one of whose key columns is NULL, so Key does
|
||||
// not address it. SQLite permits NULL in a declared PRIMARY KEY that is
|
||||
// neither INTEGER PRIMARY KEY nor NOT NULL, which no other engine does.
|
||||
@@ -69,13 +79,17 @@ func (v NULViolation) String() string {
|
||||
for _, k := range sortedKeys(v.Key) {
|
||||
parts = append(parts, k+"="+v.Key[k])
|
||||
}
|
||||
kind := "raw NUL"
|
||||
switch {
|
||||
case v.RawNUL && v.EscapedNUL:
|
||||
kind = "raw NUL + escaped NUL"
|
||||
case v.EscapedNUL:
|
||||
kind = "escaped NUL"
|
||||
var kinds []string
|
||||
if v.RawNUL {
|
||||
kinds = append(kinds, "raw NUL")
|
||||
}
|
||||
if v.EscapedNUL {
|
||||
kinds = append(kinds, "escaped NUL")
|
||||
}
|
||||
if v.InvalidUTF8 {
|
||||
kinds = append(kinds, "invalid UTF-8")
|
||||
}
|
||||
kind := strings.Join(kinds, " + ")
|
||||
out := fmt.Sprintf("%s.%s [%s] (%s)", v.Table, v.Column, strings.Join(parts, ", "), kind)
|
||||
if v.WorkspaceID != "" {
|
||||
out += " workspace=" + v.WorkspaceID
|
||||
@@ -206,7 +220,7 @@ func (s *Store) ScanNUL() (*NULScanReport, error) {
|
||||
if s.dialect.Driver() != DriverSQLite {
|
||||
return &NULScanReport{
|
||||
Applicable: false,
|
||||
Reason: "PostgreSQL refuses these values natively (SQLSTATE 22021 for a NUL in text, " +
|
||||
Reason: "PostgreSQL refuses these values natively (SQLSTATE 22021 for a NUL in text or invalid UTF-8, " +
|
||||
"22P05 for the escape reaching jsonb), so no stored row can carry one",
|
||||
}, nil
|
||||
}
|
||||
@@ -246,8 +260,15 @@ func (s *Store) ScanNUL() (*NULScanReport, error) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
report.Violations = append(report.Violations, found...)
|
||||
report.Suspects = append(report.Suspects, suspects...)
|
||||
|
||||
// BUG-3222: invalid UTF-8, folded into the same violation when a
|
||||
// row carries both defects so the census counts each value once.
|
||||
invalid, err := s.scanColumnUTF8(c, addr)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
report.Violations = append(report.Violations, mergeUTF8Violations(found, invalid)...)
|
||||
}
|
||||
sort.Strings(report.ColumnsAbsent)
|
||||
return report, nil
|
||||
@@ -346,7 +367,8 @@ func (s *Store) scanColumn(c nulColumn, addr tableAddressing) ([]NULViolation, [
|
||||
qt := quoteIdent(c.Table)
|
||||
qc := quoteIdent(c.Column)
|
||||
|
||||
sel := make([]string, 0, len(addr.KeyColumns)+2)
|
||||
sel := make([]string, 0, len(addr.KeyColumns)+3)
|
||||
sel = append(sel, "rowid")
|
||||
for _, k := range addr.KeyColumns {
|
||||
sel = append(sel, quoteIdent(k))
|
||||
}
|
||||
@@ -383,6 +405,8 @@ func (s *Store) scanColumn(c nulColumn, addr tableAddressing) ([]NULViolation, [
|
||||
// that is not INTEGER PRIMARY KEY or explicitly NOT NULL. Only the
|
||||
// value column is guaranteed non-NULL, by the query's own WHERE.
|
||||
dest := make([]any, 0, len(sel))
|
||||
var rowid int64
|
||||
dest = append(dest, &rowid)
|
||||
keyVals := make([]sql.NullString, len(addr.KeyColumns))
|
||||
for i := range keyVals {
|
||||
dest = append(dest, &keyVals[i])
|
||||
@@ -436,6 +460,7 @@ func (s *Store) scanColumn(c nulColumn, addr tableAddressing) ([]NULViolation, [
|
||||
Key: map[string]string{},
|
||||
WorkspaceID: wsID.String,
|
||||
RawNUL: textguard.ContainsNUL(value),
|
||||
rowid: rowid,
|
||||
}
|
||||
v.EscapedNUL = isJSON && textguard.DocumentDecodesNULAnyShape(value)
|
||||
for i, k := range addr.KeyColumns {
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"fmt"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
)
|
||||
|
||||
// scanColumnUTF8 finds every value in one protected column that is not valid
|
||||
// UTF-8 (BUG-3222).
|
||||
//
|
||||
// PostgreSQL refuses such a value in text or jsonb (SQLSTATE 22021 under a
|
||||
// UTF8 database), and SQLite stores it without complaint, so a legacy row of
|
||||
// this class breaks `pad db migrate-to-pg` partway through the copy, leaving
|
||||
// the destination half-populated, which is the failure the NUL preflight
|
||||
// exists to prevent. No HTTP door stores one today (measured on BUG-3220's
|
||||
// trail: JSON bodies decode through Go strings, multipart filenames are
|
||||
// substituted), so this is a guard for rows an earlier release or a direct
|
||||
// database edit left behind.
|
||||
//
|
||||
// NO SQL PRE-FILTER, unlike scanColumn: SQLite has no UTF-8 validity test, so
|
||||
// every non-NULL value is read as a BLOB and checked in Go. That is one full
|
||||
// read of each protected column, the same order of work as the copy it
|
||||
// guards, which reads every row anyway.
|
||||
//
|
||||
// CAST AS BLOB, not a plain read: the bytes are what is judged, and a BLOB is
|
||||
// returned exactly as stored.
|
||||
func (s *Store) scanColumnUTF8(c nulColumn, addr tableAddressing) ([]NULViolation, error) {
|
||||
sel := make([]string, 0, len(addr.KeyColumns)+3)
|
||||
sel = append(sel, "rowid")
|
||||
for _, k := range addr.KeyColumns {
|
||||
sel = append(sel, quoteIdent(k))
|
||||
}
|
||||
if addr.HasWorkspace {
|
||||
sel = append(sel, `"workspace_id"`)
|
||||
}
|
||||
qc := quoteIdent(c.Column)
|
||||
sel = append(sel, "CAST("+qc+" AS BLOB)")
|
||||
|
||||
q := fmt.Sprintf(`SELECT %s FROM %s WHERE %s IS NOT NULL`, strings.Join(sel, ", "), quoteIdent(c.Table), qc)
|
||||
rows, err := s.db.Query(q)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("scan %s.%s for invalid UTF-8: %w", c.Table, c.Column, err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
var out []NULViolation
|
||||
for rows.Next() {
|
||||
// Nullable key and workspace columns, for the reason scanColumn
|
||||
// gives: SQLite permits NULL in some PRIMARY KEY columns.
|
||||
dest := make([]any, 0, len(sel))
|
||||
var rowid int64
|
||||
dest = append(dest, &rowid)
|
||||
keyVals := make([]sql.NullString, len(addr.KeyColumns))
|
||||
for i := range keyVals {
|
||||
dest = append(dest, &keyVals[i])
|
||||
}
|
||||
var wsID sql.NullString
|
||||
if addr.HasWorkspace {
|
||||
dest = append(dest, &wsID)
|
||||
}
|
||||
var value []byte
|
||||
dest = append(dest, &value)
|
||||
if err := rows.Scan(dest...); err != nil {
|
||||
return nil, fmt.Errorf("scan %s.%s row for invalid UTF-8: %w", c.Table, c.Column, err)
|
||||
}
|
||||
if utf8.Valid(value) {
|
||||
continue
|
||||
}
|
||||
v := NULViolation{
|
||||
Table: c.Table,
|
||||
Column: c.Column,
|
||||
Key: map[string]string{},
|
||||
WorkspaceID: wsID.String,
|
||||
InvalidUTF8: true,
|
||||
rowid: rowid,
|
||||
}
|
||||
for i, k := range addr.KeyColumns {
|
||||
if !keyVals[i].Valid {
|
||||
v.KeyIncomplete = true
|
||||
continue
|
||||
}
|
||||
v.Key[k] = keyVals[i].String
|
||||
}
|
||||
out = append(out, v)
|
||||
}
|
||||
return out, rows.Err()
|
||||
}
|
||||
|
||||
// mergeUTF8Violations folds one column's invalid-UTF-8 findings into its NUL
|
||||
// findings, so a value carrying both defects is ONE violation with both kinds
|
||||
// rather than two rows the census would count twice. Matched on the rowid
|
||||
// both scans read, not on Key, which cannot identify a KeyIncomplete row
|
||||
// (codex r1). Order: the NUL findings in their order, then the UTF-8-only
|
||||
// ones in theirs.
|
||||
func mergeUTF8Violations(nul, invalid []NULViolation) []NULViolation {
|
||||
if len(invalid) == 0 {
|
||||
return nul
|
||||
}
|
||||
index := make(map[int64]int, len(nul))
|
||||
for i, v := range nul {
|
||||
index[v.rowid] = i
|
||||
}
|
||||
out := append([]NULViolation{}, nul...)
|
||||
for _, v := range invalid {
|
||||
if i, ok := index[v.rowid]; ok {
|
||||
out[i].InvalidUTF8 = true
|
||||
continue
|
||||
}
|
||||
out = append(out, v)
|
||||
}
|
||||
return out
|
||||
}
|
||||
@@ -0,0 +1,220 @@
|
||||
package store
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/PerpetualSoftware/pad/internal/textguard"
|
||||
)
|
||||
|
||||
// BUG-3222: invalid UTF-8 rides the NUL census. PostgreSQL refuses it
|
||||
// (SQLSTATE 22021), SQLite stores it, and a migration meeting one fails
|
||||
// partway through the copy.
|
||||
|
||||
// invalidByte is one byte that can never start or continue valid UTF-8 on its
|
||||
// own. Built from its value so no escape text passes through an editor.
|
||||
var invalidByte = string([]byte{0x80})
|
||||
|
||||
func TestScanAndRepairInvalidUTF8(t *testing.T) {
|
||||
t.Parallel()
|
||||
s := testStore(t)
|
||||
if s.dialect.Driver() != DriverSQLite {
|
||||
t.Skip("SQLite only")
|
||||
}
|
||||
ws := createTestWorkspace(t, s, "UTF8WS")
|
||||
col := createTestCollection(t, s, ws.ID, "Tasks")
|
||||
textRow := createTestItem(t, s, ws.ID, col.ID, "text", "body")
|
||||
jsonRow := createTestItem(t, s, ws.ID, col.ID, "json", "body")
|
||||
bothRow := createTestItem(t, s, ws.ID, col.ID, "both", "body")
|
||||
control := createTestItem(t, s, ws.ID, col.ID, "control", "body")
|
||||
|
||||
multibyte := "é" + string(rune(0x4E2D)) + string(rune(0x1F600))
|
||||
plantLegacyRows(t, s, func(raw *sql.DB) {
|
||||
mustExec(t, raw, `UPDATE items SET title = ? WHERE id = ?`, "ok"+invalidByte+"x", textRow.ID)
|
||||
mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`, `{"note":"x`+invalidByte+`y"}`, jsonRow.ID)
|
||||
mustExec(t, raw, `UPDATE items SET title = ? WHERE id = ?`, "a"+textguard.NUL+"b"+invalidByte, bothRow.ID)
|
||||
mustExec(t, raw, `UPDATE items SET title = ?, fields = ? WHERE id = ?`,
|
||||
multibyte, `{"note":"`+multibyte+`"}`, control.ID)
|
||||
})
|
||||
|
||||
report, err := s.ScanNUL()
|
||||
if err != nil {
|
||||
t.Fatalf("scan: %v", err)
|
||||
}
|
||||
got := map[string]NULViolation{}
|
||||
for _, v := range report.Violations {
|
||||
got[v.Key["id"]+"/"+v.Column] = v
|
||||
}
|
||||
for _, c := range []struct {
|
||||
key string
|
||||
wantInvalid, rawNUL bool
|
||||
}{
|
||||
{textRow.ID + "/title", true, false},
|
||||
{jsonRow.ID + "/fields", true, false},
|
||||
{bothRow.ID + "/title", true, true},
|
||||
} {
|
||||
v, ok := got[c.key]
|
||||
if !ok {
|
||||
t.Errorf("%s: not reported", c.key)
|
||||
continue
|
||||
}
|
||||
if v.InvalidUTF8 != c.wantInvalid || v.RawNUL != c.rawNUL {
|
||||
t.Errorf("%s: InvalidUTF8=%v RawNUL=%v, want %v/%v", c.key, v.InvalidUTF8, v.RawNUL, c.wantInvalid, c.rawNUL)
|
||||
}
|
||||
if v.WorkspaceID != ws.ID {
|
||||
t.Errorf("%s: workspace %q, want %q", c.key, v.WorkspaceID, ws.ID)
|
||||
}
|
||||
}
|
||||
for k := range got {
|
||||
if strings.HasPrefix(k, control.ID) {
|
||||
t.Errorf("valid multibyte text reported as a violation: %s", k)
|
||||
}
|
||||
}
|
||||
if report.Total() != 3 {
|
||||
t.Errorf("violations = %d, want 3 (a row with both defects counts once): %v", report.Total(), report.Violations)
|
||||
}
|
||||
if !strings.Contains(got[bothRow.ID+"/title"].String(), "raw NUL + invalid UTF-8") {
|
||||
t.Errorf("both kinds not named: %s", got[bothRow.ID+"/title"])
|
||||
}
|
||||
|
||||
rep, err := s.RepairNUL()
|
||||
if err != nil {
|
||||
t.Fatalf("repair: %v", err)
|
||||
}
|
||||
if len(rep.Failed) != 0 || len(rep.Repaired) != 3 {
|
||||
t.Fatalf("repaired %d, failed %v; want 3 and none", len(rep.Repaired), rep.Failed)
|
||||
}
|
||||
read := func(col, id string) string {
|
||||
var v string
|
||||
if err := s.db.QueryRow(`SELECT `+col+` FROM items WHERE id = ?`, id).Scan(&v); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return v
|
||||
}
|
||||
if v := read("title", textRow.ID); v != "ok"+textguard.Replacement+"x" {
|
||||
t.Errorf("text row repaired to %q", v)
|
||||
}
|
||||
if v := read("title", bothRow.ID); v != "a"+textguard.Replacement+"b"+textguard.Replacement {
|
||||
t.Errorf("both row repaired to %q", v)
|
||||
}
|
||||
if v := read("title", control.ID); v != multibyte {
|
||||
t.Errorf("the control was changed: %q", v)
|
||||
}
|
||||
after, err := s.ScanNUL()
|
||||
if err != nil {
|
||||
t.Fatalf("rescan: %v", err)
|
||||
}
|
||||
if after.Total() != 0 {
|
||||
t.Errorf("violations after repair: %v", after.Violations)
|
||||
}
|
||||
}
|
||||
|
||||
// In a VALID JSON document an invalid byte can only sit inside a string, so
|
||||
// replacing it leaves a valid document with the same shape.
|
||||
func TestRepairInvalidUTF8KeepsJSONValid(t *testing.T) {
|
||||
t.Parallel()
|
||||
s := testStore(t)
|
||||
if s.dialect.Driver() != DriverSQLite {
|
||||
t.Skip("SQLite only")
|
||||
}
|
||||
ws := createTestWorkspace(t, s, "UTF8JSON")
|
||||
col := createTestCollection(t, s, ws.ID, "Tasks")
|
||||
it := createTestItem(t, s, ws.ID, col.ID, "json", "body")
|
||||
doc := `{"note":"x` + invalidByte + `y","n":1e3,"arr":["a` + invalidByte + `"]}`
|
||||
plantLegacyRows(t, s, func(raw *sql.DB) {
|
||||
mustExec(t, raw, `UPDATE items SET fields = ? WHERE id = ?`, doc, it.ID)
|
||||
})
|
||||
if _, err := s.RepairNUL(); err != nil {
|
||||
t.Fatalf("repair: %v", err)
|
||||
}
|
||||
var got string
|
||||
if err := s.db.QueryRow(`SELECT fields FROM items WHERE id = ?`, it.ID).Scan(&got); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
want := `{"note":"x` + textguard.Replacement + `y","n":1e3,"arr":["a` + textguard.Replacement + `"]}`
|
||||
if got != want || !json.Valid([]byte(got)) || !utf8.ValidString(got) {
|
||||
t.Errorf("repaired to %q (valid JSON %v), want %q", got, json.Valid([]byte(got)), want)
|
||||
}
|
||||
}
|
||||
|
||||
// A value whose row has an INCOMPLETE key (a NULL in its primary key, which
|
||||
// SQLite permits) and carries both defects is still ONE violation (codex r1).
|
||||
// Two such rows stay two: the merge is by rowid, which both scans read, not by
|
||||
// the partial key they share.
|
||||
func TestInvalidUTF8MergesOnKeyIncompleteRows(t *testing.T) {
|
||||
t.Parallel()
|
||||
s := testStore(t)
|
||||
if s.dialect.Driver() != DriverSQLite {
|
||||
t.Skip("SQLite only")
|
||||
}
|
||||
value := `{"note":"a` + textguard.NUL + `b` + invalidByte + `"}`
|
||||
plantLegacyRows(t, s, func(raw *sql.DB) {
|
||||
for i := 0; i < 2; i++ {
|
||||
mustExec(t, raw, `INSERT INTO activities (id, action, actor, source, metadata, created_at)
|
||||
VALUES (NULL, 'created', 'agent', 'cli', ?, datetime('now'))`, value)
|
||||
}
|
||||
})
|
||||
report, err := s.ScanNUL()
|
||||
if err != nil {
|
||||
t.Fatalf("scan: %v", err)
|
||||
}
|
||||
var got []NULViolation
|
||||
for _, v := range report.Violations {
|
||||
if v.Table == "activities" && v.Column == "metadata" {
|
||||
got = append(got, v)
|
||||
}
|
||||
}
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("want 2 violations (two rows, each counted once), got %d: %v", len(got), got)
|
||||
}
|
||||
for _, v := range got {
|
||||
if !v.KeyIncomplete || !v.RawNUL || !v.InvalidUTF8 {
|
||||
t.Errorf("want one incomplete-key violation carrying both kinds, got %+v", v)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The rowid merge (and the rowid address item_wiki_links already uses) reads
|
||||
// SQLite's hidden rowid. A table that DECLARED a column named rowid, _rowid_
|
||||
// or oid would shadow it, and the scan would read that column instead (codex
|
||||
// r2). None does; this makes adding one fail here rather than miscount.
|
||||
func TestNoProtectedTableShadowsRowid(t *testing.T) {
|
||||
t.Parallel()
|
||||
s := testStore(t)
|
||||
if s.dialect.Driver() != DriverSQLite {
|
||||
t.Skip("SQLite only")
|
||||
}
|
||||
seen := map[string]bool{}
|
||||
for _, c := range NULProtectedColumns() {
|
||||
if seen[c.Table] {
|
||||
continue
|
||||
}
|
||||
seen[c.Table] = true
|
||||
rows, err := s.db.Query(`SELECT name FROM pragma_table_info(?)`, c.Table)
|
||||
if err != nil {
|
||||
t.Fatalf("table_info %s: %v", c.Table, err)
|
||||
}
|
||||
n := 0
|
||||
for rows.Next() {
|
||||
var name string
|
||||
if err := rows.Scan(&name); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
n++
|
||||
switch strings.ToLower(name) {
|
||||
case "rowid", "_rowid_", "oid":
|
||||
t.Errorf("%s declares a column %q, which shadows SQLite's rowid the NUL scan merges on", c.Table, name)
|
||||
}
|
||||
}
|
||||
rows.Close()
|
||||
if n == 0 {
|
||||
t.Errorf("%s: no columns read; the check would pass vacuously", c.Table)
|
||||
}
|
||||
}
|
||||
if len(seen) == 0 {
|
||||
t.Fatal("no protected tables")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user