Validace parametrů v presenterech

Upozornění: Tohle vlákno je hodně staré a informace nemusí být platné pro současné Nette.
Jan Tvrdík
Nette guru | 2570
+
0
-

Validace parametrů v presenterech

SOUČASNÝ STAV
Význam nepovinných argumentů
  • PHP, přestože je to stupidní jazyk, umí rozlišovat povinné a volitelné argumenty u metod a funkcí. Rozlišovací faktorem je skutečnost, zda má daný argument výchozí hodnotu.
  • Nette, přestože je to šikovný framework, nerozlišuje v presenterech povinné a volitelné argumenty. Pokud PresenterRequest neobsahuje hodnotu pro volitelný parametr, použije se jeho výchozí hodnota, pokud PresenterRequest neobsahuje hodnotu pro povinný parametr, dosadí se NULL, místo vyhození BadRequestException!
Význam výchozích hodnot
  • Pokud existuje výchozí hodnota, pak je hodnota parametru automaticky přetypována na typ výchozí hodnoty.
  • Platí i pro persistentní parametry.
PŘÍKLAD VALIDACE V SOUČASNOSTI
	public function renderDefault($id)
	{
		$page = dibi::fetch('SELECT * FROM [pages] WHERE [pageId] = %i', $id);
		if ($page === FALSE) throw new BadRequestException('Page not found!');
		$this->template->page = $page;
	}

Uvedený skript vypadá na první pohled bezpečně. Použití dibi a číselného modifikátoru %i by mělo automaticky zajistit bezpečné zpracování vstupního parametru $id. Nebo je to jinak? Abychom na tuto otázku dokázali odpovědět, je nutné si uvědomit, jakých typů může nabýt parametr $id. Základní typy, které napadnou asi každého jsou NULL a string (tak se předávají i celá čísla). Člověk s mírně lepšími znalostmi ale ví, že v URL mohou být přenášeny i pole. A to je zde právě kamenem úrazu. Stačí zavolat skript s parametry ?id[0]=1&id[1]=2 a pád skriptu je zaručen. Kromě toho se zde vyskytuje ještě další problém a tím je duplicita obsahu. Stejný obsah jako při parametru ?id=123 dostaneme díky přetypování s parametrem ?id=123XYZ.

Odborníci v Nette by ale měli vidět i to, co běžný uživatel nevidí, a to skutečnost, že parametr $id může nabývat naprosto libovolného typu (tj. třeba DateTime), protože se nejedná o parametry HttpRequestu, ale o parametry PresenterRequestu, který si sestavuje router sám.

Pokud tohle všechno chápeme, zjistíme, že validovat obecně celošíselný parametr není úplně triviální, nelze přímo použít ani is_int, ani !is_array, ani !is_numeric, ani !ctype_digit.

Zajímavé řešení poskytuje magická vlastnost Nette spočívající v automatickém přetypování na typ výchozího parametru. I toto řešení však má své úskalí:

  1. Je hodně magické (kdo neví, toho to nenapadne)
  2. Nevytváří sice duplicity, ale akceptuje i nesmyslná id je tvaru např. ?id=123ABC.
  3. Pokud je hodnota parametru shodná s výchozí hodnotou, není na první pohled zřejmé, zda má parametr skutečně takovou hodnotu, nebo neexistuje vůbec a výchozí hodnotu dosadilo Nette.

Toto všechno mě vedlo k hledáním nového řešení, které dnes předkládám k posouzení.

Představte si, že vše, co musíte udělat, pro dosažení naprosto bezpečné a spolehlivé validace je použit anotaci.

	/** @param int */
	public function renderDefault($id)
	{
		$page = dibi::fetch('SELECT * FROM [pages] WHERE [pageId] = %i', $id);
		if ($page === FALSE) throw new BadRequestException('Page not found!');
		$this->template->page = $page;
	}

Vlastnosti

  1. Pokud $id není celé číslo, dojde k vyhození BadRequestException.
  2. Automaticky přetypuje řetězec obsahující celé číslo na int.

