diff --git a/lib/REST/NextcloudNews/V1_2.php b/lib/REST/NextcloudNews/V1_2.php
index aec1f542..0759692a 100644
--- a/lib/REST/NextcloudNews/V1_2.php
+++ b/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"
- if (!HTTP::matchType($req, [self::ACCEPTED_TYPE])) {
- return HTTP::respEmpty(415, ['Accept' => self::ACCEPTED_TYPE]);
- }
- $data = @json_decode($data, true);
- if (json_last_error() !== \JSON_ERROR_NONE) {
- // if the body could not be parsed as JSON, return "400 Bad Request"
+ // Officially the body, if any, should be JSON. In practice the
+ // 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]);
+ }
+ } catch (\JsonException $e) {
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)
diff --git a/tests/cases/REST/NextcloudNews/TestV1_2.php b/tests/cases/REST/NextcloudNews/TestV1_2.php
index 9112fe9e..b3a6b70e 100644
--- a/tests/cases/REST/NextcloudNews/TestV1_2.php
+++ b/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);
diff --git a/tests/cases/REST/NextcloudNews/TestV1_3.php b/tests/cases/REST/NextcloudNews/TestV1_3.php
index bf25bfcd..62abb927 100644
--- a/tests/cases/REST/NextcloudNews/TestV1_3.php
+++ b/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();