Merge branch '2.6' into 2

MNT Use gha-dispatch-ci

.github/workflows/ci.yml

@@ -0,0 +1,11 @@
+name: CI
+
+on:
+  push:
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  ci:
+    name: CI
+    uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1

.github/workflows/dispatch-ci.yml

@@ -0,0 +1,16 @@
+name: Dispatch CI
+
+on:
+  # At 1:10 PM UTC, only on Friday and Saturday
+  schedule:
+    - cron: '10 13 * * 5,6'
+
+jobs:
+  dispatch-ci:
+    name: Dispatch CI
+    # Only run cron on the silverstripe account
+    if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
+    runs-on: ubuntu-latest
+    steps:
+      - name: Dispatch CI
+        uses: silverstripe/gha-dispatch-ci@v1

.github/workflows/keepalive.yml

@@ -0,0 +1,17 @@
+name: Keepalive
+
+on:
+  workflow_dispatch:
+  # The 4th of every month at 10:50am UTC
+  schedule:
+    - cron: '50 10 4 * *'
+
+jobs:
+  keepalive:
+    name: Keepalive
+    # Only run cron on the silverstripe account
+    if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
+    runs-on: ubuntu-latest
+    steps:
+      - name: Keepalive
+        uses: silverstripe/gha-keepalive@v1

.scrutinizer.yml

