Skip to content

Commit

Permalink
MDL-73588 curl: Fix expected CURLOPT_FILE behavior
Browse files Browse the repository at this point in the history
After cbf9dfb the CURLOPT_FILE no longer behaves as expected. All
redirect responses are appended to the same stream resource.  This fix
reverts back to the old behavior by setting the stream pointer back to
the beginning for each subsequent redirect.
  • Loading branch information
icc authored and mudrd8mz committed Jan 24, 2022
1 parent e63604f commit 080105c
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
10 changes: 10 additions & 0 deletions lib/filelib.php
Original file line number Diff line number Diff line change
Expand Up @@ -3828,6 +3828,16 @@ protected function request($url, $options = array()) {
return $urlisblocked;
}

// If the response body is written to a seekable stream resource, reset the stream pointer to avoid
// appending multiple response bodies to the same resource.
if (!empty($this->options['CURLOPT_FILE'])) {
$streammetadata = stream_get_meta_data($this->options['CURLOPT_FILE']);
if ($streammetadata['seekable']) {
ftruncate($this->options['CURLOPT_FILE'], 0);
rewind($this->options['CURLOPT_FILE']);
}
}

curl_setopt($curl, CURLOPT_URL, $redirecturl);
$ret = curl_exec($curl);

Expand Down
31 changes: 31 additions & 0 deletions lib/tests/filelib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ public function test_download_file_content() {
$contents = download_file_content("$testurl?redir=2");
$this->assertSame('done', $contents);

$contents = download_file_content("$testurl?redir=2&verbose=1");
$this->assertSame('done', $contents);

$response = download_file_content("$testurl?redir=2", null, null, true);
$this->assertInstanceOf('stdClass', $response);
$this->assertSame('200', $response->status);
Expand All @@ -153,6 +156,14 @@ public function test_download_file_content() {
$this->assertSame('done', $response->results);
$this->assertSame('', $response->error);

$response = download_file_content("$testurl?redir=2&verbose=1", null, null, true);
$this->assertInstanceOf('stdClass', $response);
$this->assertSame('200', $response->status);
$this->assertTrue(is_array($response->headers));
$this->assertMatchesRegularExpression('|^HTTP/1\.[01] 200 OK$|', rtrim($response->response_code));
$this->assertSame('done', $response->results);
$this->assertSame('', $response->error);

// Commented out this block if there are performance problems.
/*
$contents = download_file_content("$testurl?redir=6");
Expand Down Expand Up @@ -327,6 +338,17 @@ public function test_curl_redirects() {
$this->assertSame('done', file_get_contents($tofile));
@unlink($tofile);

$curl = new curl();
$tofile = "$CFG->tempdir/test.html";
@unlink($tofile);
$fp = fopen($tofile, 'w');
$result = $curl->get("$testurl?redir=1&verbose=1", array(), array('CURLOPT_FILE' => $fp));
$this->assertTrue($result);
fclose($fp);
$this->assertFileExists($tofile);
$this->assertSame('done', file_get_contents($tofile));
@unlink($tofile);

$curl = new curl();
$tofile = "$CFG->tempdir/test.html";
@unlink($tofile);
Expand All @@ -335,6 +357,15 @@ public function test_curl_redirects() {
$this->assertFileExists($tofile);
$this->assertSame('done', file_get_contents($tofile));
@unlink($tofile);

$curl = new curl();
$tofile = "$CFG->tempdir/test.html";
@unlink($tofile);
$result = $curl->download_one("$testurl?redir=1&verbose=1", array(), array('filepath' => $tofile));
$this->assertTrue($result);
$this->assertFileExists($tofile);
$this->assertSame('done', file_get_contents($tofile));
@unlink($tofile);
}

/**
Expand Down

0 comments on commit 080105c

Please sign in to comment.