Closed MHajoha closed 2 weeks ago
Ich weiß nicht, warum GitHub hier Konflikte sieht. Ein lokaler fast-forward (git merge --ff-only origin/question-ui-html
) läuft ohne Probleme durch. Nach Review würde ich dann auch so manuell mergen.
Ich frage mich generell, ob es gut ist, dass der Wert des Placeholders geparsed wird. Einerseits könnte das zu Sicherheitslücken führen (ist im SDK vielleicht egal), andererseits wird dadurch unter Umständen auf unerwartete Weise an dem HTML rumgepfuscht (werden zum Beispiel nicht geschlossene Tags automatisch geschlossen?). Das kann man natürlich auch als Feature betrachten. Hast du das bedacht?
Ich hatte mich letzten Freitag mit #86 und den Auswirkungen auf die Platzhalter beschäftigt und kam dabei auch zu dem Urteil. Wollte das heute mit euch besprechen.
Der HTML-Parser oder die tostring
Funktion im HTML-Modus verhalten schon etwas anders. Beispielsweise war mir aufgefallen, dass der plain text im CDATA (was vermutlich eh nicht die richtige Lösung war) zu normalen HTML wird (CDATA wird entfernt und die Tags haben wieder ihre normale Bedeutung -> XSS-Gefahr).
Wie gerade im Meeting besprochen: Ich merge das so, und grundlegendere Placeholder-Diskussionen stellen wir erstmal zurück, auch wenn wir Änderungsbedarf insb. mit Hinblick auf Security sehen.
Integriert @tx0c3's fix #86 (Vielen Dank dafür, sorry dass das so lange gedauert hat.), passt die Tests darauf an und korrigiert das sehr komische Verhalten, dass
_resolve_placeholders
die Placeholder-Werte in<string>
- Tags wickelt, die es so in HTML nicht gibt. Ich vermute letzteres ist dem crappigen Design von LXML geschuldet, aber diese Lösung dürfte korrekter sein.