Problémy

  1. Stále magické, i když výrazně méně, než předchozí řešení
  2. Zpětná kompatibilita
  3. Větší riziko překlepů
Podrobnosti
  • Validace parametrů se zapíná pomocí Presenter::$autoValidateParams, výchozí je FALSE.
  • Validaci lze vypnout / zapnout i na úrovni metody pomocí anotace @ValidateParams [true|false]
  • Akceptované zápisy:
    • @param int
    • @param int|float|FALSE
    • @param array $c optional comment
    • @param array|NULL
    • @param mixed – vypne validaci
  • Omezení implementace:
    • Úmyslně nefunkční zápisy
      • @param true
      • @param false
      • @param null
    • Nelze validovat typ instance objektu (@param DateTime)
    • Zatím nefunguje validace perzistentních parametrů

Funkční konverze

  • "123" => int
  • "123.4" => float
  • "1" => bool

Sporné konverze

  • "123" => float
  • int 123 => float
  • int 123 => string
  • float 123.0 => int
  • float 123.4 => string

Magické konverze

  • Zatím neimplementovány
  • "2010-02-06" => DateTime

Kompatibilita

  • Bez BC breaku:
    • Nepoužíváte @param anotace
    • Máte automatickou validaci vypnutou
  • Potenciální BC break:
    • Skript nepočítá s automatickým přetypováním podle anotací
    • Skript počítá s automatickým přetypováním dle výchozí hodnoty
jtousek
Člen | 951
+
0
-

Je to moc hezké, přidám tedy nějaké vlastní nápady:

Parametry typu array

Pokud je parametr typu array, někdy by se hodilo specifikovat i typ prvků v poli. Např. nějaké array $ids by mělo zřejmě být pole integerů. Jsem si velmi dobře vědom, že PHPdoc na tohle nemá specifikaci, což znamená, že bychom museli nějakou vhodnou syntaxi zvolit. Nejvhodnější mi připadá tato syntaxe:

/** @param array<int> */

Druhou možností je zápis ala C#:

/** @param int[] */

Parametry typu objekt

Někdy se hodí parametry typu objekt. Základem je onen zmiňovaný DateTime, ale napadá mě mnohem lepší příklad. Zejména uživatelům Doctrine 2 by se mohlo hodit předávat jako parametr přímo entitu. Typicky když máte $id tak k čemu vám je? Přece k tomu, abyste z modelu vytáhli daný záznam, ale samotné $id jinak vůbec nepotřebujete. Nebylo by ideální použít to takhle?

/**
 * @param ArticleEntity
 */
public function actionEdit(ArticleEntity $entity) {
	//nemusím volat obvyklé $this->getEm()->getRepository('ArticleEntity')->find($id);
	//ani testovat zda záznam skutečně byl nalezen a vyhazovat výjimku
	//práce s formulářem
}

Pokud entita s daným ID neexistuje, vyhodí se BadRequestException a action se ani nespustí. Může se spolehnout, že uživatel si skutečně řekl o existující článek a že nechtěl neexistující článek –157 nebo 36724.

Jak to tedy řešit? Samozřejmě objekty nemůžeme přímo předávat jako parametr. Můžeme ale mít mechanismus, který objekt dané třídy bude převádět na primitivní typ ($entity → $id, použije se když $entity předáte v šabloně jako parametr {link}) a samozřejmě onen primitivní typ zpět na objekt ($id → $entity, použije se v Presenteru před voláním actionXyz).

DateTime by se tedy převáděl na string podobu ve tvaru „Y-m-d H:i:s“, zpětná konverze je snadná.

<cokoli>Entity z Doctrine by se převáděly na int ID, zpětná konverze znamená dotaz na model. Tento dotaz nyní znovu a znovu píšeme ve většině action, pokaždé s kontrolou, zda záznam skutečně existuje.

Editoval jtousek (28. 2. 2011 20:57)

Filip Procházka
Moderator | 4668
+
0
-

