From 17715388c69ecc14608ab12535aad4e98ca9e2b9 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 31 May 2010 13:20:15 +0200 Subject: [PATCH 1/9] [ticket/9627] Support for HTTP range requests in download/file.php Initial draft of "resume support" for attachments. This should allow users to resume partially downloaded attachments. PHPBB3-9627 --- phpBB/includes/functions_download.php | 114 ++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 87bf7a91a6..6d52e92e7c 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -226,6 +226,16 @@ function send_file_to_browser($attachment, $upload_dir, $category) if ($fp !== false) { + // Deliver file partially if requested + if ($range = http_byte_range($size)) + { + fseek($fp, $range['byte_pos_start']); + + send_status_line(206, 'Partial Content'); + header('Content-Range: bytes ' . $range['byte_pos_start'] . '-' . $range['byte_pos_end'] . '/' . $range['bytes_total']); + header('Content-Length: ' . $range['bytes_requested']); + } + while (!feof($fp)) { echo fread($fp, 8192); @@ -407,3 +417,107 @@ function file_gc() $db->sql_close(); exit; } + +/** +* HTTP range support (RFC 2616 Section 14.35) +* +* Allows browsers to request partial file content +* in case a download has been interrupted. +* +* A range request can contain multiple ranges, +* we however only handle the first request and +* only support requests from a given byte to the end of the file. +* +* @param int $filesize the size of the file in bytes we are about to deliver +* +* @return mixed false if the whole file has to be delivered +* associative array on success +* +* @author bantu +*/ +function http_byte_range($filesize) +{ + if (!$filesize) + { + return false; + } + + foreach (array($_SERVER, $_ENV) as $global) + { + if (isset($global['HTTP_RANGE'])) + { + $http_range = $global['HTTP_RANGE']; + break; + } + } + + // Return if no range requested + // Make sure range request starts with "bytes=" + if (empty($http_range) || strpos($http_range, 'bytes=') !== 0) + { + return false; + } + + // Strip leading 'bytes=' + // Multiple ranges can be separated by a comma + foreach (explode(',', substr($http_range, 6)) as $range_string) + { + $range = explode('-', trim($range_string)); + + // "-" is invalid, "0-0" however is valid and means the very first byte. + if (sizeof($range) != 2 || $range[0] === '' && $range[1] === '') + { + continue; + } + + if ($range[0] === '') + { + // Return last $range[1] bytes. + + if (!$range[1]) + { + continue; + } + + if ($range[1] >= $filesize) + { + return false; + } + + $first_byte_pos = $filesize - (int) $range[1]; + $last_byte_pos = $filesize - 1; + } + else + { + // Return bytes from $range[0] to $range[1] + + $first_byte_pos = (int) $range[0]; + $last_byte_pos = (int) $range[1]; + + if ($last_byte_pos && $last_byte_pos < $first_byte_pos) + { + // The requested range contains 0 bytes. + continue; + } + + if ($first_byte_pos >= $filesize) + { + // Requested range not satisfiable + return false; + } + + // Adjust last-byte-pos if it is absent or greater than the content. + if ($range[1] === '' || $last_byte_pos >= $filesize) + { + $last_byte_pos = $filesize - 1; + } + } + + return array( + 'byte_pos_start' => $first_byte_pos, + 'byte_pos_end' => $last_byte_pos, + 'bytes_requested' => $last_byte_pos - $first_byte_pos + 1, + 'bytes_total' => $filesize, + ); + } +} From 9ed36e1e1bd79b4633c1930b09952426cf172850 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 23 Jun 2010 15:29:33 +0200 Subject: [PATCH 2/9] [ticket/9627] Do not increase download counter if file is requested partially. PHPBB3-9627 --- phpBB/download/file.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index 891352f00c..f5dd7c7d39 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -246,7 +246,7 @@ if (!download_allowed()) $download_mode = (int) $extensions[$attachment['extension']]['download_mode']; // Fetching filename here to prevent sniffing of filename -$sql = 'SELECT attach_id, is_orphan, in_message, post_msg_id, extension, physical_filename, real_filename, mimetype, filetime +$sql = 'SELECT attach_id, is_orphan, in_message, post_msg_id, extension, physical_filename, real_filename, mimetype, filesize, filetime FROM ' . ATTACHMENTS_TABLE . " WHERE attach_id = $download_id"; $result = $db->sql_query_limit($sql, 1); @@ -275,7 +275,7 @@ if ($thumbnail) { $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; } -else if (($display_cat == ATTACHMENT_CATEGORY_NONE/* || $display_cat == ATTACHMENT_CATEGORY_IMAGE*/) && !$attachment['is_orphan']) +else if (($display_cat == ATTACHMENT_CATEGORY_NONE/* || $display_cat == ATTACHMENT_CATEGORY_IMAGE*/) && !$attachment['is_orphan'] && !http_byte_range($attachment['filesize'])) { // Update download count $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' From 56b0268d1de0575018fd542b9d1b9b71b88c1057 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 23 Jun 2010 15:38:24 +0200 Subject: [PATCH 3/9] [ticket/9627] Make sure the database record for the filesize is correct. PHPBB3-9627 --- phpBB/includes/functions_download.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 6d52e92e7c..606d6eec32 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -157,6 +157,16 @@ function send_file_to_browser($attachment, $upload_dir, $category) trigger_error('UNABLE_TO_DELIVER_FILE'); } + // Make sure the database record for the filesize is correct + if ($size > 0 && $size != $attachment['filesize']) + { + // Update database record + $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' + SET filesize = ' . (int) $size . ' + WHERE attach_id = ' . (int) $attachment['attach_id']; + $db->sql_query($sql); + } + // Now the tricky part... let's dance header('Pragma: public'); From 18e55708518fc53f5b134235895328de03680521 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 23 Jun 2010 16:26:53 +0200 Subject: [PATCH 4/9] [ticket/9627] Make use of 'static' since the function is called more than once PHPBB3-9627 --- phpBB/includes/functions_download.php | 37 ++++++++++++++++++++------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 606d6eec32..7013a80418 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -447,30 +447,49 @@ function file_gc() */ function http_byte_range($filesize) { + // The range request array; + // contains all requested ranges. + static $request_array; + if (!$filesize) { return false; } - foreach (array($_SERVER, $_ENV) as $global) + if (!isset($request_array)) { - if (isset($global['HTTP_RANGE'])) + $request_array = false; + + $globals = array( + array('_SERVER', 'HTTP_RANGE'), + array('_ENV', 'HTTP_RANGE'), + ); + + foreach ($globals as $array) { - $http_range = $global['HTTP_RANGE']; - break; + $global = $array[0]; + $key = $array[1]; + + // Make sure range request starts with "bytes=" + if (isset($GLOBALS[$global][$key]) && strpos($GLOBALS[$global][$key], 'bytes=') === 0) + { + // Strip leading 'bytes=' + // Multiple ranges can be separated by a comma + $request_array = explode(',', substr($GLOBALS[$global][$key], 6)); + + break; + } } } // Return if no range requested - // Make sure range request starts with "bytes=" - if (empty($http_range) || strpos($http_range, 'bytes=') !== 0) + if (empty($request_array)) { return false; } - // Strip leading 'bytes=' - // Multiple ranges can be separated by a comma - foreach (explode(',', substr($http_range, 6)) as $range_string) + // Go through all ranges + foreach ($request_array as $range_string) { $range = explode('-', trim($range_string)); From 7463a988ea03c2b6548e1a37f422f0974b3e1446 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Wed, 23 Jun 2010 16:31:25 +0200 Subject: [PATCH 5/9] [ticket/9627] Make sure range request reads till the end of the file. PHPBB3-9627 --- phpBB/includes/functions_download.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index 7013a80418..d863d9f87b 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -542,6 +542,12 @@ function http_byte_range($filesize) } } + // We currently do not support range requests that end before the end of the file + if ($last_byte_pos != $filesize - 1) + { + continue; + } + return array( 'byte_pos_start' => $first_byte_pos, 'byte_pos_end' => $last_byte_pos, From 3c618310105ea36ef0d4ea54b9bf9adb5d73e22e Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Thu, 14 Oct 2010 00:30:29 +0200 Subject: [PATCH 6/9] [ticket/9627] Split http_range_request() into several functions. Split http_range_request() into several functions for better reusability and to allow some unit testing. PHPBB3-9627 --- phpBB/includes/functions_download.php | 81 +++++++++++++++++---------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index d863d9f87b..ad0c7f1cff 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -434,21 +434,14 @@ function file_gc() * Allows browsers to request partial file content * in case a download has been interrupted. * -* A range request can contain multiple ranges, -* we however only handle the first request and -* only support requests from a given byte to the end of the file. -* * @param int $filesize the size of the file in bytes we are about to deliver * * @return mixed false if the whole file has to be delivered * associative array on success -* -* @author bantu */ function http_byte_range($filesize) { - // The range request array; - // contains all requested ranges. + // Only call find_range_request() once. static $request_array; if (!$filesize) @@ -458,36 +451,62 @@ function http_byte_range($filesize) if (!isset($request_array)) { - $request_array = false; + $request_array = find_range_request(); + } + + return (empty($request_array)) ? false : parse_range_request($request_array, $filesize); +} - $globals = array( - array('_SERVER', 'HTTP_RANGE'), - array('_ENV', 'HTTP_RANGE'), - ); +/** +* Searches for HTTP range request in super globals. +* +* @return mixed false if no request found +* array of strings containing the requested ranges otherwise +* e.g. array(0 => '0-0', 1 => '123-125') +*/ +function find_range_request() +{ + $globals = array( + array('_SERVER', 'HTTP_RANGE'), + array('_ENV', 'HTTP_RANGE'), + ); - foreach ($globals as $array) + foreach ($globals as $array) + { + $global = $array[0]; + $key = $array[1]; + + // Make sure range request starts with "bytes=" + if (isset($GLOBALS[$global][$key]) && strpos($GLOBALS[$global][$key], 'bytes=') === 0) { - $global = $array[0]; - $key = $array[1]; - - // Make sure range request starts with "bytes=" - if (isset($GLOBALS[$global][$key]) && strpos($GLOBALS[$global][$key], 'bytes=') === 0) - { - // Strip leading 'bytes=' - // Multiple ranges can be separated by a comma - $request_array = explode(',', substr($GLOBALS[$global][$key], 6)); - - break; - } + // Strip leading 'bytes=' + // Multiple ranges can be separated by a comma + return explode(',', substr($GLOBALS[$global][$key], 6)); } } - // Return if no range requested - if (empty($request_array)) - { - return false; - } + return false; +} +/** +* Analyses a range request array. +* +* A range request can contain multiple ranges, +* we however only handle the first request and +* only support requests from a given byte to the end of the file. +* +* @param array $request_array array of strings containing the requested ranges +* @param int $filesize the full size of the file in bytes that has been requested +* +* @return mixed false if the whole file has to be delivered +* associative array on success +* byte_pos_start the first byte position, can be passed to fseek() +* byte_pos_end the last byte position +* bytes_requested the number of bytes requested +* bytes_total the full size of the file +*/ +function parse_range_request($request_array, $filesize) +{ // Go through all ranges foreach ($request_array as $range_string) { From 5f034c0a0ad786f36a342e3815fba82e05a03343 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Mon, 30 Aug 2010 00:39:29 +0200 Subject: [PATCH 7/9] [ticket/9627] Adding download unit tests. PHPBB3-9627 --- tests/all_tests.php | 2 ++ tests/download/all_tests.php | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/download/all_tests.php diff --git a/tests/all_tests.php b/tests/all_tests.php index feaaa1f0a1..4c2fa891a2 100644 --- a/tests/all_tests.php +++ b/tests/all_tests.php @@ -26,6 +26,7 @@ require_once 'dbal/all_tests.php'; require_once 'regex/all_tests.php'; require_once 'network/all_tests.php'; require_once 'random/all_tests.php'; +require_once 'download/all_tests.php'; // exclude the test directory from code coverage reports if (version_compare(PHPUnit_Runner_Version::id(), '3.5.0') >= 0) @@ -59,6 +60,7 @@ class phpbb_all_tests $suite->addTest(phpbb_regex_all_tests::suite()); $suite->addTest(phpbb_network_all_tests::suite()); $suite->addTest(phpbb_random_all_tests::suite()); + $suite->addTest(phpbb_download_all_tests::suite()); return $suite; } diff --git a/tests/download/all_tests.php b/tests/download/all_tests.php new file mode 100644 index 0000000000..59a937f70a --- /dev/null +++ b/tests/download/all_tests.php @@ -0,0 +1,36 @@ + Date: Mon, 30 Aug 2010 00:54:36 +0200 Subject: [PATCH 8/9] [ticket/9627] Adding unit tests for http_byte_range(). PHPBB3-9627 --- tests/download/all_tests.php | 4 ++ tests/download/http_byte_range.php | 62 ++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 tests/download/http_byte_range.php diff --git a/tests/download/all_tests.php b/tests/download/all_tests.php index 59a937f70a..21305a887c 100644 --- a/tests/download/all_tests.php +++ b/tests/download/all_tests.php @@ -15,6 +15,8 @@ if (!defined('PHPUnit_MAIN_METHOD')) require_once 'test_framework/framework.php'; require_once 'PHPUnit/TextUI/TestRunner.php'; +require_once 'download/http_byte_range.php'; + class phpbb_download_all_tests { public static function main() @@ -26,6 +28,8 @@ class phpbb_download_all_tests { $suite = new PHPUnit_Framework_TestSuite('phpBB Download Tests'); + $suite->addTestSuite('phpbb_download_http_byte_range_test'); + return $suite; } } diff --git a/tests/download/http_byte_range.php b/tests/download/http_byte_range.php new file mode 100644 index 0000000000..075311a47c --- /dev/null +++ b/tests/download/http_byte_range.php @@ -0,0 +1,62 @@ +assertEquals(false, find_range_request()); + unset($_SERVER['HTTP_RANGE']); + + $_ENV['HTTP_RANGE'] = 'bztes='; + $this->assertEquals(false, find_range_request()); + unset($_ENV['HTTP_RANGE']); + + $_SERVER['HTTP_RANGE'] = 'bytes=0-0,123-125'; + $this->assertEquals(array('0-0', '123-125'), find_range_request()); + unset($_SERVER['HTTP_RANGE']); + } + + /** + * @dataProvider parse_range_request_data() + */ + public function test_parse_range_request($request_array, $filesize, $expected) + { + $this->assertEquals($expected, parse_range_request($request_array, $filesize)); + } + + public function parse_range_request_data() + { + return array( + // Does not read until the end of file. + array( + array('3-4'), + 10, + false, + ), + + // Valid request, handle second range. + array( + array('0-0', '120-125'), + 125, + array( + 'byte_pos_start' => 120, + 'byte_pos_end' => 124, + 'bytes_requested' => 5, + 'bytes_total' => 125, + ) + ), + ); + } +} From 0f49e529407e0b20610f980ff3cac47dcf141db1 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sat, 23 Oct 2010 18:03:49 +0200 Subject: [PATCH 9/9] [ticket/9627] Prefix function names with 'phpbb_'. PHPBB3-9627 --- phpBB/download/file.php | 2 +- phpBB/includes/functions_download.php | 12 ++++++------ tests/download/http_byte_range.php | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/phpBB/download/file.php b/phpBB/download/file.php index f5dd7c7d39..68a4afe03c 100644 --- a/phpBB/download/file.php +++ b/phpBB/download/file.php @@ -275,7 +275,7 @@ if ($thumbnail) { $attachment['physical_filename'] = 'thumb_' . $attachment['physical_filename']; } -else if (($display_cat == ATTACHMENT_CATEGORY_NONE/* || $display_cat == ATTACHMENT_CATEGORY_IMAGE*/) && !$attachment['is_orphan'] && !http_byte_range($attachment['filesize'])) +else if (($display_cat == ATTACHMENT_CATEGORY_NONE/* || $display_cat == ATTACHMENT_CATEGORY_IMAGE*/) && !$attachment['is_orphan'] && !phpbb_http_byte_range($attachment['filesize'])) { // Update download count $sql = 'UPDATE ' . ATTACHMENTS_TABLE . ' diff --git a/phpBB/includes/functions_download.php b/phpBB/includes/functions_download.php index ad0c7f1cff..94d851e383 100644 --- a/phpBB/includes/functions_download.php +++ b/phpBB/includes/functions_download.php @@ -237,7 +237,7 @@ function send_file_to_browser($attachment, $upload_dir, $category) if ($fp !== false) { // Deliver file partially if requested - if ($range = http_byte_range($size)) + if ($range = phpbb_http_byte_range($size)) { fseek($fp, $range['byte_pos_start']); @@ -439,7 +439,7 @@ function file_gc() * @return mixed false if the whole file has to be delivered * associative array on success */ -function http_byte_range($filesize) +function phpbb_http_byte_range($filesize) { // Only call find_range_request() once. static $request_array; @@ -451,10 +451,10 @@ function http_byte_range($filesize) if (!isset($request_array)) { - $request_array = find_range_request(); + $request_array = phpbb_find_range_request(); } - return (empty($request_array)) ? false : parse_range_request($request_array, $filesize); + return (empty($request_array)) ? false : phpbb_parse_range_request($request_array, $filesize); } /** @@ -464,7 +464,7 @@ function http_byte_range($filesize) * array of strings containing the requested ranges otherwise * e.g. array(0 => '0-0', 1 => '123-125') */ -function find_range_request() +function phpbb_find_range_request() { $globals = array( array('_SERVER', 'HTTP_RANGE'), @@ -505,7 +505,7 @@ function find_range_request() * bytes_requested the number of bytes requested * bytes_total the full size of the file */ -function parse_range_request($request_array, $filesize) +function phpbb_parse_range_request($request_array, $filesize) { // Go through all ranges foreach ($request_array as $range_string) diff --git a/tests/download/http_byte_range.php b/tests/download/http_byte_range.php index 075311a47c..cc42dee353 100644 --- a/tests/download/http_byte_range.php +++ b/tests/download/http_byte_range.php @@ -16,15 +16,15 @@ class phpbb_download_http_byte_range_test extends phpbb_test_case { // Missing 'bytes=' prefix $_SERVER['HTTP_RANGE'] = 'bztes='; - $this->assertEquals(false, find_range_request()); + $this->assertEquals(false, phpbb_find_range_request()); unset($_SERVER['HTTP_RANGE']); $_ENV['HTTP_RANGE'] = 'bztes='; - $this->assertEquals(false, find_range_request()); + $this->assertEquals(false, phpbb_find_range_request()); unset($_ENV['HTTP_RANGE']); $_SERVER['HTTP_RANGE'] = 'bytes=0-0,123-125'; - $this->assertEquals(array('0-0', '123-125'), find_range_request()); + $this->assertEquals(array('0-0', '123-125'), phpbb_find_range_request()); unset($_SERVER['HTTP_RANGE']); } @@ -33,7 +33,7 @@ class phpbb_download_http_byte_range_test extends phpbb_test_case */ public function test_parse_range_request($request_array, $filesize, $expected) { - $this->assertEquals($expected, parse_range_request($request_array, $filesize)); + $this->assertEquals($expected, phpbb_parse_range_request($request_array, $filesize)); } public function parse_range_request_data()