@@ -1,15 +0,0 @@
-inherit: true
-
-build:
-  nodes:
-    analysis:
-      tests:
-        override: [php-scrutinizer-run]
-
-checks:
-  php:
-    code_rating: true
-    duplication: true
-
-filter:
-  paths: [src/*, tests/*]

.travis.yml

@@ -1,4 +0,0 @@
-version: ~> 1.0
-
-import:
-  - silverstripe/silverstripe-travis-shared:config/provision/standard-jobs-range.yml

README.md

@@ -1,9 +1,7 @@
-# SilverStripe RestfulServer Module
+# Silverstripe RestfulServer Module
 
-[![Build Status](https://api.travis-ci.com/silverstripe/silverstripe-restfulserver.svg?branch=2)](https://travis-ci.com/silverstripe/silverstripe-restfulserver)
+[![CI](https://github.com/silverstripe/silverstripe-restfulserver/actions/workflows/ci.yml/badge.svg)](https://github.com/silverstripe/silverstripe-restfulserver/actions/workflows/ci.yml)
-[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/silverstripe/silverstripe-restfulserver/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/silverstripe/silverstripe-restfulserver/?branch=master)
+[![Silverstripe supported module](https://img.shields.io/badge/silverstripe-supported-0071C4.svg)](https://www.silverstripe.org/software/addons/silverstripe-commercially-supported-module-list/)
-[![codecov](https://codecov.io/gh/silverstripe/silverstripe-restfulserver/branch/master/graph/badge.svg)](https://codecov.io/gh/silverstripe/silverstripe-restfulserver)
-[![SilverStripe supported module](https://img.shields.io/badge/silverstripe-supported-0071C4.svg)](https://www.silverstripe.org/software/addons/silverstripe-commercially-supported-module-list/)
 
 ## Overview
 
@@ -14,9 +12,9 @@ applications.
 
 ## Requirements
 
-* SilverStripe 4.0 or higher
+* Silverstripe 4.0 or higher
 
-For a SilverStripe 3.x compatible version of this module, please see the [1.0 branch, or 1.x release line](https://github.com/silverstripe/silverstripe-restfulserver/tree/1.0#readme).
+For a Silverstripe 3.x compatible version of this module, please see the [1.0 branch, or 1.x release line](https://github.com/silverstripe/silverstripe-restfulserver/tree/1.0#readme).
 
 ## Configuration
 

composer.json

@@ -19,7 +19,7 @@
         }
     ],
     "require": {
-        "php": "^7.3 || ^8.0",
+        "php": "^7.4 || ^8.0",
         "silverstripe/framework": "^4.10"
     },
     "require-dev": {

phpunit.xml.dist

@@ -1,3 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
 <phpunit bootstrap="vendor/silverstripe/framework/tests/bootstrap.php" colors="true">
     <testsuites>
         <testsuite name="Default">

src/DataFormatter.php

@@ -103,7 +103,7 @@ abstract class DataFormatter
      */
     protected function sanitiseClassName($className)
     {
-        return str_replace('\\', '-', $className);
+        return str_replace('\\', '-', $className ?? '');
     }
 
     /**
@@ -123,7 +123,7 @@ abstract class DataFormatter
         arsort($sortedClasses);
         foreach ($sortedClasses as $className => $priority) {
             $formatter = new $className();
-            if (in_array($extension, $formatter->supportedExtensions())) {
+            if (in_array($extension, $formatter->supportedExtensions() ?? [])) {
                 return $formatter;
             }
         }
@@ -163,7 +163,7 @@ abstract class DataFormatter
         arsort($sortedClasses);
         foreach ($sortedClasses as $className => $priority) {
             $formatter = new $className();
-            if (in_array($mimeType, $formatter->supportedMimeTypes())) {
+            if (in_array($mimeType, $formatter->supportedMimeTypes() ?? [])) {
                 return $formatter;
             }
         }
@@ -329,7 +329,10 @@ abstract class DataFormatter
         $dbFields = array_merge($dbFields, ['ID' => 'Int']);
 
         if (is_array($this->removeFields)) {
-            $dbFields = array_diff_key($dbFields, array_combine($this->removeFields, $this->removeFields));
+            $dbFields = array_diff_key(
+                $dbFields ?? [],
+                array_combine($this->removeFields ?? [], $this->removeFields ?? [])
+            );
         }
 
         return $dbFields;
@@ -418,7 +421,7 @@ abstract class DataFormatter
     public function getFieldAlias($className, $field)
     {
         $apiMapping = $this->getApiMapping($className);
-        $apiMapping = array_flip($apiMapping);
+        $apiMapping = array_flip($apiMapping ?? []);
         return $this->getMappedKey($apiMapping, $field);
     }
 
@@ -448,7 +451,7 @@ abstract class DataFormatter
     protected function getMappedKey($map, $key)
     {
         if (is_array($map)) {
-            if (array_key_exists($key, $map)) {
+            if (array_key_exists($key, $map ?? [])) {
                 return $map[$key];
             } else {
                 return $key;

src/DataFormatter/FormEncodedDataFormatter.php

@@ -35,7 +35,7 @@ class FormEncodedDataFormatter extends XMLDataFormatter
     public function convertStringToArray($strData)
     {
         $postArray = array();
-        parse_str($strData, $postArray);
+        parse_str($strData ?? '', $postArray);
         return $postArray;
         //TODO: It would be nice to implement this function in Convert.php
         //return Convert::querystr2array($strData);

src/DataFormatter/JSONDataFormatter.php

@@ -86,7 +86,7 @@ class JSONDataFormatter extends DataFormatter
 
         foreach ($this->getFieldsForObj($obj) as $fieldName => $fieldType) {
             // Field filtering
-            if ($fields && !in_array($fieldName, $fields)) {
+            if ($fields && !in_array($fieldName, $fields ?? [])) {
                 continue;
             }
 
@@ -97,15 +97,15 @@ class JSONDataFormatter extends DataFormatter
 
         if ($this->relationDepth > 0) {
             foreach ($obj->hasOne() as $relName => $relClass) {
-                if (!singleton($relClass)->stat('api_access')) {
+                if (!$relClass::config()->get('api_access')) {
                     continue;
                 }
 
                 // Field filtering
-                if ($fields && !in_array($relName, $fields)) {
+                if ($fields && !in_array($relName, $fields ?? [])) {
                     continue;
                 }
-                if ($this->customRelations && !in_array($relName, $this->customRelations)) {
+                if ($this->customRelations && !in_array($relName, $this->customRelations ?? [])) {
                     continue;
                 }
                 if ($obj->$relName() && (!$obj->$relName()->exists() || !$obj->$relName()->canView())) {
@@ -129,18 +129,18 @@ class JSONDataFormatter extends DataFormatter
                 $relClass = RestfulServer::parseRelationClass($relClass);
 
                 //remove dot notation from relation names
-                $parts = explode('.', $relClass);
+                $parts = explode('.', $relClass ?? '');
                 $relClass = array_shift($parts);
 
-                if (!singleton($relClass)->stat('api_access')) {
+                if (!$relClass::config()->get('api_access')) {
                     continue;
                 }
 
                 // Field filtering
-                if ($fields && !in_array($relName, $fields)) {
+                if ($fields && !in_array($relName, $fields ?? [])) {
                     continue;
                 }
-                if ($this->customRelations && !in_array($relName, $this->customRelations)) {
+                if ($this->customRelations && !in_array($relName, $this->customRelations ?? [])) {
                     continue;
                 }
 
@@ -195,7 +195,7 @@ class JSONDataFormatter extends DataFormatter
      */
     public function convertStringToArray($strData)
     {
-        return json_decode($strData, true);
+        return json_decode($strData ?? '', true);
     }
 
     public static function cast(FieldType\DBField $dbfield)

src/DataFormatter/XMLDataFormatter.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\RestfulServer\DataFormatter;
 
+use SimpleXMLElement;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\Convert;
 use SilverStripe\Dev\Debug;
@@ -11,6 +12,7 @@ use SilverStripe\ORM\DataObjectInterface;
 use SilverStripe\Control\Director;
 use SilverStripe\ORM\SS_List;
 use SilverStripe\RestfulServer\RestfulServer;
+use InvalidArgumentException;
 
 /**
  * Formats a DataObject's member fields into an XML string
@@ -121,7 +123,7 @@ class XMLDataFormatter extends DataFormatter
         $xml = "<$className href=\"$objHref.xml\">\n";
         foreach ($this->getFieldsForObj($obj) as $fieldName => $fieldType) {
             // Field filtering
-            if ($fields && !in_array($fieldName, $fields)) {
+            if ($fields && !in_array($fieldName, $fields ?? [])) {
                 continue;
             }
             $fieldValue = $obj->obj($fieldName)->forTemplate();
@@ -134,7 +136,7 @@ class XMLDataFormatter extends DataFormatter
             } else {
                 if ('HTMLText' == $fieldType) {
                     // Escape HTML values using CDATA
-                    $fieldValue = sprintf('<![CDATA[%s]]>', str_replace(']]>', ']]]]><![CDATA[>', $fieldValue));
+                    $fieldValue = sprintf('<![CDATA[%s]]>', str_replace(']]>', ']]]]><![CDATA[>', $fieldValue ?? ''));
                 } else {
                     $fieldValue = Convert::raw2xml($fieldValue);
                 }
@@ -145,15 +147,15 @@ class XMLDataFormatter extends DataFormatter
 
         if ($this->relationDepth > 0) {
             foreach ($obj->hasOne() as $relName => $relClass) {
-                if (!singleton($relClass)->stat('api_access')) {
+                if (!singleton($relClass)::config()->get('api_access')) {
                     continue;
                 }
 
                 // Field filtering
-                if ($fields && !in_array($relName, $fields)) {
+                if ($fields && !in_array($relName, $fields ?? [])) {
                     continue;
                 }
-                if ($this->customRelations && !in_array($relName, $this->customRelations)) {
+                if ($this->customRelations && !in_array($relName, $this->customRelations ?? [])) {
                     continue;
                 }
 
@@ -169,19 +171,19 @@ class XMLDataFormatter extends DataFormatter
 
             foreach ($obj->hasMany() as $relName => $relClass) {
                 //remove dot notation from relation names
-                $parts = explode('.', $relClass);
+                $parts = explode('.', $relClass ?? '');
                 $relClass = array_shift($parts);
-                if (!singleton($relClass)->stat('api_access')) {
+                if (!singleton($relClass)::config()->get('api_access')) {
                     continue;
                 }
                 // backslashes in FQCNs kills both URIs and XML
                 $relClass = $this->sanitiseClassName($relClass);
 
                 // Field filtering
-                if ($fields && !in_array($relName, $fields)) {
+                if ($fields && !in_array($relName, $fields ?? [])) {
                     continue;
                 }
-                if ($this->customRelations && !in_array($relName, $this->customRelations)) {
+                if ($this->customRelations && !in_array($relName, $this->customRelations ?? [])) {
                     continue;
                 }
 
@@ -200,19 +202,19 @@ class XMLDataFormatter extends DataFormatter
                 $relClass = RestfulServer::parseRelationClass($relClass);
 
                 //remove dot notation from relation names
-                $parts = explode('.', $relClass);
+                $parts = explode('.', $relClass ?? '');
                 $relClass = array_shift($parts);
-                if (!singleton($relClass)->stat('api_access')) {
+                if (!singleton($relClass)::config()->get('api_access')) {
                     continue;
                 }
                 // backslashes in FQCNs kills both URIs and XML
                 $relClass = $this->sanitiseClassName($relClass);
 
                 // Field filtering
-                if ($fields && !in_array($relName, $fields)) {
+                if ($fields && !in_array($relName, $fields ?? [])) {
                     continue;
                 }
-                if ($this->customRelations && !in_array($relName, $this->customRelations)) {
+                if ($this->customRelations && !in_array($relName, $this->customRelations ?? [])) {
                     continue;
                 }
 
@@ -261,6 +263,79 @@ class XMLDataFormatter extends DataFormatter
      */
     public function convertStringToArray($strData)
     {
-        return Convert::xml2array($strData);
+        return self::xml2array($strData);
+    }
+
+    /**
+     * This was copied from Convert::xml2array() which is deprecated/removed
+     *
+     * Converts an XML string to a PHP array
+     * See http://phpsecurity.readthedocs.org/en/latest/Injection-Attacks.html#xml-external-entity-injection
+     *
+     * @uses recursiveXMLToArray()
+     * @param string $val
+     * @param boolean $disableDoctypes Disables the use of DOCTYPE, and will trigger an error if encountered.
+     * false by default.
+     * @param boolean $disableExternals Does nothing because xml entities are removed
+     * @return array
+     * @throws Exception
+     */
+    private static function xml2array($val, $disableDoctypes = false, $disableExternals = false)
+    {
+        // Check doctype
+        if ($disableDoctypes && strpos($val ?? '', '<!DOCTYPE') !== false) {
+            throw new InvalidArgumentException('XML Doctype parsing disabled');
+        }
+
+        // CVE-2021-41559 Ensure entities are removed due to their inherent security risk via
+        // XXE attacks and quadratic blowup attacks, and also lack of consistent support
+        $val = preg_replace('/(?s)<!ENTITY.*?>/', '', $val ?? '');
+
+        // If there's still an <!ENTITY> present, then it would be the result of a maliciously
+        // crafted XML document e.g. <!ENTITY><!<!ENTITY>ENTITY ext SYSTEM "http://evil.com">
+        if (strpos($val ?? '', '<!ENTITY') !== false) {
+            throw new InvalidArgumentException('Malicious XML entity detected');
+        }
+
+        // This will throw an exception if the XML contains references to any internal entities
+        // that were defined in an <!ENTITY /> before it was removed
+        $xml = new SimpleXMLElement($val ?? '');
+        return self::recursiveXMLToArray($xml);
+    }
+
+    /**
+     * @param SimpleXMLElement $xml
+     *
+     * @return mixed
+     */
+    private static function recursiveXMLToArray($xml)
+    {
+        $x = null;
+        if ($xml instanceof SimpleXMLElement) {
+            $attributes = $xml->attributes();
+            foreach ($attributes as $k => $v) {
+                if ($v) {
+                    $a[$k] = (string) $v;
+                }
+            }
+            $x = $xml;
+            $xml = get_object_vars($xml);
+        }
+        if (is_array($xml)) {
+            if (count($xml ?? []) === 0) {
+                return (string)$x;
+            } // for CDATA
+            $r = [];
+            foreach ($xml as $key => $value) {
+                $r[$key] = self::recursiveXMLToArray($value);
+            }
+            // Attributes
+            if (isset($a)) {
+                $r['@'] = $a;
+            }
+            return $r;
+        }
+
+        return (string) $xml;
     }
 }

