Aplikace padne, když dostane do parametru presenteru neočekávané pole

Eda
Backer | 220
+
0
-

Zdarec.

Přišel mi report z jednoho mého webu.

Zlý robot začal zkoušet různé hodnoty GET parametrů na různých stránkách a narazil na něco, co mi shodí aplikaci.

Stačí do nějakého standardního parametru presenteru poslat pole místo skalární hodnoty, která je očekávána.

Metoda action presenteru vypadá třeba takto:

	public function actionDefault($regionId)
	{
		$this->regionId = $regionId;
	}

Routu mám takovou:

		$router[] = new Route('kraj/<regionId>', [
			'presenter' => 'Region',
			'action' => 'default',
		]);

Plus samozřejmě na konci routeru standardní (pojistkovou routu):

		$router[] = new Route('<presenter>[/<action>]', [
			'presenter' => 'Homepage',
			'action' => 'default',
		]);

Standardně URL vypadá takto:

/kraj/5

Když ale robot přistoupí na URL:

/region?regionId[countryId]=2

Spadne to na pětistovce s Argument $regionId passed to RegionPresenter::actionDefault() must be scalar, array given.

Nechytne se to totiž do „své routy“, ale až do té poslední (pojistkové). Takže i kdybych validoval parametry v té první routě, vůbec by to nepomohlo.

Tu rezervní routu ale oddělat nemůžu, protože některé presentery jsou záměrně routované přes ni.

Co s tím?

Vsadím se, že tímto způsobem by se dalo shodit docela dost Nette aplikací, protože tohle fakt nenapadne nikoho ošetřit. Nicméně to, že tu URL, na které to spadne, nikdo nezná, neznamená, že bychom to neměli řešit.

Nebo existuje nějaké ultimátní řešení?

Editoval Eda (1. 6. 2017 1:46)

David Grudl
Nette Core | 8111
+
0
-

To je divné, má to být normální 404. Jakou máš verzi nette/application?

Eda
Backer | 220
+
0
-

v2.4.4

Verze ostatních balíčků: http://www.imgup.cz/…01-59-52.png

David Grudl
Nette Core | 8111
+
0
-

Co je to za třídu výjimky?

Eda
Backer | 220
+
0
-

Jejda, koukám, že je to trochu jinak a u toho sandboxu to funguje správně :-/

V rámci osekání příkladu pro fórum jsem routy a akci trochu zjednodušil. A u tak jednoduchého příkladu to opravdu háže správně 404.

Tak tedy kompletní příklad, který se liší tím, že v akci presenteru je typehint pro Entitu:

RegionPresenter:

	public function actionDefault(RegionEntity $region)
	{
		$this->region = $region;
	}

RouterFactory:

		$router[] = new Route($baseUrl.$langParam.'kraj/<region>', [
			'presenter' => 'Region',
			'action' => 'default',
			'region' => [
				Route::FILTER_IN => function ($url) {
					$region = $this->regionRepository->findOne([
						'countryId' =>RegionRepository::CZECH_REPUBLIC_ID,
						'url' => $url,
					]);

					return $region;
				},
				Route::FILTER_OUT => function ($entity) {
					/** @var RegionEntity $entity */
					$ret = null;

					if (
						$entity instanceof RegionEntity &&
						$entity->countryId === RegionRepository::CZECH_REPUBLIC_ID
					) {
						$ret = $entity->url;
					}

					return $ret;
				},
			],
			'locale' => $defaultLang,
		]);
...
		$router[] = new Route($baseUrl.$langParam.'<presenter>[/<action>]?strana=<page>', [
			'presenter' => [
				Route::VALUE => 'Homepage',
				Route::FILTER_TABLE => [
					'vyhledavani' => 'Search',
					'poslednidokumentacky' => 'Photogallery',
					'owebu' => 'PageAbout',
					'navody' => 'PageGuides',
					'spravci' => 'PageAdministrators',
				],
			],
			'action' => 'default',
			'page' => 1,
			'locale' => $defaultLang,
		]);

Chyba pak je na URL:

/region?region[countryId]=2

A konkrétně:

Nette\InvalidArgumentException

Argument $region passed to BusyModule\RegionPresenter::actionDefault() must be BMHD\Model\RegionEntity, array given

Matchne se ta druhá routa.

Pro jistotu ještě screen výjimky: http://www.imgup.cz/image/LgIE


Omlouvám se za to zmatení se sandboxem a zjednodušeným příkladem.

