From 3821a2b5b77109e187a666d04d7dd6787aea44dd Mon Sep 17 00:00:00 2001
From: Thiemo Kreuz <thiemo.kreuz@wikimedia.de>
Date: Wed, 27 Mar 2019 16:22:39 +0100
Subject: [PATCH] linkeddata: Fix broken check in PageDataRequestHandler
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

First I noticed a weird check that assumes that explode() would return
an integer. But it returns an array of strings. There is obviously a
count() missing.

Then I started simplifying the code:
* The count can only be 2 or 1, never anything else.
* If it is one, the first element in the array contains the original string.
* The empty string is already checked above and can never end down there.
* Finally I made the remaining `if ( … ) return true else return false` a
  straight `return …`.

Change-Id: I289144e64f449ee0875009aaa22e10a5c0eb2734
---
 .../linkeddata/PageDataRequestHandler.php     | 17 +++-----
 .../linkeddata/PageDataRequestHandlerTest.php | 39 ++++++++++++++++---
 2 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/includes/linkeddata/PageDataRequestHandler.php b/includes/linkeddata/PageDataRequestHandler.php
index 93aa89f724..c90cea9426 100644
--- a/includes/linkeddata/PageDataRequestHandler.php
+++ b/includes/linkeddata/PageDataRequestHandler.php
@@ -47,20 +47,15 @@ class PageDataRequestHandler {
 		}
 
 		$parts = explode( '/', $subPage, 2 );
-		if ( $parts !== 2 ) {
-			$slot = $parts[0];
-			if ( $slot === 'main' || $slot === '' ) {
-				return true;
-			}
-		}
-
-		return false;
+		$slot = $parts[0];
+		$title = $parts[1] ?? '';
+		return ( $slot === 'main' || $slot === '' ) && $title !== '';
 	}
 
 	/**
 	 * Main method for handling requests.
 	 *
-	 * @param string $subPage
+	 * @param string|null $subPage
 	 * @param WebRequest $request The request parameters. Known parameters are:
 	 *        - title: the page title
 	 *        - format: the format
@@ -82,9 +77,9 @@ class PageDataRequestHandler {
 
 		$revision = 0;
 
-		$parts = explode( '/', $subPage, 2 );
 		if ( $subPage !== '' ) {
-			$title = $parts[1];
+			$parts = explode( '/', $subPage, 2 );
+			$title = $parts[1] ?? '';
 		} else {
 			$title = $request->getText( 'target' );
 		}
diff --git a/tests/phpunit/includes/linkeddata/PageDataRequestHandlerTest.php b/tests/phpunit/includes/linkeddata/PageDataRequestHandlerTest.php
index ad0c3d1eae..e2616c8869 100644
--- a/tests/phpunit/includes/linkeddata/PageDataRequestHandlerTest.php
+++ b/tests/phpunit/includes/linkeddata/PageDataRequestHandlerTest.php
@@ -99,7 +99,7 @@ class PageDataRequestHandlerTest extends \MediaWikiTestCase {
 			'',
 			[ 'target' => 'Helsinki' ],
 			[ 'Accept' => 'text/HTML' ],
-			'!!',
+			'!^$!',
 			303,
 			[ 'Location' => '!Helsinki$!' ]
 		];
@@ -111,7 +111,7 @@ class PageDataRequestHandlerTest extends \MediaWikiTestCase {
 				'revision' => '4242',
 			],
 			[ 'Accept' => 'text/HTML' ],
-			'!!',
+			'!^$!',
 			303,
 			[ 'Location' => '!Helsinki(\?|&)oldid=4242!' ]
 		];
@@ -120,7 +120,7 @@ class PageDataRequestHandlerTest extends \MediaWikiTestCase {
 			'/Helsinki',
 			[],
 			[],
-			'!!',
+			'!^$!',
 			303,
 			[ 'Location' => '!Helsinki&action=raw!' ]
 		];
@@ -136,10 +136,37 @@ class PageDataRequestHandlerTest extends \MediaWikiTestCase {
 		];
 
 		$cases[] = [
-			'main/Helsinki',
+			'no slash',
+			[],
+			[ 'Accept' => 'text/HTML' ],
+			'!!',
+			400,
+			[]
+		];
+
+		$cases[] = [
+			'main',
+			[],
+			[ 'Accept' => 'text/HTML' ],
+			'!!',
+			400,
+			[]
+		];
+
+		$cases[] = [
+			'xyz/Helsinki',
 			[],
 			[ 'Accept' => 'text/HTML' ],
 			'!!',
+			400,
+			[]
+		];
+
+		$cases[] = [
+			'main/Helsinki',
+			[],
+			[ 'Accept' => 'text/HTML' ],
+			'!^$!',
 			303,
 			[ 'Location' => '!Helsinki$!' ]
 		];
@@ -148,7 +175,7 @@ class PageDataRequestHandlerTest extends \MediaWikiTestCase {
 			'/Helsinki',
 			[],
 			[ 'Accept' => 'text/HTML' ],
-			'!!',
+			'!^$!',
 			303,
 			[ 'Location' => '!Helsinki$!' ]
 		];
@@ -157,7 +184,7 @@ class PageDataRequestHandlerTest extends \MediaWikiTestCase {
 			'main/AC/DC',
 			[],
 			[ 'Accept' => 'text/HTML' ],
-			'!!',
+			'!^$!',
 			303,
 			[ 'Location' => '!AC/DC$!' ]
 		];
-- 
2.20.1