src/RestfulServer.php

@@ -138,7 +138,7 @@ class RestfulServer extends Controller
      */
     protected function sanitiseClassName($className)
     {
-        return str_replace('\\', '-', $className);
+        return str_replace('\\', '-', $className ?? '');
     }
 
     /**
@@ -150,7 +150,7 @@ class RestfulServer extends Controller
      */
     protected function unsanitiseClassName($className)
     {
-        return str_replace('-', '\\', $className);
+        return str_replace('-', '\\', $className ?? '');
     }
 
     /**
@@ -163,13 +163,13 @@ class RestfulServer extends Controller
     {
         // detect many many through syntax
         if (is_array($class)
-            && array_key_exists('through', $class)
+            && array_key_exists('through', $class ?? [])
-            && array_key_exists('to', $class)
+            && array_key_exists('to', $class ?? [])
         ) {
             $toRelation = $class['to'];
 
             $hasOne = Config::inst()->get($class['through'], 'has_one');
-            if (empty($hasOne) || !is_array($hasOne) || !array_key_exists($toRelation, $hasOne)) {
+            if (empty($hasOne) || !is_array($hasOne) || !array_key_exists($toRelation, $hasOne ?? [])) {
                 return $class;
             }
 
@@ -190,14 +190,14 @@ class RestfulServer extends Controller
         $relation = $request->param('Relation') ?: null;
 
         // Check input formats
-        if (!class_exists($className)) {
+        if (!class_exists($className ?? '')) {
             return $this->notFound();
         }
         if ($id && !is_numeric($id)) {
             return $this->notFound();
         }
         if ($relation
-            && !preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $relation)
+            && !preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*$/', $relation ?? '')
         ) {
             return $this->notFound();
         }
@@ -289,6 +289,10 @@ class RestfulServer extends Controller
             'limit' => (int) $this->request->getVar('limit'),
         ];
 
+        if ($limit['limit'] === 0) {
+            $limit = null;
+        }
+
         $params = $this->request->getVars();
 
         $responseFormatter = $this->getResponseDataFormatter($className);
@@ -326,7 +330,7 @@ class RestfulServer extends Controller
         $this->getResponse()->addHeader('Content-Type', $responseFormatter->getOutputContentType());
 
         $rawFields = $this->request->getVar('fields');
-        $realFields = $responseFormatter->getRealFields($className, explode(',', $rawFields));
+        $realFields = $responseFormatter->getRealFields($className, explode(',', $rawFields ?? ''));
         $fields = $rawFields ? $realFields : null;
 
         if ($obj instanceof SS_List) {
@@ -391,7 +395,7 @@ class RestfulServer extends Controller
     {
         $extension = $this->request->getExtension();
         $contentTypeWithEncoding = $this->request->getHeader('Content-Type');
-        preg_match('/([^;]*)/', $contentTypeWithEncoding, $contentTypeMatches);
+        preg_match('/([^;]*)/', $contentTypeWithEncoding ?? '', $contentTypeMatches);
         $contentType = $contentTypeMatches[0];
         $accept = $this->request->getHeader('Accept');
         $mimetypes = $this->request->getAcceptMimetypes();
@@ -402,7 +406,7 @@ class RestfulServer extends Controller
         // get formatter
         if (!empty($extension)) {
             $formatter = DataFormatter::for_extension($extension);
-        } elseif ($includeAcceptHeader && !empty($accept) && strpos($accept, '*/*') === false) {
+        } elseif ($includeAcceptHeader && !empty($accept) && strpos($accept ?? '', '*/*') === false) {
             $formatter = DataFormatter::for_mimetypes($mimetypes);
             if (!$formatter) {
                 $formatter = DataFormatter::for_extension($this->config()->default_extension);
@@ -419,11 +423,11 @@ class RestfulServer extends Controller
 
         // set custom fields
         if ($customAddFields = $this->request->getVar('add_fields')) {
-            $customAddFields = $formatter->getRealFields($className, explode(',', $customAddFields));
+            $customAddFields = $formatter->getRealFields($className, explode(',', $customAddFields ?? ''));
             $formatter->setCustomAddFields($customAddFields);
         }
         if ($customFields = $this->request->getVar('fields')) {
-            $customFields = $formatter->getRealFields($className, explode(',', $customFields));
+            $customFields = $formatter->getRealFields($className, explode(',', $customFields ?? ''));
             $formatter->setCustomFields($customFields);
         }
         $formatter->setCustomRelations($this->getAllowedRelations($className));
@@ -537,7 +541,7 @@ class RestfulServer extends Controller
         // or else we'll use the default (XML)
         $types = $responseFormatter->supportedExtensions();
         $type = '';
-        if (count($types)) {
+        if (count($types ?? [])) {
             $type = ".{$types[0]}";
         }
 
@@ -581,7 +585,7 @@ class RestfulServer extends Controller
             }
 
             if (!Config::inst()->get($className, 'allowed_actions') ||
-                !in_array($relation, Config::inst()->get($className, 'allowed_actions'))) {
+                !in_array($relation, Config::inst()->get($className, 'allowed_actions') ?? [])) {
                 return $this->permissionFailure();
             }
 
@@ -622,7 +626,7 @@ class RestfulServer extends Controller
         // or else we'll use the default (XML)
         $types = $responseFormatter->supportedExtensions();
         $type = '';
-        if (count($types)) {
+        if (count($types ?? [])) {
             $type = ".{$types[0]}";
         }
 
@@ -675,11 +679,11 @@ class RestfulServer extends Controller
         }
 
         // @todo Disallow editing of certain keys in database
-        $data = array_diff_key($data, ['ID', 'Created']);
+        $data = array_diff_key($data ?? [], ['ID', 'Created']);
 
         $apiAccess = singleton($className)->config()->api_access;
         if (is_array($apiAccess) && isset($apiAccess['edit'])) {
-            $data = array_intersect_key($data, array_combine($apiAccess['edit'], $apiAccess['edit']));
+            $data = array_intersect_key($data ?? [], array_combine($apiAccess['edit'] ?? [], $apiAccess['edit'] ?? []));
         }
 
         $obj->update($data);
@@ -878,7 +882,7 @@ class RestfulServer extends Controller
                 $relClass = static::parseRelationClass($relClass);
 
                 //remove dot notation from relation names
-                $parts = explode('.', $relClass);
+                $parts = explode('.', $relClass ?? '');
                 $relClass = array_shift($parts);
                 if (Config::inst()->get($relClass, 'api_access')) {
                     $allowedRelations[] = $relName;

tests/unit/RestfulServerTest.php

@@ -20,6 +20,7 @@ use SilverStripe\Dev\SapphireTest;
 use SilverStripe\RestfulServer\DataFormatter\JSONDataFormatter;
 use Page;
 use SilverStripe\Core\Config\Config;
+use SilverStripe\RestfulServer\DataFormatter\XMLDataFormatter;
 
 /**
  *
@@ -46,7 +47,7 @@ class RestfulServerTest extends SapphireTest
 
     protected function urlSafeClassname($classname)
     {
-        return str_replace('\\', '-', $classname);
+        return str_replace('\\', '-', $classname ?? '');
     }
 
     protected function setUp(): void
@@ -126,7 +127,8 @@ class RestfulServerTest extends SapphireTest
         $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthorRating::class);
         $url = "{$this->baseURI}/api/v1/$urlSafeClassname/" . $rating1->ID;
         $response = Director::test($url, null, null, 'GET');
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals(3, $responseArr['rate']);
     }
 
@@ -165,10 +167,11 @@ class RestfulServerTest extends SapphireTest
         $response = Director::test($url, null, null, 'GET');
         $this->assertEquals(200, $response->getStatusCode());
 
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $xmlTagSafeClassName = $this->urlSafeClassname(RestfulServerTestAuthorRating::class);
         $ratingsArr = $responseArr['Ratings'][$xmlTagSafeClassName];
-        $this->assertEquals(2, count($ratingsArr));
+        $this->assertEquals(2, count($ratingsArr ?? []));
         $ratingIDs = array(
             (int)$ratingsArr[0]['@attributes']['id'],
             (int)$ratingsArr[1]['@attributes']['id']
@@ -192,11 +195,12 @@ class RestfulServerTest extends SapphireTest
         $response = Director::test($url, null, null, 'GET');
         $this->assertEquals(200, $response->getStatusCode());
 
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $xmlTagSafeClassName = $this->urlSafeClassname(RestfulServerTestAuthorRating::class);
 
-        $this->assertTrue(array_key_exists('Ratings', $responseArr));
+        $this->assertTrue(array_key_exists('Ratings', $responseArr ?? []));
-        $this->assertFalse(array_key_exists('stars', $responseArr));
+        $this->assertFalse(array_key_exists('stars', $responseArr ?? []));
     }
 
     public function testGETManyManyRelationshipsXML()
@@ -210,11 +214,13 @@ class RestfulServerTest extends SapphireTest
         $url = "{$this->baseURI}/api/v1/$urlSafeClassname/" . $author4->ID . '/RelatedAuthors';
         $response = Director::test($url, null, null, 'GET');
         $this->assertEquals(200, $response->getStatusCode());
-        $arr = Convert::xml2array($response->getBody());
+        
+        $formatter = new XMLDataFormatter();
+        $arr = $formatter->convertStringToArray($response->getBody());
         $xmlSafeClassName = $this->urlSafeClassname(RestfulServerTestAuthor::class);
         $authorsArr = $arr[$xmlSafeClassName];
 
-        $this->assertEquals(2, count($authorsArr));
+        $this->assertEquals(2, count($authorsArr ?? []));
         $ratingIDs = array(
             (int)$authorsArr[0]['ID'],
             (int)$authorsArr[1]['ID']
@@ -239,7 +245,8 @@ class RestfulServerTest extends SapphireTest
         $response = Director::test($url, null, null, 'PUT', $body, $headers);
         $this->assertEquals(202, $response->getStatusCode()); // Accepted
         // Assumption: XML is default output
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals($comment1->ID, $responseArr['ID']);
         $this->assertEquals('updated', $responseArr['Comment']);
         $this->assertEquals('Updated Comment', $responseArr['Name']);
@@ -264,7 +271,8 @@ class RestfulServerTest extends SapphireTest
         $response = Director::test($url, null, null, 'POST', $body, $headers);
         $this->assertEquals(201, $response->getStatusCode()); // Created
         // Assumption: XML is default output
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertTrue($responseArr['ID'] > 0);
         $this->assertNotEquals($responseArr['ID'], $comment1->ID);
         $this->assertEquals('created', $responseArr['Comment']);
@@ -307,7 +315,7 @@ class RestfulServerTest extends SapphireTest
             'Accept' => 'application/json'
         ));
         $this->assertEquals(202, $response->getStatusCode()); // Accepted
-        $obj = json_decode($response->getBody());
+        $obj = json_decode($response->getBody() ?? '');
         $this->assertEquals($comment1->ID, $obj->ID);
         $this->assertEquals('updated', $obj->Comment);
 
@@ -318,7 +326,7 @@ class RestfulServerTest extends SapphireTest
         $response = Director::test($url, null, null, 'PUT', $body);
         $this->assertEquals(202, $response->getStatusCode()); // Accepted
         $this->assertEquals($url, $response->getHeader('Location'));
-        $obj = json_decode($response->getBody());
+        $obj = json_decode($response->getBody() ?? '');
         $this->assertEquals($comment1->ID, $obj->ID);
         $this->assertEquals('updated', $obj->Comment);
 
@@ -339,7 +347,8 @@ class RestfulServerTest extends SapphireTest
         $body = '<RestfulServerTestComment><Comment>updated</Comment></RestfulServerTestComment>';
         $response = Director::test($url, null, null, 'PUT', $body, array('Content-Type'=>'text/xml'));
         $this->assertEquals(202, $response->getStatusCode()); // Accepted
-        $obj = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $obj = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals($comment1->ID, $obj['ID']);
         $this->assertEquals('updated', $obj['Comment']);
 
@@ -350,7 +359,8 @@ class RestfulServerTest extends SapphireTest
         $response = Director::test($url, null, null, 'PUT', $body);
         $this->assertEquals(202, $response->getStatusCode()); // Accepted
         $this->assertEquals($url, $response->getHeader('Location'));
-        $obj = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $obj = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals($comment1->ID, $obj['ID']);
         $this->assertEquals('updated', $obj['Comment']);
 
@@ -368,7 +378,7 @@ class RestfulServerTest extends SapphireTest
         $headers = array('Accept' => 'application/json');
         $response = Director::test($url, null, null, 'GET', null, $headers);
         $this->assertEquals(200, $response->getStatusCode()); // Success
-        $obj = json_decode($response->getBody());
+        $obj = json_decode($response->getBody() ?? '');
         $this->assertEquals($comment1->ID, $obj->ID);
         $this->assertEquals('application/json', $response->getHeader('Content-Type'));
     }
@@ -553,7 +563,8 @@ class RestfulServerTest extends SapphireTest
         );
         $response = Director::test($url, $data, null, 'PUT');
         // Assumption: XML is default output
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals(42, $responseArr['Rating']);
         $this->assertNotEquals('haxx0red', $responseArr['WriteProtectedField']);
     }
@@ -570,7 +581,8 @@ class RestfulServerTest extends SapphireTest
         );
         $response = Director::test($url, $data, null, 'PUT');
         // Assumption: XML is default output
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         // should output with aliased name
         $this->assertEquals(42, $responseArr['rate']);
     }
@@ -631,7 +643,8 @@ class RestfulServerTest extends SapphireTest
         $url = "{$this->baseURI}/api/v1/{$urlSafeClassname}?sort=FirstName&dir=DESC&fields=FirstName";
 
         $response = Director::test($url);
-        $results = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $results = $formatter->convertStringToArray($response->getBody());
 
         $this->assertSame('Author 4', $results[$urlSafeClassname][0]['FirstName']);
         $this->assertSame('Author 3', $results[$urlSafeClassname][1]['FirstName']);
@@ -645,7 +658,8 @@ class RestfulServerTest extends SapphireTest
         $url = "{$this->baseURI}/api/v1/{$urlSafeClassname}?sort=FirstName&dir=ASC&fields=FirstName";
 
         $response = Director::test($url);
-        $results = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $results = $formatter->convertStringToArray($response->getBody());
 
         $this->assertSame('Author 1', $results[$urlSafeClassname][0]['FirstName']);
         $this->assertSame('Author 2', $results[$urlSafeClassname][1]['FirstName']);
@@ -660,7 +674,8 @@ class RestfulServerTest extends SapphireTest
 
         $response = Director::test($url);
 
-        $results = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $results = $formatter->convertStringToArray($response->getBody());
 
         $this->assertSame('Author 1', $results[$urlSafeClassname][0]['FirstName']);
         $this->assertSame('Author 2', $results[$urlSafeClassname][1]['FirstName']);
@@ -678,7 +693,8 @@ class RestfulServerTest extends SapphireTest
         ];
         $response = Director::test($url, $data, null, 'POST');
         // Assumption: XML is default output
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals(42, $responseArr['Rating']);
         $this->assertNotEquals('haxx0red', $responseArr['WriteProtectedField']);
     }
@@ -692,7 +708,8 @@ class RestfulServerTest extends SapphireTest
             'rate' => '42',
         ];
         $response = Director::test($url, $data, null, 'POST');
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals(42, $responseArr['rate']);
     }
 
@@ -710,7 +727,7 @@ class RestfulServerTest extends SapphireTest
         $response = Director::test($url, null, null, 'GET');
         $this->assertEquals(200, $response->getStatusCode());
         $this->assertStringNotContainsString('Unspeakable', $response->getBody());
-        $responseArray = json_decode($response->getBody(), true);
+        $responseArray = json_decode($response->getBody() ?? '', true);
         $this->assertSame(0, $responseArray['totalSize']);
 
         // With authentication
@@ -722,7 +739,8 @@ class RestfulServerTest extends SapphireTest
         $this->assertEquals(200, $response->getStatusCode());
         $this->assertStringContainsString('Unspeakable', $response->getBody());
         // Assumption: default formatter is XML
-        $responseArray = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArray = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals(1, $responseArray['@attributes']['totalSize']);
         unset($_SERVER['PHP_AUTH_USER']);
         unset($_SERVER['PHP_AUTH_PW']);
@@ -737,7 +755,8 @@ class RestfulServerTest extends SapphireTest
         ];
         $response = Director::test($url, $data, null, 'POST');
         // Assumption: XML is default output
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals('SilverStripe\\ORM\\ValidationException', $responseArr['type']);
     }
 
@@ -750,7 +769,8 @@ class RestfulServerTest extends SapphireTest
         ];
         $response = Director::test($url, $data, null, 'POST');
         // Assumption: XML is default output
-        $responseArr = Convert::xml2array($response->getBody());
+        $formatter = new XMLDataFormatter();
+        $responseArr = $formatter->convertStringToArray($response->getBody());
         $this->assertEquals(\Exception::class, $responseArr['type']);
     }
 

tests/unit/Stubs/RestfulServerTestValidationFailure.php

@@ -30,11 +30,11 @@ class RestfulServerTestValidationFailure extends DataObject implements TestOnly
     {
         $result = parent::validate();
 
-        if (strlen($this->Content) === 0) {
+        if (strlen($this->Content ?? '') === 0) {
             $result->addFieldError('Content', 'Content required');
         }
 
-        if (strlen($this->Title) === 0) {
+        if (strlen($this->Title ?? '') === 0) {
             $result->addFieldError('Title', 'Title required');
         }
 

tests/unit/XMLDataFormatterTest.php

@@ -0,0 +1,77 @@
+<?php
+
+namespace SilverStripe\RestfulServer\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\RestfulServer\DataFormatter\XMLDataFormatter;
+use Exception;
+
+class XMLDataFormatterTest extends SapphireTest
+{
+    /**
+     * Tests {@link Convert::xml2array()}
+     */
+    public function testConvertStringToArray()
+    {
+        $inputXML = <<<XML
+<?xml version="1.0"?>
+<!DOCTYPE results [
+  <!ENTITY long "SOME_SUPER_LONG_STRING">
+]>
+<results>
+    <result>My para</result>
+    <result>Ampersand &amp; is retained and not double encoded</result>
+</results>
+XML
+        ;
+        $expected = [
+            'result' => [
+                'My para',
+                'Ampersand & is retained and not double encoded'
+            ]
+        ];
+        $formatter = new XMLDataFormatter();
+        $actual = $formatter->convertStringToArray($inputXML);
+        $this->assertEquals($expected, $actual);
+    }
+
+    /**
+     * Tests {@link Convert::xml2array()} if an exception the contains a reference to a removed <!ENTITY />
+     */
+    public function testConvertStringToArrayEntityException()
+    {
+        $inputXML = <<<XML
+        <?xml version="1.0"?>
+        <!DOCTYPE results [
+            <!ENTITY long "SOME_SUPER_LONG_STRING">
+        ]>
+        <results>
+            <result>Now include &long; lots of times to expand the in-memory size of this XML structure</result>
+            <result>&long;&long;&long;</result>
+        </results>
+        XML;
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('String could not be parsed as XML');
+        $formatter = new XMLDataFormatter();
+        $formatter->convertStringToArray($inputXML);
+    }
+
+    /**
+     * Tests {@link Convert::xml2array()} if an exception the contains a reference to a multiple removed <!ENTITY />
+     */
+    public function testConvertStringToArrayMultipleEntitiesException()
+    {
+        $inputXML = <<<XML
+        <?xml version="1.0"?>
+        <!DOCTYPE results [<!ENTITY long "SOME_SUPER_LONG_STRING"><!ENTITY short "SHORT_STRING">]>
+        <results>
+            <result>Now include &long; and &short; lots of times</result>
+            <result>&long;&long;&long;&short;&short;&short;</result>
+        </results>
+        XML;
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('String could not be parsed as XML');
+        $formatter = new XMLDataFormatter();
+        $formatter->convertStringToArray($inputXML);
+    }
+}