Skip to content
Snippets Groups Projects
Commit 17c877e0 authored by Jan-Hendrik Willms's avatar Jan-Hendrik Willms
Browse files

fix StudipController::redirect() accepting URLs, fixes #642

parent 96dd4204
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
app/controllers/studip_controller.php
+ 52
− 19
View file @ 17c877e0
......@@ -252,6 +252,21 @@ abstract class StudipController extends Trails_Controller
{
$args = func_get_args();
// Create url for a specific action
// TODO: This seems odd. You kinda specify an absolute path
// to receive a relative url. Meh...
//
// @deprecated Do not use this, please!
if ($to[0] === '/') {
$args[0] = substr($to, 1);
return $this->action_url(...$args);
}
// Check for absolute URL
if ($this->isURL($to)) {
throw new InvalidArgumentException(__METHOD__ . ' cannot be used with absolute URLs');
}
// Extract parameters (if any)
$params = [];
if (is_array(end($args))) {
......@@ -280,16 +295,8 @@ abstract class StudipController extends Trails_Controller
: $this->current_action;
}
// Create url for a specific action
// TODO: This seems odd. You kinda specify an absolute path
// to receive a relative url. Meh...
if ($to[0] === '/') {
$to = $this->controller_path() . $to;
}
$url = parent::url_for($to);
$url = preg_match('#^(/|\w+://)#', $to)
? $to
: parent::url_for($to);
if ($fragment) {
$url .= '#' . $fragment;
}
......@@ -321,10 +328,9 @@ abstract class StudipController extends Trails_Controller
*/
public function redirect($to)
{
if (func_num_args() > 1) {
$to = $this->url_for(...func_get_args());
}
return parent::redirect($to);
$to = $this->adjustToArguments(...func_get_args());
parent::redirect($to);
}
/**
......@@ -340,13 +346,9 @@ abstract class StudipController extends Trails_Controller
*/
public function relocate($to)
{
$from_dialog = Request::isDialog();
if (func_num_args() > 1 || $from_dialog) {
$to = $this->url_for(...func_get_args());
}
$to = $this->adjustToArguments(...func_get_args());
if ($from_dialog) {
if (Request::isDialog()) {
$this->response->add_header('X-Location', rawurlencode($to));
$this->render_nothing();
} else {
......@@ -354,6 +356,37 @@ abstract class StudipController extends Trails_Controller
}
}
/**
* Returns a URL to a specified route to your Trails application, unless
* the parameter is already a valid URL (which is returned unchanged).
*
* If no absolute url or more than one argument is given, url_for() is
* used.
*/
private function adjustToArguments(...$args): string
{
if ($this->isURL($args[0]) && count($args) > 1) {
throw new InvalidArgumentException('Method may not be used with a URL and multiple parameters');
}
if (count($args) > 1 || !$this->isURL($args[0])) {
return $this->url_for(...$args);
}
return $args[0];
}
/**
* Returns whether the given parameter is a valid url.
*
* @param string $to
* @return bool
*/
private function isURL(string $to): bool
{
return preg_match('#^(/|\w+://)#', $to);
}
/**
* Exception handler called when the performance of an action raises an
* exception.
......
tests/unit/lib/classes/StudipControllerTest.php 0 → 100644
+ 216
− 0
View file @ 17c877e0
<?php
/**
* StudipControllerTest.php - unit tests for the StudipController class
*
* @author Jan-Hendrik Willms <tleilax+studip@gmail.com>
* @license GPL2 or any later version
*
* @covers StudipController
*/
final class StudipControllerTest extends Codeception\Test\Unit
{
private $old_uri;
public static function setUpBeforeClass(): void
{
parent::setUpBeforeClass();
require_once 'vendor/trails/trails-abridged.php';
require_once 'app/controllers/studip_controller.php';
}
public function setUp(): void
{
parent::setUp();
$this->old_uri = $GLOBALS['ABSOLUTE_URI_STUDIP'] ?? null;
$GLOBALS['ABSOLUTE_URI_STUDIP'] = 'https://studip.example.org/';
}
public function tearDown(): void
{
$GLOBALS['ABSOLUTE_URI_STUDIP'] = $this->old_uri;
parent::tearDown();
}
private function getDispatcher(): Trails_Dispatcher
{
$trails_root = $GLOBALS['STUDIP_BASE_PATH'] . DIRECTORY_SEPARATOR . 'app';
$trails_uri = rtrim($GLOBALS['ABSOLUTE_URI_STUDIP'], '/') . '/dispatch.php';
$default_controller = 'default';
return new Trails_Dispatcher($trails_root, $trails_uri, $default_controller);
}
private function getController(): StudipController
{
$dispatcher = $this->getDispatcher();
return new class($dispatcher) extends StudipController {
};
}
/**
* @dataProvider UrlForProvider
* @covers StudipController::url_for
*/
public function testUrlFor(string $expected, ...$args): void
{
$url = $this->getController()->url_for(...$args);
$this->assertEquals(
$expected,
$this->getRelativeURL($url)
);
}
/**
* @dataProvider absoluteUrlForProvider
* @covers StudipController::url_for
*/
public function testUrlForWithAbsoluteURL(...$args): void
{
$this->expectException(InvalidArgumentException::class);
$this->getController()->url_for(...$args);
}
/**
* @dataProvider UrlForProvider
* @covers StudipController::redirect
*/
public function testRedirect(string $expected, ...$args): void
{
$controller = $this->getController();
$controller->redirect(...$args);
$headers = $controller->get_response()->headers;
$this->assertArrayHasKey('Location', $headers);
$this->assertEquals(
$expected,
$this->getRelativeURL($headers['Location'])
);
}
/**
* @dataProvider absoluteRedirectProvider
* @covers StudipController::redirect
*/
public function testRedirectAbsoluteURL(bool $should_suceed, ...$args): void
{
if (!$should_suceed) {
$this->expectException(InvalidArgumentException::class);
}
$this->getController()->redirect(...$args);
}
/**
* @dataProvider UrlForProvider
* @covers StudipController::relocate
*/
public function testRelocate(string $expected, ...$args): void
{
$controller = $this->getController();
$controller->relocate(...$args);
$headers = $controller->get_response()->headers;
$this->assertArrayHasKey('Location', $headers);
$this->assertEquals(
$expected,
$this->getRelativeURL($headers['Location'])
);
}
/**
* @dataProvider absoluteRedirectProvider
* @covers StudipController::relocate
*/
public function testRelocateAbsoluteURL(bool $should_suceed, ...$args): void
{
if (!$should_suceed) {
$this->expectException(InvalidArgumentException::class);
}
$this->getController()->relocate(...$args);
}
/**
* @dataProvider UrlForProvider
* @covers StudipController::relocate
* @backupGlobals enabled
*/
public function testRelocateFromDialog(string $expected, ...$args): void
{
$_SERVER['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest';
$_SERVER['HTTP_X_DIALOG'] = true;
$controller = $this->getController();
$controller->relocate(...$args);
$headers = $controller->get_response()->headers;
$this->assertArrayHasKey('X-Location', $headers);
$this->assertEquals(
$expected,
$this->getRelativeURL(rawurldecode($headers['X-Location']))
);
}
/**
* @dataProvider absoluteRedirectProvider
* @covers StudipController::relocate
* @backupGlobals enabled
*/
public function testRelocateFromDialogAbsoluteURL(bool $should_suceed, ...$args): void
{
$_SERVER['HTTP_X_REQUESTED_WITH'] = 'XMLHttpRequest';
$_SERVER['HTTP_X_DIALOG'] = true;
if (!$should_suceed) {
$this->expectException(InvalidArgumentException::class);
}
$this->getController()->relocate(...$args);
}
/**
* Returns a relative url for Stud.IP if given url matches.
*/
private function getRelativeURL(string $url): string
{
if (strpos($url, $GLOBALS['ABSOLUTE_URI_STUDIP']) === 0) {
return str_replace($GLOBALS['ABSOLUTE_URI_STUDIP'], '', $url);
}
return $url;
}
public function UrlForProvider(): array
{
return [
'1-action' => ['dispatch.php/foo', 'foo'],
'1-action-and-parameter' => ['dispatch.php/foo?bar=42', 'foo', ['bar' => 42]],
'1-action-and-parameters' => ['dispatch.php/foo?bar=42&baz=23', 'foo', ['bar' => 42, 'baz' => 23]],
'2-actions' => ['dispatch.php/foo/bar', 'foo', 'bar'],
'2-actions-and-parameter' => ['dispatch.php/foo/bar?bar=42', 'foo', 'bar', ['bar' => 42]],
'2-actions-and-parameters' => ['dispatch.php/foo/bar?bar=42&baz=23', 'foo', 'bar', ['bar' => 42, 'baz' => 23]],
];
}
public function absoluteUrlForProvider(): array
{
return [
'url' => ['https://example.org'],
'url-and-parameters' => ['https://example.org', ['foo' => 'bar', 'baz' => 42]],
];
}
public function absoluteRedirectProvider(): array
{
return [
'url' => [true, 'https://example.org'],
'url-and-parameters' => [false, 'https://example.org', ['foo' => 'bar', 'baz' => 42]],
];
}
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment