linkeddata: Fix broken check in PageDataRequestHandler

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

diff --git a/includes/linkeddata/PageDataRequestHandler.php b/includes/linkeddata/PageDataRequestHandler.php
index 93aa89f..c90cea9 100644 (file)
--- 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 ad0c3d1..e2616c8 100644 (file)
--- 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$!' ]
                ];