Automatická validace parametrů se mi líbí. +1

Parametry typu array – byl bych pro @param array of int, popř. @param array<int>, což jsem i viděl někde použito a mám pocit, že mi to i sežralo jednou netbeans, ale to mohl být klidně omyl :)

Automatické hledání entity je brutálně magické a rozhodně bych to nevyužil, protože mám raději, když si můžu ošetřit co přesně si vezmu z úložiště. Toto automatické hledání entity, by mi bránilo získat z databáze i některé další asociované entity v jednom dotazu a „musel“ bych použít lazy načítání, které je výchozí. Což mi nemusí vyhovovat. Navíc mi kompletně chybí jakákoliv Access Control vrstva. Takže tohle –1

Jan Tvrdík
Nette guru | 2570
+
0
-

Automatické hledání entity může být pro určité projekty užitečná vlastnost, ale obecně do Nette se mi to nelíbí. Jestli to ve svém projektu potřebuješ, tak si to implementuj do startup.

Editoval Jan Tvrdík (28. 2. 2011 21:31)

jtousek
Člen | 951
+
0
-

Jan Tvrdík: Nejde mi konkrétně o entity, ale o mapování libovolné třídy (např. i DateTime) na přimitivní typ a zpět. Nechci aby bylo přímo v Nette napsané, jak se získávají entity z DB, to je blbost. Chci jen obecné API. Jak se ten primitivní typ převede na objekt dané třídy (a naopak) už Nette nezajímá.

EDIT:

HosipLan: Moje Access Control vrstva je zcela záměrně napsaná tak aby tomuhle přístupu naprosto nevadila. V podstatě v action se nic nekontroluje, kontrola práv se provádí mnohem dříve a pokud práva nejsou tak BadRequestException. Samozřejmě že u složitějších případů to takhle nejde, ale zatím jsem na takový případ nenarazil.

Řeším to tak kvůli tomu abych na akci, na kterou chybí právo ani nezobrazoval odkaz. Když bych AC řešil až v action, tak bych to musel stejně řešit znova ve všech šablonách u každýho odkazu, což se mi fakt nechtělo.

Tvůj argument s lazy načítáním naprosto chápu a uznávám. Argument s Access Control ne.

Editoval jtousek (28. 2. 2011 21:55)

Ondřej Brejla
Člen | 746
+
0
-

Java-like zápis pole int[] $arr se mi líbí. A API pro mapování objektů, to zní taky moc zajímavě:-)

Jan Tvrdík
Nette guru | 2570
+
0
-

Jak často předáváte v URL pole?

Obávám se, že tvorba API pro pohodlné mapování objektů přesahuje toto RFC. Můžete si ale naštěstí napsat vlastní :) Základní problém u objektů je skutečnost, že nelze zjistit definované use. A psát všechno absolutně se mi hnusí.

Editoval Jan Tvrdík (28. 2. 2011 23:42)

jtousek
Člen | 951
+
0
-

Pokud programuji nějaké dávkové úpravy většího množství entit tak potřebuju předávat pole poměrně často. (Sice to pole tvoří javascript, ale to je jedno.)

OK, chápu. Právě mě napadly ještě další problémy s tím mapováním objektů. Konkrétně dědičnost – nebyla by možnost aby každý zděděný presenter používal jiné objekty. Už mě i napadlo řešení, ale ke zveřejnění to má ještě daleko.

hrach
Člen | 1819
+
0
-

pokud možno bych ty objekty přestal řešit, toto je jiný task a objekty by mohli zabranit implementaci teto nadhery :)

Jan Tvrdík
Nette guru | 2570
+
0
-

Má někdo nápad, jak elegantně pořešit validace persitentních parametrů?

jtousek
Člen | 951
+
0
-

Aha tys dal tu normální validaci do reflexe, takže to není omezené jen na komponenty / presentery. Zajímavé.

V případě perzistentních parametrů asi není jiná šance než PresenterComponent::loadState().

