Files
rdb/doc/review.md
T

7.7 KiB
Raw Blame History

Code-Review: Schwachstellen, Fehler, Verbesserungspotential

Stand: 2026-10-30, Version 0.7.0. Die 114 Tests und der Handbuch-Test liefen grün. „Bestätigt“ heißt: nachgestellt. Alles andere stammt aus dem Lesen des Codes.

Schwachstellen (alle vier behoben, siehe CHANGELOG)

  1. Befehlseinschleusung über Tabellendaten in report (bestätigt). lib/rdb/cmd/report.rb:11-12: Bei <!cmd <Spalte>!> werden Zellwerte ungefiltert in einen Backtick-Aufruf eingesetzt. Eine Zelle mit touch PWNED hat die Datei angelegt. Abhilfe: Shellwords.escape, oder in der Doku als „nur vertrauenswürdige Daten“ kennzeichnen. Behoben: Werte werden mit Shellwords.escape eingesetzt.
  2. screen erzeugt Shell-Code aus Spaltennamen (report.rb:50-70). Namen stehen unquotiert in read NAME und echo "$NAME"; Leerzeichen, ; oder $(…) brechen das Skript oder führen Code aus. Werte werden ebenfalls nicht maskiert. Behoben: Spaltennamen müssen gültige Shell-Variablennamen sein.
  3. Lock-Dateien in /tmp (access.rb:121). Vorhersagbare Namen, andere Benutzer können sie anlegen oder löschen. lock liest und hängt an, ohne flock (Race). Keine Prüfung, ob die PID noch lebt; nach einem Absturz bleibt das Schloss. Behoben: privates Verzeichnis /tmp/rdb-locks-<UID> (0700) und flock. Offen: verwaiste Schlösser nach Absturz (eine PID-Prüfung bricht Tests und Handbuch, die erfundene PIDs nutzen).
  4. vilock nicht atomar (access.rb:134). Race zwischen File.exist? und rename; nach kill -9 bleibt LOCK… liegen. Behoben: atomare Sperre über File.link. Offen: nach kill -9 bleibt LOCK… liegen.

