From e6aeab1fc0fdfb28261540187f07eeb087e4ffe2 Mon Sep 17 00:00:00 2001
From: "Eloy Lafuente (stronk7)" <stronk7@moodle.org>
Date: Thu, 10 Feb 2022 10:44:26 +0100
Subject: [PATCH 1/2] MDL-73826 mod_lti: Fix for Windows/PHP8 with empty curl
 responses

Sometimes (detected with Windows, when running @ GHA), both the
response and the error of a curl request to non-existing URL
returns the empty string.

In that case, we cannot call to DOMDocument::loadXML() because the
1st param cannot be empty. So here, whenever that happens, we are
throwing the moodle_exception earlier, instead of waiting for the
XML errors to be processed later.
---
 mod/lti/locallib.php | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php
index 7668e676631..b0e05eba707 100644
--- a/mod/lti/locallib.php
+++ b/mod/lti/locallib.php
@@ -4392,6 +4392,13 @@ function lti_load_cartridge($url, $map, $propertiesmap = array()) {
     $curl = new curl();
     $response = $curl->get($url);
 
+    // Got a completely empty response (real or error), cannot process this with
+    // DOMDocument::loadXML() because it errors with ValueError. So let's throw
+    // the moodle_exception before waiting to examine the errors later.
+    if (trim($response) === '') {
+        throw new moodle_exception('errorreadingfile', '', '', $url);
+    }
+
     // TODO MDL-46023 Replace this code with a call to the new library.
     $origerrors = libxml_use_internal_errors(true);
     $origentity = lti_libxml_disable_entity_loader(true);

From ad1c072d04fa3041fc2a9e732dccd0f17a17dd38 Mon Sep 17 00:00:00 2001
From: "Eloy Lafuente (stronk7)" <stronk7@moodle.org>
Date: Thu, 10 Feb 2022 11:33:33 +0100
Subject: [PATCH 2/2] MDL-73826 phpunit: Allow curl mock responses to handle
 empty strings

Before this commit, is_empty() was being applied before returning
the mock response. But we want to be able to mock the empty response
for some tests, hence moving the condition to null/isset, that is
the value that array_pop() returns where there aren't more elements
in the array.

With that change performed, we can test lti_load_cartridge() with
empty responses, hence adding a new test for that.
---
 lib/filelib.php                 |  3 ++-
 mod/lti/tests/locallib_test.php | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/filelib.php b/lib/filelib.php
index b5f3fd13ce0..60fcea88f7c 100644
--- a/lib/filelib.php
+++ b/lib/filelib.php
@@ -3706,7 +3706,8 @@ class curl {
         $this->reset_request_state_vars();
 
         if ((defined('PHPUNIT_TEST') && PHPUNIT_TEST)) {
-            if ($mockresponse = array_pop(self::$mockresponses)) {
+            $mockresponse = array_pop(self::$mockresponses);
+            if ($mockresponse !== null) {
                 $this->info = [ 'http_code' => 200 ];
                 return $mockresponse;
             }
diff --git a/mod/lti/tests/locallib_test.php b/mod/lti/tests/locallib_test.php
index 37791f3e1ca..02f1e9c1f84 100644
--- a/mod/lti/tests/locallib_test.php
+++ b/mod/lti/tests/locallib_test.php
@@ -1841,6 +1841,20 @@ MwIDAQAB
         ];
     }
 
+    /**
+     * Verify that empty curl responses lead to the proper moodle_exception, not to XML ValueError.
+     *
+     * @covers ::lti_load_cartridge()
+     */
+    public function test_empty_reponse_lti_load_cartridge() {
+        // Mock the curl response to empty string, this is hardly
+        // reproducible in real life (only Windows + GHA).
+        \curl::mock_response('');
+
+        $this->expectException(\moodle_exception::class);
+        lti_load_cartridge('http://example.com/mocked/empty/response', []);
+    }
+
     /**
      * Create an LTI Tool.
      *