Dotaz: může mít komponenta (ne presenter) i neperzistentní parametry? Pokud ano, jak fungují? Ideálně příklad využití.

Patrik Votoček
Člen | 2221
+
0
-

jtousek napsal(a):

Dotaz: může mít komponenta (ne presenter) i neperzistentní parametry? Pokud ano, jak fungují? Ideálně příklad využití.

Proč by nemohla?

public function render($param)
{
	$this->template->param = $param; // sample
}
{var $param = 'foo'}
{control foo $param}
jtousek
Člen | 951
+
0
-

Patrik Votoček: Díky. :) A ta validace parametrů funguje i tady nebo jen v presenterech?

Jan Tvrdík
Nette guru | 2570
+
0
-

jtousek napsal(a):

Aha tys dal tu normální validaci do reflexe, takže to není omezené jen na komponenty / presentery. Zajímavé.

Tam to dal David, já to akorát nepřesunul jinam.

Může mít komponenta (ne presenter) i neperzistentní parametry? Pokud ano, jak fungují? Ideálně příklad využití.

Jedná se sice o off topic, ale rád bych ti v tom udělal jasno, už proto, že kolega Patrik se bohužel mýlí. Termín „parametry komponenty“ označuje (alespoň dle mého názoru) obsah proměnné PresenterComponent::$params. Pokud máš v presenteru komponentu nazvanou foo, tak parametr s názvem name a hodnotou abc bude v url reprezentován jako ?foo-name=abc. Perzistentní parametry komponenty jsou zvláštním druhem („podmnožinou“) parametrů komponenty, jedná se právě a pouze o takové parametry, které vrací metoda PresenterComponent::getPersistentParams.

A ta validace parametrů funguje i tady nebo jen v presenterech?

Validace bude fungovat i v handlerech signálů v komponentách.

David Grudl
Nette Core | 7464
+
0
-

Používání @param int je velmi logické a zažité. Má ovšem jeden a řekl bych fatální problém. Neexistuje dostatečně dobrá vazba mezi anotací a proměnnou, takže během života kódu se dřív nebo později stane, že přestanou být anotace a skutečné parametry zesynchronizované. Když bychom provedli analýzu kódu (klidně Nette), asi bychom nejvíc chyb v phpDoc oproti skutečnému stavu našli právě v @params. Zkrátka to není blbuvzdorné.

Z toho důvodu jsem kdysi dávno zvolil nestandardní „anotování“ pomocí výchozího typu.

Mám za to, že skutečný problém vězí v tom, že v místě, kde očekáváme skalár, dostaneme pole – to je skutečná bolíska mnoha aplikací v PHP. Rozlišování mezi int, string, float a bool už v PHP nehraje zásadní roli. Jde prostě o to, že chci buď skalár/NULL nebo pole/NULL.

Což je ovšem stav, který se dá velmi dobře popsat pomocí standardních typehintů.

Jsem si vědom problému, který tento RFC řeší, ale navrhl bych, alespoň prozatím, tuto jednodušší úpravu: Nette se postará, aby všechny předávané typy byly skalár/NULL (při zachování současného přetypovávání podle výchozího typu) a pole/objekt bude používat jen v případě, že bude výslovně uvedeno jako typehint nebo výchozí hodnota.

Parametry ve tvaru ?id=123ABC může buď rovnou odmítat, nebo, jako dosud, řešit kanoni… ehm… no to slovo.

Jan Tvrdík
Nette guru | 2570
+
0
-

Díky za reakci. Nečekal jsem, že se jí někdy dočkám :)

Souhlasím, že hlavním problémem je pole / objekt v situaci, kdy očekávám skalár. To, co navrhuješ, by mělo ve většině případů zabránit pádu aplikace. Jedná se tedy určitě o krok dopředu.


Vrátím se zase k výše uvedenému ukázkovému příkladu:

public function renderDefault($id)
{
        $page = dibi::fetch('SELECT * FROM [pages] WHERE [pageId] = %i', $id);
        if ($page === FALSE) throw new BadRequestException('Page not found!');
        $this->template->page = $page;
}

