Merge pull request #49515 from nextcloud/bugfix/noid/boolean-false-in…

…-multipart-form-data fix(controller): Fix false booleans in multipart/form-data
nextcloud · Nov 28, 2024 · dd101dd · dd101dd
2 parents 379f575 + 2b6da9e
commit dd101dd
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 16 deletions.
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
@@ -1384,9 +1384,6 @@
     </UndefinedInterfaceMethod>
   </file>
   <file src="lib/private/AppFramework/Http/Dispatcher.php">
-    <NoInterfaceProperties>
-      <code><![CDATA[$this->request->method]]></code>
-    </NoInterfaceProperties>
     <NullArgument>
       <code><![CDATA[null]]></code>
     </NullArgument>

diff --git a/lib/private/AppFramework/Http/Dispatcher.php b/lib/private/AppFramework/Http/Dispatcher.php
@@ -183,16 +183,8 @@ private function executeController(Controller $controller, string $methodName):
 			$value = $this->request->getParam($param, $default);
 			$type = $this->reflector->getType($param);
 
-			// if this is submitted using GET or a POST form, 'false' should be
-			// converted to false
-			if (($type === 'bool' || $type === 'boolean') &&
-				$value === 'false' &&
-				(
-					$this->request->method === 'GET' ||
-					str_contains($this->request->getHeader('Content-Type'),
-						'application/x-www-form-urlencoded')
-				)
-			) {
+			// Converted the string `'false'` to false when the controller wants a boolean
+			if ($value === 'false' && ($type === 'bool' || $type === 'boolean')) {
 				$value = false;
 			} elseif ($value !== null && \in_array($type, $types, true)) {
 				settype($value, $type);

diff --git a/tests/lib/AppFramework/Http/DispatcherTest.php b/tests/lib/AppFramework/Http/DispatcherTest.php
@@ -328,7 +328,7 @@ public function testControllerParametersInjected(): void {
 		$this->dispatcherPassthrough();
 		$response = $this->dispatcher->dispatch($controller, 'exec');
 
-		$this->assertEquals('[3,true,4,1]', $response[3]);
+		$this->assertEquals('[3,false,4,1]', $response[3]);
 	}
 
 
@@ -361,7 +361,7 @@ public function testControllerParametersInjectedDefaultOverwritten(): void {
 		$this->dispatcherPassthrough();
 		$response = $this->dispatcher->dispatch($controller, 'exec');
 
-		$this->assertEquals('[3,true,4,7]', $response[3]);
+		$this->assertEquals('[3,false,4,7]', $response[3]);
 	}
 
 
@@ -471,6 +471,41 @@ public function testResponseTransformedByAcceptHeader(): void {
 		$this->assertEquals('{"text":[3,false,4,1]}', $response[3]);
 	}
 
+	public function testResponseTransformedBySendingMultipartFormData(): void {
+		$this->request = new Request(
+			[
+				'post' => [
+					'int' => '3',
+					'bool' => 'false',
+					'double' => 1.2,
+				],
+				'server' => [
+					'HTTP_ACCEPT' => 'application/text, test',
+					'HTTP_CONTENT_TYPE' => 'multipart/form-data'
+				],
+				'method' => 'POST'
+			],
+			$this->createMock(IRequestId::class),
+			$this->createMock(IConfig::class)
+		);
+		$this->dispatcher = new Dispatcher(
+			$this->http, $this->middlewareDispatcher, $this->reflector,
+			$this->request,
+			$this->config,
+			\OC::$server->getDatabaseConnection(),
+			$this->logger,
+			$this->eventLogger,
+			$this->container
+		);
+		$controller = new TestController('app', $this->request);
+
+		// reflector is supposed to be called once
+		$this->dispatcherPassthrough();
+		$response = $this->dispatcher->dispatch($controller, 'exec');
+
+		$this->assertEquals('{"text":[3,false,4,1]}', $response[3]);
+	}
+
 
 	public function testResponsePrimarilyTransformedByParameterFormat(): void {
 		$this->request = new Request(
@@ -506,7 +541,7 @@ public function testResponsePrimarilyTransformedByParameterFormat(): void {
 		$this->dispatcherPassthrough();
 		$response = $this->dispatcher->dispatch($controller, 'exec');
 
-		$this->assertEquals('{"text":[3,true,4,1]}', $response[3]);
+		$this->assertEquals('{"text":[3,false,4,1]}', $response[3]);
 	}