diff --git a/app/controllers/studip_controller.php b/app/controllers/studip_controller.php
index 0e340d6d722485a18f58df482d3c7aadff2cb0f3..556ff8b6284b866679fc2636c9fd186484d347d1 100644
--- a/app/controllers/studip_controller.php
+++ b/app/controllers/studip_controller.php
@@ -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.
diff --git a/tests/unit/lib/classes/StudipControllerTest.php b/tests/unit/lib/classes/StudipControllerTest.php
new file mode 100644
index 0000000000000000000000000000000000000000..db2d6591aac6b6bab1add9859fd3c96807959d95
--- /dev/null
+++ b/tests/unit/lib/classes/StudipControllerTest.php
@@ -0,0 +1,216 @@
+<?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]],
+ ];
+ }
+}
+