Skip to content

Commit f983b2d

Browse files
dgdg
committed
ParameterConverter: malformed current-request params in self-links return 4xx, not 500
Building a 'this'/self link re-validates the current request's own (client-controlled) parameters via toParameters(). When such a value cannot be coerced to the target action signature it now throws InvalidRequestParameterException (a subtype of InvalidLinkException); redirect()/redirectPermanent()/forward() degrade it to a BadRequestException (4xx) and isLinkCurrent() to false, while link()/canonicalize() keep rendering '#error'. Explicitly passed link arguments still throw plain InvalidLinkException, preserving programmer-error detection.
1 parent 76b5344 commit f983b2d

5 files changed

Lines changed: 145 additions & 9 deletions

File tree

‎src/Application/UI/Component.php‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,11 @@ public function isLinkCurrent(?string $destination = null, $args = []): bool
329329
$args = func_num_args() < 3 && is_array($args)
330330
? $args
331331
: array_slice(func_get_args(), 1);
332-
$this->getPresenter()->getLinkGenerator()->createRequest($this, $destination, $args, 'test');
332+
try {
333+
$this->getPresenter()->getLinkGenerator()->createRequest($this, $destination, $args, 'test');
334+
} catch (InvalidRequestParameterException) {
335+
return false;
336+
}
333337
}
334338

335339
return $this->getPresenter()->getLastCreatedRequestFlag('current');
@@ -350,7 +354,13 @@ public function redirect(string $destination, $args = []): void
350354
: array_slice(func_get_args(), 1);
351355
$presenter = $this->getPresenter();
352356
$presenter->saveGlobalState();
353-
$presenter->redirectUrl($presenter->getLinkGenerator()->link($destination, $args, $this, 'redirect'));
357+
try {
358+
$url = $presenter->getLinkGenerator()->link($destination, $args, $this, 'redirect');
359+
} catch (InvalidRequestParameterException $e) {
360+
$this->error($e->getMessage());
361+
}
362+
363+
$presenter->redirectUrl($url);
354364
}
355365

356366

@@ -367,10 +377,13 @@ public function redirectPermanent(string $destination, $args = []): void
367377
? $args
368378
: array_slice(func_get_args(), 1);
369379
$presenter = $this->getPresenter();
370-
$presenter->redirectUrl(
371-
$presenter->getLinkGenerator()->link($destination, $args, $this, 'redirect'),
372-
Nette\Http\IResponse::S301_MovedPermanently,
373-
);
380+
try {
381+
$url = $presenter->getLinkGenerator()->link($destination, $args, $this, 'redirect');
382+
} catch (InvalidRequestParameterException $e) {
383+
$this->error($e->getMessage());
384+
}
385+
386+
$presenter->redirectUrl($url, Nette\Http\IResponse::S301_MovedPermanently);
374387
}
375388

376389

‎src/Application/UI/InvalidRequestParameterException.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php declare(strict_types=1);
2+
3+
/**
4+
* This file is part of the Nette Framework (https://nette.org)
5+
* Copyright (c) 2004 David Grudl (https://davidgrudl.com)
6+
*/
7+
8+
namespace Nette\Application\UI;
9+
10+
11+
/**
12+
* A self-link parameter taken from the current request could not be coerced;
13+
* treated as a client error (4xx) by redirect(), forward() and isLinkCurrent().
14+
*/
15+
class InvalidRequestParameterException extends InvalidLinkException
16+
{
17+
}

‎src/Application/UI/ParameterConverter.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ public static function toParameters(
8181
foreach ($method->getParameters() as $param) {
8282
$type = self::getType($param);
8383
$name = $param->getName();
84+
$fromSupplemental = false;
8485

8586
if (array_key_exists($i, $args)) {
8687
$args[$name] = $args[$i];
@@ -92,6 +93,7 @@ public static function toParameters(
9293

9394
} elseif (array_key_exists($name, $supplemental)) {
9495
$args[$name] = $supplemental[$name];
96+
$fromSupplemental = true;
9597
}
9698

9799
if (!isset($args[$name])) {
@@ -110,13 +112,16 @@ public static function toParameters(
110112
}
111113

112114
if (!self::convertType($args[$name], $type)) {
113-
throw new InvalidLinkException(sprintf(
115+
$message = sprintf(
114116
'Argument $%s passed to %s must be %s, %s given.',
115117
$name,
116118
Reflection::toString($method),
117119
$type,
118120
get_debug_type($args[$name]),
119-
));
121+
);
122+
throw $fromSupplemental
123+
? new InvalidRequestParameterException($message)
124+
: new InvalidLinkException($message);
120125
}
121126

122127
$def = $param->isDefaultValueAvailable()

‎src/Application/UI/Presenter.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,12 @@ public function forward(string|Nette\Application\Request $destination, $args = [
734734
$args = func_num_args() < 3 && is_array($args)
735735
? $args
736736
: array_slice(func_get_args(), 1);
737-
$request = $this->linkGenerator->createRequest($this, $destination, $args, 'forward');
737+
try {
738+
$request = $this->linkGenerator->createRequest($this, $destination, $args, 'forward');
739+
} catch (InvalidRequestParameterException $e) {
740+
$this->error($e->getMessage());
741+
}
742+
738743
$this->sendResponse(new Responses\ForwardResponse($request));
739744
}
740745

‎tests/UI/Presenter.selfLinkInvalidParam.phpt‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
<?php declare(strict_types=1);
2+
3+
/**
4+
* Test: a non-coercible current-request parameter in a self-link degrades to a 4xx,
5+
* not an uncaught 500.
6+
*/
7+
8+
use Nette\Application;
9+
use Nette\Application\UI\InvalidRequestParameterException;
10+
use Nette\Application\UI\InvalidLinkException;
11+
use Nette\Application\UI\Presenter;
12+
use Nette\Http;
13+
use Tester\Assert;
14+
15+
16+
require __DIR__ . '/../bootstrap.php';
17+
18+
19+
class SelfLinkPresenter extends Presenter
20+
{
21+
public string $op = '';
22+
public mixed $observed = null;
23+
24+
25+
protected function startup(): void
26+
{
27+
parent::startup();
28+
match ($this->op) {
29+
'redirect-this' => $this->redirect('this', ['keep' => 'x']),
30+
'redirect-explicit' => $this->redirect('default', ['tier' => 'abc']),
31+
'link-this' => $this->observed = $this->link('this'),
32+
'isLinkCurrent-this' => $this->observed = $this->isLinkCurrent('this'),
33+
};
34+
35+
$this->terminate();
36+
}
37+
38+
39+
public function renderDefault(?int $tier = null): void
40+
{
41+
}
42+
}
43+
44+
45+
function runSelfLink(string $op, array $params): SelfLinkPresenter
46+
{
47+
$factory = Mockery::mock(Application\IPresenterFactory::class);
48+
$factory->shouldReceive('getPresenterClass')->andReturnUsing(fn($name) => $name . 'Presenter');
49+
50+
$presenter = new SelfLinkPresenter;
51+
$presenter->op = $op;
52+
$presenter->autoCanonicalize = false;
53+
$presenter->invalidLinkMode = Presenter::InvalidLinkTextual;
54+
$presenter->injectPrimary(
55+
new Http\Request(new Http\UrlScript('http://localhost/index.php', '/index.php')),
56+
new Http\Response,
57+
$factory,
58+
new Application\Routers\SimpleRouter,
59+
);
60+
$presenter->run(new Application\Request('Self', Http\Request::Get, $params + ['action' => 'default']));
61+
return $presenter;
62+
}
63+
64+
65+
// BC: subtype of InvalidLinkException
66+
Assert::type(InvalidLinkException::class, new InvalidRequestParameterException);
67+
68+
69+
// self-link with a bad current param -> 4xx
70+
Assert::exception(
71+
fn() => runSelfLink('redirect-this', ['tier' => 'abc']),
72+
Application\BadRequestException::class,
73+
'Argument $tier passed to SelfLinkPresenter::renderDefault() must be ?int, string given.',
74+
);
75+
76+
// a valid value still redirects normally
77+
Assert::noError(fn() => runSelfLink('redirect-this', ['tier' => '5']));
78+
79+
80+
// isLinkCurrent('this') -> false, no throw
81+
Assert::false(runSelfLink('isLinkCurrent-this', ['tier' => 'abc'])->observed);
82+
83+
84+
// link('this') stays graceful (#error), no leak
85+
Assert::same(
86+
'#error: Argument $tier passed to SelfLinkPresenter::renderDefault() must be ?int, string given.',
87+
runSelfLink('link-this', ['tier' => 'abc'])->observed,
88+
);
89+
90+
91+
// explicit bad arg -> InvalidLinkException (programmer error)
92+
Assert::exception(
93+
fn() => runSelfLink('redirect-explicit', []),
94+
InvalidLinkException::class,
95+
'Argument $tier passed to SelfLinkPresenter::renderDefault() must be ?int, string given.',
96+
);

0 commit comments

Comments
 (0)