resurrect trails parameter type 'string', fixes #2165

Closes #2165

Merge request studip/studip!1399

app/controllers/studip_controller.php

@@ -145,11 +145,11 @@ abstract class StudipController extends Trails_Controller
 
     /**
      * Validate arguments based on a list of given types. The types are:
-     * 'int', 'float', 'option'. If the list of types is NULL
+     * 'int', 'float', 'option' and 'string'. If the list of types is NULL
      * or shorter than the argument list, 'option' is assumed for all
      * remaining arguments. 'option' differs from Request::option() in
      * that it also accepts the charaters '-' and ',' in addition to all
-     * word charaters.
+     * word characters.
      *
      * Since Stud.IP 4.0 it is also possible to directly inject
      * SimpleORMap objects. If types is NULL, the signature of the called
@@ -160,15 +160,15 @@ abstract class StudipController extends Trails_Controller
      * If $_autobind is set to true, the created object is also assigned
      * to the controller so that it is available in a view.
      *
-     * @param array   an array of arguments to the action
-     * @param array   list of argument types (optional)
+     * @param array $args  an array of arguments to the action
+     * @param array $types list of argument types (optional)
      */
     public function validate_args(&$args, $types = null)
     {
         $class_infos = [];
 
         if ($types === null) {
-            $types = array_fill(0, count($args), 'option');
+            $types = [];
         }
 
         if ($this->has_action($this->current_action)) {
@@ -177,10 +177,11 @@ abstract class StudipController extends Trails_Controller
             foreach ($parameters as $i => $parameter) {
                 $class_type = $parameter->getType();
 
-                if (!$class_type
+                if (
+                    !$class_type
                     || !class_exists($class_type->getName())
-                    || !is_a($class_type->getName(), SimpleORMap::class, true))
-                {
+                    || !is_a($class_type->getName(), SimpleORMap::class, true)
+                ) {
                     continue;
                 }
 
@@ -198,7 +199,7 @@ abstract class StudipController extends Trails_Controller
         }
 
         foreach ($args as $i => &$arg) {
-            $type = $types[$i] ?: 'option';
+            $type = $types[$i] ?? 'option';
             switch ($type) {
                 case 'int':
                     $arg = (int) $arg;
@@ -241,6 +242,9 @@ abstract class StudipController extends Trails_Controller
                     }
                     break;
 
+                case 'string':
+                    break;
+
                 default:
                     throw new Trails_Exception(500, 'Unknown type "' . $type . '"');
             }

tests/unit/lib/classes/StudipControllerTest.php

@@ -228,6 +228,29 @@ final class StudipControllerTest extends Codeception\Test\Unit
         $this->assertEquals([23], $variables['bad']);
     }
 
+    /**
+     * @dataProvider argumentValidationProvider
+     * @covers StudipController::validate_args
+     */
+    public function testArgumentValidation(array $args, array $expected, ?array $types): void
+    {
+        $this->getController()->validate_args($args, $types);
+
+        $this->assertEquals($expected, $args);
+    }
+
+    /**
+     * @dataProvider argumentValidationOptionProvider
+     * @covers StudipController::validate_args
+     */
+    public function testArgumentValidationOption(string $should_fail): void
+    {
+        $args = [$should_fail];
+
+        $this->expectException(Trails_Exception::class);
+        $this->getController()->validate_args($args);
+    }
+
     /**
      * Returns a relative url for Stud.IP if given url matches.
      */
@@ -293,6 +316,29 @@ final class StudipControllerTest extends Codeception\Test\Unit
             'url-and-parameters' => [false, 'https://example.org', ['foo' => 'bar', 'baz' => 42]],
         ];
     }
+
+    public function argumentValidationProvider(): array
+    {
+        return [
+            'default' => [['2', '2,5', 'abc', 'a-b-c'], ['2', '2,5', 'abc', 'a-b-c'], null],
+            'typed'   => [['2.5', '2.5', 'abc', 'a-b-c'], [2, 2.5, 'abc', 'a-b-c'], ['int', 'float', 'option', 'string']],
+            'int'     => [['1.5', '2.5', '-1.5'], [1, 2, -1], ['int', 'int', 'int']],
+            'float'   => [['1.5', '2.5', '-1.5'], [1.5, 2.5, -1.5], ['float', 'float', 'float']],
+            'option'   => [['a-b', '1,2,3', 'foobar'], ['a-b', '1,2,3', 'foobar'], ['option', 'option', 'option']],
+            'string'   => [['a', 'b!', 'http://c#?'], ['a', 'b!', 'http://c#?'], ['string', 'string', 'string']],
+        ];
+    }
+
+    public function argumentValidationOptionProvider(): array
+    {
+        return [
+            'space'               => [' '],
+            'float'               => ['1.5'],
+            'url'                 => ['http://example.org'],
+            'special'             => ['a#b'],
+            'little-bobby-tables' => ["Robert'); DROP TABLE STUDENTS; --"],
+        ];
+    }
 }
 
 class StudipControllerTestController extends StudipController
@@ -303,4 +349,9 @@ class StudipControllerTestController extends StudipController
 
         $this->current_action = 'foo';
     }
+
+    public function foo_action(int $int, float $float, string $option, string $string): void
+    {
+        $this->render_text('In a test?');
+    }
 }