Stále zde bude problém s duplicitou (např. pro ?id=123ABC). Pokud doplníme jako výchozí hodnotu nulu, tak problém s duplicitou zmizí, ale objeví se dva problémy nové:

  1. Akceptování ?id=123ABC nemusí být žádoucí (osobně bych čekal 404, ne přesměrování na ?id=123).
  2. Nula získá speciální význam, takže najednou ji nelze mít v URL (?id=0 přesměruje na ?, aspoň myslím).

Anotace tento problém řeší velmi elegantně a čitelně. Bohužel máš pravdu, že lidé mají často v anotacích chyby. Osobně si to v rámci své perfekcionistické povahy docela hlídám, ale je mi jasné, že pro spoustu lidí to bude představovat problém.

Napadají mě tři způsoby, jak tento problém částečně řešit (nic ultimátního nemám).

1. Kombinace s tvým nápadem
Tvé řešení velmi elegantně chrání aplikaci před pádem, což je hlavní problém. Pokud bude anotační validace napsána omylem příliš přísně, tak si toho programátor (nebo jeho testy) všimnou, protože aplikace nebude fungovat. Pokud bude napsána volněji, tak tvá část řešení ochrání aplikaci před pádem a ostatní problémy jsou nekritické (duplicity, akceptování divných URL apod.).

2. Povinné uvedení názvu parametru
Jakkoliv se mi to hnusí, protože se jedná o zbytečnou a duplicitní informaci, tak je pravdou, že pokud by se povinně uváděl i název parametru, tak by to vedlo k výrazně nižšímu množství chyb způsobených nepozorností.

3. Přidat do Nette nástroj, který by problémy hledal
Nevím přesně, jak by fungoval, ale něco by se určitě napsat dalo :)


Připomínám existenci souvisejícího vlákna a experimentální implementace.

David Grudl
Nette Core | 7464
+
0
-

Zkusil jsem implementovat chování, které striktně rozlišuje mezi polem a skalárem a vyžaduje, aby parametr neoznačený explicitně jako pole (typehintem nebo výchozí hodnotou) musel být skalár https://github.com/…66f469c87554 – pochopitelně jde o slušný BC break ve chvíli, kdy se parametr s polem očekává. Pokud tohle necháme, bude to chtít dobře zdokumentovat.

A dále jsem zkusil implementovat tvůj návrh, aby se místo tichého přetypování typy striktně kontrolovaly https://github.com/…0a5434a5ccaf, tj. neplatné argumenty způsobí místo přesměrování chybu 404. To by nemělo k žádnému BC breaku vést, leda v případě, že by někdo třeba v šabloně odkaz s neplatnými argumenty vytvářel. V takovém případě se místo tichého přetypování vyhodí InvalidLinkException. Což je v duchu Nette pozitivní změna.

Jan Tvrdík
Nette guru | 2570
+
0
-

Díky, za implementaci. Asi by bylo dobré, založit pro to samostatné vlákno v changelogu. Takový BC break by neměl být utopen v rámci novinek v Nette 2.0.


Validace anotacemi mi ale stejně přijde hezčí :)

David Grudl
Nette Core | 7464
+
0
-

Zatím nejsem přesvědčen, jestli to tam fakt nechat. Nechtěl by někdo udělat ďáblova advokáta?

Jan Tvrdík
Nette guru | 2570
+
0
-

Varování: Následující věta obsahuje rýpnutí do tvé osoby.

Osobně bych to v souladu s mým názorem na vydávání verzí (představeným na Plzeňské Poslední sobotě, kdes bohužel nebyl) přesunul do větve (ta věc, které je pro rozumné vydávání verzí věc nezbytná a tebou zatím silně nevyužitá) pro 2.1, protože verzi 2.0 je třeba vydat a tohle bude vyžadovat dlouhodobější testování.

BTW: Nehledáš spíš ďáblova žalobce? Já myslel, že ten advokát jsem já. Ale možná se vůbec nechytám :)