[2009-07-18] Povinné volání parent::startup() v presenteru

před 9 lety

David Grudl
Nette Core | 6770
+
0
-

Na fóru proběhla diskuse, která poukázala na možné bezpečnostní rizika toho, že zapomeneme v překryté metodě zavolat jejího předchůdce.

Tohle je samozřejmě obecný problém. Jazyk nevyžaduje a ani nekontroluje, jestli metodu předchůdce zavoláte, ani u konstruktorů, což může vést (a také vede) k řadě problémů. Hloupá chyba, která se těžko dohledává.

Právě u metody presenteru startup() mám za to, že mechanismus, který by zkontroloval, že přechůdce byl zavolán, by byl nesmírně užitečný. Proto jsem ho doplnil. Je pravda, že to bude nejspíš vyžadovat zásah do kódu, ovšem zcela triviální, ale může vám v budoucnu zachránit skutečně hodně.

  • Co se mění? Je potřeba povinně v každé metodě startup() zavolat parent::startup(). Na chybějící volání vás framework upozorní chybou E_USER_WARNING doplněnou (jako obvykle) co nejvýstižnější hláškou.
  • Co když schválně nechci volat předka? Tak místo parent::startup() zavolejte Presenter::startup().

před 9 lety

pekelnik
Člen | 468
+
0
-

Co když nechci vůbec volat parent::startup() ani Presenter::starup(). Přijde mi, že se zde framework míchá do něčeho do čeho mu nic není (dělání chůvy programátorovi).

Nutit všechny aby volali metodu parent::startup() jen proto, že by na to někdy mohl někdo zapomenout mi přijde takové ehm… divné…

Navíc pokud na to někdo zapomene, pravděpodobně si toho všimne dřív než by se nadál…

Editoval pekelnik (28. 7. 2009 15:25)

před 9 lety

Panda
Člen | 570
+
0
-

pekelnik napsal(a):

Co když nechci vůbec volat parent::startup() ani Presenter::starup(). Přijde mi, že se zde framework míchá do něčeho do čeho mu nic není (dělání chůvy programátorovi).

Nutit všechny aby volali metodu parent::startup() jen proto, že by na to někdy mohl někdo zapomenout mi přijde takové ehm… divné…

Navíc pokud na to někdo zapomene, pravděpodobně si toho všimne dřív než by se nadál…

Důvod vzniku byl právě ten, že si toho člověk často ani nevšiml a chyba se obtížně hledala, což mohlo vyústit v nepříjemná bezpečnostní rizika. Já osobně považuji jeden povinný řádek v metodě startup() za přijatelný ve srovnání s tím, jaká rizika přináší jeho nechtěné vynechání.

// Doplnění: a pokud se to někomu nelíbí, nic mu nebrání si na vlastní riziko v BasePresenteru překrýt metodu run následujícím způsobem:

<?php
    public function run()
    {
        Presenter::startup();
        parent::run();
    }
?>

Editoval Panda (28. 7. 2009 16:21)

před 9 lety

sodae
Nette Evangelist | 251
+
0
-

Davide, měl bych nápad, tohle by se spíše hodilo u parent::__contruct() u komponenty, předešlo by se problémům

před 9 lety

jasir
Člen | 748
+
0
-

pekelnik napsal(a):

Co když nechci vůbec volat parent::startup() ani Presenter::starup(). Přijde mi, že se zde framework míchá do něčeho do čeho mu nic není (dělání chůvy programátorovi).

Co když nechci vůbec používat Presenter? ;-)

Okej, dříve to frameworku nebylo a taky to fungovalo, ale praxí se ukázalo, že je to užitečné. Zapomenutí volání parent::startup() si většinou opravdu nevšimneš a zjistíš to třeba až když už je to vše na produkci a někdo se začne hrabat v administraci webu, protože jsi zapomněl to volání.

před 9 lety

pekelnik
Člen | 468
+
0
-

…volání parent::startup() si většinou opravdu nevšimneš a zjistíš to třeba až když už je to vše na
produkci a někdo se začne hrabat v administraci webu…

Uff!

To u toho konstruktoru komponenty mi to přijde smysluplnější. V podstatě mi to ale nevadí ani v tom presenteru :)

Editoval pekelnik (29. 7. 2009 12:27)

před 9 lety

PetrP
Člen | 587
+
0
-

sodae napsal(a):

Davide, měl bych nápad, tohle by se spíše hodilo u parent::__contruct() u komponenty, předešlo by se problémům

U Presenter::startup se často ověřuje přihlášení, opravnění apod. takže tam to smysl má, u component si bud jednoduchou detekci napis sám (ale u constructoru to nebude uplně triviální) nebo si v najekem BaseControl pridej podporu pro startup, a pripadne si pridej kontrolu po vzoru presenteru.

před 9 lety

mcl
Člen | 2
+
0
-
  • Co se mění? Je potřeba povinně v každé metodě startup() zavolat parent::startup().

Zdravím, bylo by dobré toto upravit ve Quickstartu, aby se začátečníci na této chybě nezasekli, jako před chvílí já :-)

před 9 lety

Majkl578
Moderator | 1379
+
0
-

mcl napsal(a):

  • Co se mění? Je potřeba povinně v každé metodě startup() zavolat parent::startup().

Zdravím, bylo by dobré toto upravit ve Quickstartu, aby se začátečníci na této chybě nezasekli, jako před chvílí já :-)

Doplnil jsem to (mohl jsi to ale udělat i ty – v pravo nahoře nad článkem Wiki > Edituj) ;)

před 9 lety

Inza
Člen | 330
+
0
-

peklíčko;-)… Musíme s tím NDP hodně rychle pohnout páč jinak se nám začátečníci budou zasekávat na mnohem víc věcech než na tomhletom…

před 9 lety

Majkl578
Moderator | 1379
+
0
-

Inza napsal(a):

peklíčko;-)… Musíme s tím NDP hodně rychle pohnout páč jinak se nám začátečníci budou zasekávat na mnohem víc věcech než na tomhletom…

kdyby byl zájem, mohl bych do toho vosího hnízda taky píchnout :)