Fehler (1–8 behoben, 9 teilweise)

  1. Umlaute in Spaltennamen (bestätigt). to_awk erkennt nur [A-Za-z_#] und ASCII-\w. row 'Über > 3' scheitert mit awk-Fehler. Relevant für die DE/AT/CH-Tarife.
  2. Nicht-UTF-8-Daten (bestätigt). Latin-1-Datei führt zu invalid byte sequence in UTF-8 mit Stacktrace (table.rb:28). Abhilfe: binär lesen und scrub/ force_encoding, oder Kodierungsoption.
  3. Kein Fehlerfang auf oberster Ebene (bestätigt). Stacktraces statt Meldungen bei ungültiger Regex (update.rdb mit /[, access.rb:277; sale, act.rb:345), Date::Error in to_julian, Errno::*. Abhilfe: rescue StandardError in bin/rdb/Rdb.run.
  4. EDITOR mit Argumenten (bestätigt, behoben). system(ENV['EDITOR'], datei) in vilock und edit_cells (access.rb:138, access.rb:150) behandelt "nvim -f" als einen Programmnamen. Der Editor startet nie, update.rdb meldet trotzdem „record replaced“. Abhilfe: Shellwords.split, Rückgabewert prüfen.
  5. Datenverlust-Risiko bei update.rdb e (access.rb:289). Der Satz wird vor dem Editieren geleert; bei Abbruch ist er weg. Erst nach erfolgreichem Editieren schreiben.
  6. Tab/Zeilenumbruch in Werten ungeprüft (bestätigt für Tab). append, enter, replace, ve schreiben Zellen unverändert; ein Tab erzeugte eine Zeile mit 3 statt 2 Spalten (check.rdb meldet es).
  7. Nicht atomares Schreiben. splice, build, substitute überschreiben direkt, ohne temporäre Datei plus rename, ohne flock.
  8. Inkonsistente Vergleiche bei -mb/-mi (nur im Code gesehen). Index sortiert mit to_s, Suche vergleicht mit strip; führende Leerzeichen und -x ohne Teilsuche können falsch ausfallen.
  9. Kleinigkeiten. sorttable -r kehrt gleiche Schlüssel um (reverse! nach stabiler Sortierung); -nrf gilt global für alle Spalten; jointable -n wird ignoriert; total/mean lehnen 1e3 ab; eine Zeile aus lauter leeren Zellen gilt als gelöscht (fast.rb:21).

Stand der Behebung: 1 (Umlaute: Unicode-fähiger Umsetzer), 2 (Latin-1 wird als UTF-8 gelesen), 3 (Fehler als eine Zeile, RDB_DEBUG=1 für den Backtrace), 4 (EDITOR mit Argumenten, Editorfehler wird gemeldet), 5 (Satz wird in jedem Fall zurückgeschrieben), 6 (Rdb.check_cells!), 7 (Rdb.write_file: temporäre Datei plus rename) und 8 (Sortierschlüssel ohne Leerzeichen) sind behoben, jeweils mit Test.

Bei 9 sind sorttable -r (stabil) und jointable -n (Schlüssel als Zahlen, wie in der Man-Page) behoben. Bleibt so (Entscheidung 2026-10-30):

  • -nrf gilt für alle Spalten: laut Man-Page reicht sorttable die Optionen an sort weiter, Optionen pro Spalte gibt es im Original nicht.
  • 1e3 ist keine Zahl: sonst würden Spalten mit Codes wie 2E5 numerisch.
  • Eine Zeile aus lauter leeren Zellen gilt als gelöscht: so arbeitet delete im Format.

Offen bei 8: index -x (gefaltet) hat in search ohne -x kein Gegenstück, weil die .i-Datei die Faltung nicht vermerkt. Das bräuchte eine Markierung im Indexkopf.

Verbesserungspotential

  1. Indexe sind nicht schnell. — Behoben (Seek statt Volllesen, lazy Data). Ursprünglich: Jede Suche liest Tabelle und Indexdatei komplett (fast.rb:151, fast.rb:171, Data.new pro Schlüssel in delete/update.rdb): O(n) pro Schlüssel. Abhilfe: mit seek/pread direkt an der Fixbreiten-Position lesen.
  2. Schlechte Hash-Funktion — Behoben (FNV-1a, größter Eimer 4). Ursprünglich: (fast.rb:68): bytes.sum % slots. Bestätigt: 3000 fünfstellige Nummern landen in 30 Slots, größter Bucket 223. Abhilfe: FNV-1a o. Ä., Primzahl-Slotzahl.
  3. Geld in Float — Behoben (Rational in total/mean/subtotal und /act). Ursprünglich: (act.rb:11, to_f im /act-Teil und in total). Abhilfe: BigDecimal oder ganzzahlige Rappen.
  4. awk-Brücke — bleibt so (Entscheidung 2026-10-30). awk und awk_text teilen sich jetzt den Aufruf. Streaming und ein eigener Ausdrucksparser wären ein Umbau mit Verhaltensrisiko (awk-Syntax ist dokumentierte Schnittstelle). Ursprünglich: (env.rb:119-138): ganze Tabelle als String durch capture3, kein Streaming; handgeschriebener Parser erkennt Strings und Regexe heuristisch (prev) und hängt vom awk-Dialekt ab.
  5. Globale Methoden auf Object — Behoben (Rdb::Helpers, Konstanten in Rdb). Ursprünglich: (fast_opts, lock_file, num, money, ask, need, show_record …). In ein Modul kapseln und in Env einbinden.
  6. Doppelter Code — Behoben (Table.head_text, dash_cells, headed?; awk über awk_text). Ursprünglich: awk/awk_text; Schlüsseltabellen-Parsing in fast_opts/ key_columns; von Hand gebaute Kopf-/Strichzeilen trotz Table.dash_row.
  7. Tests — Behoben (Suite 70 s → 21 s). Parallel, und bin/rdb startet mit --disable-gems (ca. 85 statt 170 ms je Befehl). Fehlerfälle haben jetzt Tests. Den Rest der Zeit braucht test_testall_runs_demo_script (rund 300 Befehle nacheinander). Ursprünglich: Laufzeit ca. 66 s wegen eines Ruby-Prozesses pro Test. Fehlerfälle (Encoding, Tab in Wert, Locks, Editor) nicht abgedeckt; Tests direkt gegen Rdb.run im Prozess wären schneller.
  8. Repo — bleibt so (Entscheidung 2026-10-30). Ursprünglich: demo/fact.pl, fact_lc.pl, rule.pl sind ungetrackt; demo/testall.old (1187 Zeilen) wäre als Golden-File-Test besser gepflegt.

Empfohlene Reihenfolge

Schwachstellen, Fehler und Verbesserungen 1–3 und 5–7 sind erledigt. Die awk-Brücke (4) und das Repo (8) bleiben auf Entscheidung des Autors so. Das Review ist damit abgeschlossen.