Accept x-www-form-urlencoded in NCNv1

lib/REST/NextcloudNews/V1_2.php

@@ -16,6 +16,7 @@ use JKingWeb\Arsse\Db\ExceptionInput;
 use JKingWeb\Arsse\Feed\Exception as FeedException;
 use JKingWeb\Arsse\Misc\HTTP;
 use JKingWeb\Arsse\REST\Exception;
+use MensBeam\Mime\MimeType;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\ResponseInterface;
 
@@ -87,16 +88,51 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
         } else {
             return HTTP::respEmpty(401);
         }
-        // normalize the input
+        // parse the input
         $data = (string) $req->getBody();
         if ($data) {
-            // if the entity body is not JSON according to content type, return "415 Unsupported Media Type"
+            // Officially the body, if any, should be JSON. In practice the
-            if (!HTTP::matchType($req, [self::ACCEPTED_TYPE])) {
+            //   server must also accept application/x-www-form-urlencoded;
+            //   it's also possible that input is mislabelled, so we'll try
+            //   different combinations till something works, or return an
+            //   error status in the end
+            $type = MimeType::extract($req->getHeaderLine("Content-Type"));
+            try {
+                switch ($type->essence ?? "") {
+                    case "application/json":
+                    case "text/json":
+                        $data = $this->parseJson($data);
+                        break;
+                    case "application/x-www-form-urlencoded":
+                        if ($this->guessForm($data)) {
+                            $data = $this->parseForm($data);
+                        } else {
+                            $data = $this->parseJson($data);
+                        }
+                        break;
+                    case "":
+                        if ($this->guessJson($data)) {
+                            $data = $this->parseJson($data);
+                        } elseif ($this->guessForm($data)) {
+                            $data = $this->parseForm($data);
+                        } else {
+                            return HTTP::respEmpty(400);
+                        }
+                        break;
+                    default:
+                        // other media types would normally be rejected, but 
+                        //   if it happens to be mislabelled JSON we can accept
+                        //   it; we will not try form data here, though,
+                        //   because input is really expected to be JSON
+                        if ($this->guessJson($data)) {
+                            try {
+                                $data = $this->parseJson($data);
+                                break;
+                            } catch (\JsonException $e) {}
+                        }
                         return HTTP::respEmpty(415, ['Accept' => self::ACCEPTED_TYPE]);
                 }
-            $data = @json_decode($data, true);
+            } catch (\JsonException $e) {
-            if (json_last_error() !== \JSON_ERROR_NONE) {
-                // if the body could not be parsed as JSON, return "400 Bad Request"
                 return HTTP::respEmpty(400);
             }
         } else {
@@ -124,6 +160,28 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
         // @codeCoverageIgnoreEnd
     }
 
+    protected function guessJson(string $data): bool {
+        return (bool) preg_match('/^\s*\{\s*"[a-zA-Z]+"\s*:/s', $data);
+    }
+
+    protected function parseJson(string $data): array {
+        $out = json_decode($data, true, 512, \JSON_THROW_ON_ERROR);
+        if (!is_array($out)) {
+            throw new \JsonException("JSON input must be an object");
+        }
+        return $out;
+    }
+
+    protected function guessForm(string $data): bool {
+        return (bool) preg_match('/^\s*[a-zA-Z]+=/s', $data);
+    }
+
+    protected function parseForm(string $data): array {
+        // we assume that, as PHP application, Nextcloud News uses PHP logic for interpreting form data
+        parse_str($data, $out); // this cannot fail as any string can be interpreted into some sort of array
+        return $out;
+    }
+
     protected function normalizePathIds(string $url): string {
         $path = explode("/", $url);
         // any path components which are database IDs (integers greater than zero) should be replaced with "1", for easier comparison (we don't care about the specific ID)

tests/cases/REST/NextcloudNews/TestV1_2.php

@@ -26,6 +26,7 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
     protected $h;
     protected $transaction;
     protected $userId;
+    protected $prefix = "/index.php/apps/news/api/v1-2";
     protected static $feeds = [ // expected sample output of a feed list from the database, and the resultant expected transformation by the REST handler
         'db' => [
             [
@@ -307,20 +308,6 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         ],
     ];
 
-    protected function req(string $method, string $target, $data = "", array $headers = [], bool $authenticated = true, bool $body = true): ResponseInterface {
-        Arsse::$user->id = $this->userId;
-        $prefix = "/index.php/apps/news/api/v1-2";
-        $url = $prefix.$target;
-        if ($body) {
-            $params = [];
-        } else {
-            $params = $data;
-            $data = [];
-        }
-        $req = $this->serverRequest($method, $url, $prefix, $headers, [], $data, "application/json", $params, $authenticated ? "john.doe@example.com" : "");
-        return $this->h->dispatch($req);
-    }
-
     public function setUp(): void {
         parent::setUp();
         self::setConf();
@@ -329,6 +316,7 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         Arsse::$user = \Phake::mock(User::class);
         \Phake::when(Arsse::$user)->auth->thenReturn(true);
         \Phake::when(Arsse::$user)->propertiesGet->thenReturn(['admin' => true]);
+        Arsse::$user->id = $this->userId;
         // create a mock database interface
         Arsse::$db = \Phake::mock(Database::class);
         \Phake::when(Arsse::$db)->begin->thenReturn(\Phake::mock(Transaction::class));
@@ -340,6 +328,24 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         return $value;
     }
 
+    protected function req(string $method, string $target, $data = "", array $headers = [], bool $authenticated = true, bool $body = true): ResponseInterface {
+        $url = $this->prefix.$target;
+        if ($body) {
+            $params = [];
+        } else {
+            $params = $data;
+            $data = [];
+        }
+        $req = $this->serverRequest($method, $url, $this->prefix, $headers, [], $data, "application/json", $params, $authenticated ? $this->userId : "");
+        return $this->h->dispatch($req);
+    }
+
+    protected function reqText(string $method, string $target, string $data, string $type, array $headers = [], bool $authenticated = true): ResponseInterface {
+        $url = $this->prefix.$target;
+        $req = $this->serverRequest($method, $url, $this->prefix, $headers, [], $data, $type, [], $authenticated ? $this->userId : "");
+        return $this->h->dispatch($req);
+    }
+
     public function testSendAuthenticationChallenge(): void {
         $exp = HTTP::respEmpty(401);
         $this->assertMessage($exp, $this->req("GET", "/", "", [], false));
@@ -552,6 +558,38 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest {
         ];
     }
 
+    #[DataProvider("provideNewSubscriptionsWithType")]
+    public function testAddSubscriptionsWithDifferentMediaTypes(string $input, string $type, ResponseInterface $exp): void {
+        \Phake::when(Arsse::$db)->subscriptionAdd->thenReturn(42);
+        \Phake::when(Arsse::$db)->subscriptionPropertiesSet->thenReturn(true);
+        \Phake::when(Arsse::$db)->subscriptionPropertiesGet->thenReturn(self::v(self::$feeds['db'][1]));
+        \Phake::when(Arsse::$db)->editionLatest->thenReturn(4758915);
+        $act = $this->reqText("POST", "/feeds", $input, $type);
+        $this->assertMessage($exp, $act);
+    }
+
+    public static function provideNewSubscriptionsWithType(): iterable {
+        $success = HTTP::respJson(['feeds' => [self::$feeds['rest'][1]], 'newestItemId' => 4758915]);
+        $badType = HTTP::respEmpty(415, ['Accept' => "application/json"]);
+        return [
+            ['{"url":"http://example.org/news.atom","folderId":8}', "application/json",                  $success],
+            ['{"url":"http://example.org/news.atom","folderId":8}', "text/json",                         $success],
+            ['{"url":"http://example.org/news.atom","folderId":8}', "",                                  $success],
+            ['{"url":"http://example.org/news.atom","folderId":8}', "/",                                 $success],
+            ['{"url":"http://example.org/news.atom","folderId":8}', "application/x-www-form-urlencoded", $success],
+            ['{"url":"http://example.org/news.atom","folderId":8}', "application/octet-stream",          $success],
+            ['url=http://example.org/news.atom&folderId=8',         "application/x-www-form-urlencoded", $success],
+            ['url=http://example.org/news.atom&folderId=8',         "",                                  $success],
+            ['{"url":',                                             "application/json",                  HTTP::respEmpty(400)],
+            ['{"url":',                                             "text/json",                         HTTP::respEmpty(400)],
+            ['{"url":',                                             "",                                  HTTP::respEmpty(400)],
+            ['{"url":',                                             "application/x-www-form-urlencoded", HTTP::respEmpty(400)],
+            ['{"url":',                                             "application/octet-stream",          $badType],
+            ['null',                                                "application/json",                  HTTP::respEmpty(400)],
+            ['null',                                                "text/json",                         HTTP::respEmpty(400)],
+        ];
+    }
+
     public function testRemoveASubscription(): void {
         \Phake::when(Arsse::$db)->subscriptionRemove($this->userId, 1)->thenReturn(true)->thenThrow(new ExceptionInput("subjectMissing"));
         $exp = HTTP::respEmpty(204);

tests/cases/REST/NextcloudNews/TestV1_3.php

@@ -17,6 +17,7 @@ use PHPUnit\Framework\Attributes\CoversClass;
 
 #[CoversClass(V1_3::class)]
 class TestV1_3 extends TestV1_2 {
+    protected $prefix = "/index.php/apps/news/api/v1-3";
 
     public function setUp(): void {
         parent::setUp();