Teď už snad půjde identifikovat problematické místo.

Editoval Eda (1. 6. 2017 2:24)

David Grudl
Nette Core | 8111
+
0
-

Tohle je věc, která se mi nelíbí, kdy se vytváří závislost presenteru na konkrétní routě.

Možná (už mi to nemyslí) to souvisí s tímto https://github.com/…it/212ec4301 a pak možná pomůže update na nette/application 2.4.5.

ondraondra81
Člen | 82
+
+1
-

Mit v action typehint pro object je chyba v navrhu. Router se nema starat o data ale o routovani. Nacteni entity z db ma resit az presenter!

Nejak me unika jak schodi aplikaci? Spadne to do 500 coz je ale podle me spravne, resit jestli bot dostane 404 nebo 500 je podle me ztrata casu

Eda
Backer | 220
+
+2
-

@DavidGrudl No, to vytváří. Ale je to mnohem čistší takto. Presenter potřebuje entitu, tak dostane rovnou entitu. A když budu pak chtít změnit routování, místo ID entity v URL mít třeba nějaký slug, jediné místo, které upravím, je router. Což je správně. Od toho tady router je, aby presenter odstínil od toho, jak, bude URL vypadat.

Zatímco když to budu dělat „postaru“ tak, jak (asi) chceš ty (IDčko v parametru akce), tak pokud se rozhodnu časem routovat jinak (místo ID slugem), kolik budu muset upravit míst? Ano. Úplně všechny (presenteří akce + úplně všechna místa, kde na tu akci routuju – lattečka + PHP). A to je třeba na stovkách míst v našem frameworku, který v INSPIRE používáme. Tzn. takovýto refaktoring/úprava je nereálný (jestliže to pohání 100 projektů…). A na každém projektu se routuje jinak. Záleží to na požadavku klienta. Ve frameworku/CMS prostě nemůžeme routovat takto pevně.

Navíc to přináší hezkou věc a to je to, že nemusím mít v úplně každém presenteru:

public function actionDefault($id)
{
	$entity = $this->manager->findOneById($id);

    if (!$entity) {
        $this->error('Nic nenalezeno...');
    }
}

Což mnohdy znamená, že managera ani nemusím injectovat → další zpřehlednění presenteru.

Stejně jako ve firmě to používám i doma a je to super přehledné.

Jak to tedy máme dělat, abychom nemuseli při změně routování měnit všechna místa, kde routujeme?

@ondraondra81 Nemyslím si. Presenter má dostat to, co ve skutečnosti potřebuje. A to je v tomto případě entita. Jak se na ni naroutuje, je již věc routování. A pokud kontroluje presenter, jaké ID je validní a jaké ne, tak je to tříštění zodpovědnosti za routování – „trochu“ tu URL zvaliduje router a „trochu“ presenter.

Tak 500 znamená pád aplikace. Podle mne je spíš ztráta času furt mazat nagenerované laděnky s pětistovkou.

Navíc vědomě mít v aplikaci URL, která háže 500, odmítám. Chyby fakt nelogujeme kvůli tomu, abychom je ignorovali.

A to, že je to robot, je jen náhoda. Taky to mohl být třeba Spaze, který testuje, jak máme aplikaci zabezpečenou :-)

Felix
Nette Core | 1186
+
+4
-

Eda napsal(a):

Jak to tedy máme dělat, abychom nemuseli při změně routování měnit všechna místa, kde routujeme?

Premyslim nad 2 vecmi.

  1. Co kdyz by jsi chtel zobrazit nejakou specialni stranku, ze produkt neni dostupny? Nebo ze neexistuje, ale misto toho mu nabidnes jiny. Kdyz je to v rukach routeru, tak ses odkazanej na Error:404? A jeho sablonu? Jenom se tak ptam, jestli jsi to neresil.
  2. Mozna pridat nejakou mezi vrstvu, mezi routerem a presenterem. Rekneme, ze router by to naroutoval a mohl by se zavolat parameterResolver. Je otazka kdy ale..? No rekneme, ze na Application::onPresenter(). To by se mi celkem libilo, asi to pujdu zkusit udelat. :-)
Eda
Backer | 220
+
0
-
  1. Jo, tohle řešíš v Error presenteru. Nebo si uděláš spešl presenter + routu (méně striktní), která ti tyto případy odchytne.
  2. Tohle mě taky napadlo. Nějaký takový middleware, který by „hydratoval“ nebo překládal parametry. To by bylo fajn řešení.