diskreter rex_sql-Code vs. SQL-Datei

[christophboecker]

In der install.php werden zur Installation der URL-Profile zwei SQL-Dateien ausgeführt. Soweit so gut. RexStan meckert aus seiner Sicht nicht ganz zu unrecht an:

Possible SQL-injection in expression str_replace('999999', (string) \rex_article::getSiteStartArticleId(), \rex_file::get(__DIR__ . '/install/rex_url_profile_neues_category.sql')).

Der Code dazu ist

$query = str_replace('999999', (string) rex_article::getSiteStartArticleId(), rex_file::get(__DIR__ . '/install/rex_url_profile_neues_category.sql'));
rex_sql::factory()->setQuery($query);

Schon aus Gründen der Übersicht (alles an einem Ort) und der Prüfbarkeit mit RexStan wäre mein favorisiertes Vorgehen ein Insert mit dem SQL-Object; wir reden hier ja nur über einen Datensatz.

$sql->setTable(rex::getTable('url_generator_profile'));
$sql->setValue('namespace','neues-category-id'); 
$sql->setValue('article_id',rex_article::getSiteStartArticleId()); 
$sql->setValue('clang_id',1); 
$sql->setValue('ep_pre_save_called',0); 
$sql->setValue('table_name','1_xxx_rex_neues_category'); 
$sql->setValue('table_parameters','{............}'); 
$sql->setValue('relation_1_table_name',''); 
$sql->setValue('relation_1_table_parameters','[]'); 
$sql->setValue('relation_2_table_name',''); 
$sql->setValue('relation_2_table_parameters','[]'); 
$sql->setValue('relation_3_table_name',''); 
$sql->setValue('relation_3_table_parameters','[]');
$sql->addGlobalCreateFields('neues');
$sql->insert();

Was ist Deine Meinung?

[alxndr-w]

Die table_parameters machen das so unübersichtlich, deswegen hätte ich die gerne nicht direkt in der install.php

Das war auch ein Grund für den Schnellschuss – selbst wenn ich mir nicht vorstellen kann, wie das praktisch zu einer SQL-injection führen kann.

In jedem Fall bin ich damit einverstanden, das wie vorgeschlagen zu lösen. Gerne jedoch in einer separaten Datei via include_once für die Übersichtlichkeit.

[christophboecker]

Jo, das klappt gut:

sql->setTable(rex::getTable('url_generator_profile'));
$sql->setValue('namespace','neues-category-id'); 
$sql->setValue('article_id',rex_article::getSiteStartArticleId()); 
$sql->setValue('clang_id',1); 
$sql->setValue('ep_pre_save_called',0); 
$sql->setValue('table_name','1_xxx_rex_neues_category'); 
$sql->setValue('table_parameters',json_encode([
    'column_id' => 'id',
    'column_clang_id' => '',
    'restriction_1_column' => 'status',
    'restriction_1_comparison_operator' => '>',
    'restriction_1_value' => '0',
    'restriction_2_logical_operator' => '',
    'restriction_2_column' => '',
    'restriction_2_comparison_operator' => '=',
    'restriction_2_value' => '',
    'restriction_3_logical_operator' => '',
    'restriction_3_column' => '',
    'restriction_3_comparison_operator' => '=',
    'restriction_3_value' => '',
    'column_segment_part_1' => 'name',
    'column_segment_part_2_separator' => '/',
    'column_segment_part_2' => '',
    'column_segment_part_3_separator' => '/',
    'column_segment_part_3' => '',
    'relation_1_column' => '',
    'relation_1_position' => 'BEFORE',
    'relation_2_column' => '',
    'relation_2_position' => 'BEFORE',
    'relation_3_column' => '',
    'relation_3_position' => 'BEFORE',
    'append_user_paths' => '',
    'append_structure_categories' => '0',
    'column_seo_title' => 'name',
    'column_seo_description' => '',
    'column_seo_image' => '',
    'sitemap_add' => '1',
    'sitemap_frequency' => 'weekly',
    'sitemap_priority' => '0.5',
    'column_sitemap_lastmod' => ''
]));
$sql->setValue('relation_1_table_name',''); 
$sql->setValue('relation_1_table_parameters','[]'); 
$sql->setValue('relation_2_table_name',''); 
$sql->setValue('relation_2_table_parameters','[]'); 
$sql->setValue('relation_3_table_name',''); 
$sql->setValue('relation_3_table_parameters','[]');
$sql->addGlobalCreateFields('neues');
$sql->insert();

Zusatzfrage: warum erstmal das Flag abfragen, ob die Installation der URL-Profile schon mal gelaufen ist? Da das Flag dann gesetzt ist, kann bei einer Re-Installation ein warum ach immer fehlendes Profil nicht mehr ergänzt/repariert werden. Ich würde das Flag weglassen.

[alxndr-w]

Zusatzfrage: warum erstmal das Flag abfragen, ob die Installation der URL-Profile schon mal gelaufen ist? Da das Flag dann gesetzt ist, kann bei einer Re-Installation ein warum ach immer fehlendes Profil nicht mehr ergänzt/repariert werden. Ich würde das Flag weglassen.

Die Verwendung eines URL-Profils ist optional.

Das Flag wird nur gesetzt, wenn wenigstens 1x die URL-Profile erfolgreich installiert wurden. Als Entwickler kann ich jedoch selbst entscheiden, ob und wie ein URL-Profil aufgebaut ist.

Ich möchte vermeiden, dass bspw. ein anderer URL-Key verwendet wird und dann wieder ohne es zu merken mit einem Update erneut ein Profil installiert wird, obwohl das Standard-Profil bewusst nicht existiert.

[alxndr-w]

Zum Code selbst: Du hast mich überzeugt.

'column_sitemap_lastmod' => ''
'column_seo_description' => '',
'column_seo_image' => '',

Tatsächlich müsste das mitgelieferte Profil noch aktualisiert werden, wie man jetzt gut sehen kann.

Es gibt längst Felder wie teaser / Kurztext als SEO-Description, image / Poster, updatedate.

[christophboecker]

Es gibt zwei Situationen, die meiner Erwartungshaltung nach zum selben Ergenis führen sollten, es hier aber nicht tun :

Nach einer Deinstallation ist das Flag weg (Config-Namespace gelöscht). Aber alle Einträge in Tabellen in den URL-Tabellen bestehen fort. Re-Installieren hätte also eine andere Auswirkung (url-Profile nicht noch einmal versuchen zu installieren) als De-Installieren + Installieren (Url-Profile einspielen bzw. ergänzen).

Kann man aber auch als Feature sehen :-)

[alxndr-w]

Gute Frage. Meist arbeiten Add-ons in REDAXO ja unabhängig voneinander und nicht integriert.

Ich wünsche mir schon länger einen Installations-Dialog und einen Deinstallieren-Dialog, bei dem man Optionen setzen kann.

Das wär für mich die beste UX weil ich über die einzelnen Prozesse

1. In Kenntnis gesetzt werde und
2. Diese Steuern kann.

Das würde die Frage zurück an den Entwickler werfen, statt ihn zu bevormunden.

[christophboecker]

Meinst Du sowas?

[christophboecker]

Ist nicht meine Idee; der Trick kommt von @skerbis. Abbrechen mit Fehlermeldung und Links anbieten, die die Url einfach noch mal anbieten, aber mit einen Action-Code je nach Wahl.

[alxndr-w]

Das ist spitze. @skerbis hast du das sonstwo auch im Einsatz oder gerade erst darauf gekommen?

[skerbis]

Habe es mal beim Wechsel sked zu for_cal gemacht. @christophboecker denkt es hier aber weiter 👍