From 850d33e658d6d4cc6ea8d3f8774fc7b5ff80fecb Mon Sep 17 00:00:00 2001 From: sam marshall Date: Fri, 22 Feb 2013 17:23:01 +0000 Subject: [PATCH] MDL-38170 SimplePie: Cannot read https feeds through proxy --- lib/filelib.php | 39 ++++++++++++ lib/simplepie/moodle_simplepie.php | 2 +- lib/tests/filelib_test.php | 97 ++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) diff --git a/lib/filelib.php b/lib/filelib.php index 4713d82145b..27bf68c4cd6 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -3417,6 +3417,45 @@ class curl { public function get_errno() { return $this->errno; } + + /** + * When using a proxy, an additional HTTP response code may appear at + * the start of the header. For example, when using https over a proxy + * there may be 'HTTP/1.0 200 Connection Established'. Other codes are + * also possible and some may come with their own headers. + * + * If using the return value containing all headers, this function can be + * called to remove unwanted doubles. + * + * Note that it is not possible to distinguish this situation from valid + * data unless you know the actual response part (below the headers) + * will not be included in this string, or else will not 'look like' HTTP + * headers. As a result it is not safe to call this function for general + * data. + * + * @param string $input Input HTTP response + * @return string HTTP response with additional headers stripped if any + */ + public static function strip_double_headers($input) { + // I have tried to make this regular expression as specific as possible + // to avoid any case where it does weird stuff if you happen to put + // HTTP/1.1 200 at the start of any line in your RSS file. This should + // also make it faster because it can abandon regex processing as soon + // as it hits something that doesn't look like an http header. The + // header definition is taken from RFC 822, except I didn't support + // folding which is never used in practice. + $crlf = "\r\n"; + return preg_replace( + // HTTP version and status code (ignore value of code). + '~^HTTP/1\..*' . $crlf . + // Header name: character between 33 and 126 decimal, except colon. + // Colon. Header value: any character except \r and \n. CRLF. + '(?:[\x21-\x39\x3b-\x7e]+:[^' . $crlf . ']+' . $crlf . ')*' . + // Headers are terminated by another CRLF (blank line). + $crlf . + // Second HTTP status code, this time must be 200. + '(HTTP/1.[01] 200 )~', '$1', $input); + } } /** diff --git a/lib/simplepie/moodle_simplepie.php b/lib/simplepie/moodle_simplepie.php index 54e2d07ee3f..5fe27555ee7 100644 --- a/lib/simplepie/moodle_simplepie.php +++ b/lib/simplepie/moodle_simplepie.php @@ -139,7 +139,7 @@ class moodle_simplepie_file extends SimplePie_File } } - $this->headers = $curl->get($url); + $this->headers = curl::strip_double_headers($curl->get($url)); if ($curl->error) { $this->error = 'cURL Error: '.$curl->error; diff --git a/lib/tests/filelib_test.php b/lib/tests/filelib_test.php index 53bc3178fe0..7365a441311 100644 --- a/lib/tests/filelib_test.php +++ b/lib/tests/filelib_test.php @@ -270,4 +270,101 @@ class filelib_testcase extends advanced_testcase { $this->assertEquals($contenthash, $fileref->get_contenthash()); $this->assertEquals($filecontent, $fileref->get_content()); } + + /** + * Tests the strip_double_headers function in the curl class. + */ + public function test_curl_strip_double_headers() { + // Example from issue tracker. + $mdl30648example = <<... +EOF; + $mdl30648expected = <<... +EOF; + // For HTTP, replace the \n with \r\n. + $mdl30648example = preg_replace("~(?!<\r)\n~", "\r\n", $mdl30648example); + $mdl30648expected = preg_replace("~(?!<\r)\n~", "\r\n", $mdl30648expected); + + // Test stripping works OK. + $this->assertEquals($mdl30648expected, curl::strip_double_headers($mdl30648example)); + // Test it does nothing to the 'plain' data. + $this->assertEquals($mdl30648expected, curl::strip_double_headers($mdl30648expected)); + + // Example from OU proxy. + $httpsexample = << +... +EOF; + $httpsexpected = << +... +EOF; + // For HTTP, replace the \n with \r\n. + $httpsexample = preg_replace("~(?!<\r)\n~", "\r\n", $httpsexample); + $httpsexpected = preg_replace("~(?!<\r)\n~", "\r\n", $httpsexpected); + + // Test stripping works OK. + $this->assertEquals($httpsexpected, curl::strip_double_headers($httpsexample)); + // Test it does nothing to the 'plain' data. + $this->assertEquals($httpsexpected, curl::strip_double_headers($httpsexpected)); + } }