[2009-07-18] Povinné volání parent::startup() v presenteru
- David Grudl
- Nette Core | 8227
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()zavolatparent::startup(). Na chybějící volání vás framework upozorní chybouE_USER_WARNINGdoplněnou (jako obvykle) co nejvýstižnější hláškou. - Co když schválně nechci volat předka? Tak místo
parent::startup()zavolejtePresenter::startup().
- pekelnik
- Člen | 462
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)
- Panda
- Člen | 569
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)
- jasir
- Člen | 746
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í.
- pekelnik
- Člen | 462
…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)
- PetrP
- Člen | 587
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.
- Majkl578
- Moderator | 1364
mcl napsal(a):
- Co se mění? Je potřeba povinně v každé metodě
startup()zavolatparent::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) ;)
