From 6fe586e54c69782194511d898ffbd6b7852d473b Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Thu, 23 Dec 2021 10:47:41 +1100 Subject: [PATCH] Round down s3 expiry to nearest minute, add more comments #457 - This should mean any identical files that expiry within the same minute of each other, should theoretically now have the same URL and can be cached and prevent multiple loads from the browser. - Also pushed out previous handling of expiry less than 1 minute to 2 minutes, which will be rounded down to the nearest minute by the new code. - Adjusted dataprovider for tests to reflect expected changes. --- classes/local/store/s3/client.php | 22 ++++++++----- tests/object_file_system_test.php | 52 ++++++++++++++++++------------- version.php | 4 +-- 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/classes/local/store/s3/client.php b/classes/local/store/s3/client.php index d9551f04..ee6948f3 100644 --- a/classes/local/store/s3/client.php +++ b/classes/local/store/s3/client.php @@ -571,26 +571,32 @@ public function get_expiration_time($now, $expires) { // Convert to a valid timestamp. $expires = strtotime($expires); } + // Invalid or already expired: // If it's set to 0 or strtotime() returned false, // set it to default + 1 min as a healthy margin. if (empty($expires)) { $expires = $now + $this->expirationtime + MINSECS; } - // If it's already expired or expires less than in a minute, - // set it to 1 minute. - if ($expires < $now + MINSECS) { - $expires = $now + MINSECS; + // Expiry too short, push it out to the next 2 minutes (will round down later): + // If it's already expired or expires less than 2 minutes, set it to 2 + // minutes. This works together with rounding down later on to ensure a + // non zero expiry time, and a minimum expiry of 1 minute. + if ($expires < $now + (2 * MINSECS)) { + $expires = $now + (2 * MINSECS); } + // Checks upper bound: // The expiration date of a signature version 4 presigned URL must be // less than one week. So if it's greater than a week set it to 1 week. // Use MINSECS as a healthy margin of error. if ($expires - $now > WEEKSECS - MINSECS) { $expires = $now + WEEKSECS - MINSECS; } - if (is_null($expires) || false === $expires) { - // Invalid date format use default instead. - $expires = $now + $this->expirationtime; - } + // Rounds (down) to nearest minute: + // With our new expiry time, ensure we round down to the nearest minute + // (#457) to ensure expiry of potentially the same file will use the + // same URL, and will result in less duplicate requests. + $expires -= ($expires % MINSECS); + return $expires; } diff --git a/tests/object_file_system_test.php b/tests/object_file_system_test.php index 7545a9f8..db632e3e 100644 --- a/tests/object_file_system_test.php +++ b/tests/object_file_system_test.php @@ -713,36 +713,44 @@ public function test_presigned_url_should_redirect_method_with_data_provider($en */ public function test_get_expiration_time_method_if_supported_provider() { $now = time(); + + // Seconds after the minute from X. + $secondsafternow = ($now % MINSECS); + $secondsafternowsub100 = $secondsafternow; + $secondsafternowadd30 = $secondsafternow; + $secondsafternowadd100 = $secondsafternow; + $secondsafternowaddweek = ($now + WEEKSECS) % MINSECS; + return [ // Default Pre-Signed URL expiration time and int-like 'Expires' header. - [7200, $now, 0, $now + 7200 + MINSECS], - [7200, $now, $now - 100, $now + MINSECS], - [7200, $now, $now + 30, $now + MINSECS], - [7200, $now, $now + 100, $now + 100], - [7200, $now, $now + WEEKSECS + HOURSECS, $now + WEEKSECS - MINSECS], + [7200, $now, 0, $now + 7200 + MINSECS - $secondsafternow], + [7200, $now, $now - 100, $now + (2 * MINSECS) - $secondsafternowsub100], + [7200, $now, $now + 30, $now + (2 * MINSECS) - $secondsafternowadd30], + [7200, $now, $now + 100, $now + (2 * MINSECS) - $secondsafternowadd100], + [7200, $now, $now + WEEKSECS + HOURSECS, $now + WEEKSECS - MINSECS - $secondsafternowaddweek], // Default Pre-Signed URL expiration time and string-like 'Expires' header. - [7200, $now, 'Thu, 01 Jan 1970 00:00:00 GMT', $now + 7200 + MINSECS], - [7200, $now, userdate($now - 100, '%a, %d %b %Y %H:%M:%S'), $now + MINSECS], - [7200, $now, userdate($now + 30, '%a, %d %b %Y %H:%M:%S'), $now + MINSECS], - [7200, $now, userdate($now + 100, '%a, %d %b %Y %H:%M:%S'), $now + 100], - [7200, $now, userdate($now + WEEKSECS + HOURSECS, '%a, %d %b %Y %H:%M:%S'), $now + WEEKSECS - MINSECS], + [7200, $now, 'Thu, 01 Jan 1970 00:00:00 GMT', $now + 7200 + MINSECS - $secondsafternow], + [7200, $now, userdate($now - 100, '%a, %d %b %Y %H:%M:%S'), $now + (2 * MINSECS) - $secondsafternowsub100], + [7200, $now, userdate($now + 30, '%a, %d %b %Y %H:%M:%S'), $now + (2 * MINSECS) - $secondsafternowadd30], + [7200, $now, userdate($now + 100, '%a, %d %b %Y %H:%M:%S'), $now + (2 * MINSECS) - $secondsafternowadd100], + [7200, $now, userdate($now + WEEKSECS + HOURSECS, '%a, %d %b %Y %H:%M:%S'), $now + WEEKSECS - MINSECS - $secondsafternowaddweek], // Custom Pre-Signed URL expiration time and int-like 'Expires' header. - [0, $now, 0, $now + MINSECS], - [600, $now, 0, $now + 600 + MINSECS], - [600, $now, $now - 100, $now + MINSECS], - [600, $now, $now + 30, $now + MINSECS], - [600, $now, $now + 100, $now + 100], - [600, $now, $now + WEEKSECS + HOURSECS, $now + WEEKSECS - MINSECS], + [0, $now, 0, $now + (2 * MINSECS) - $secondsafternow], + [600, $now, 0, $now + 600 + MINSECS - $secondsafternow], + [600, $now, $now - 100, $now + (2 * MINSECS) - $secondsafternowsub100], + [600, $now, $now + 30, $now + (2 * MINSECS) - $secondsafternowadd30], + [600, $now, $now + 100, $now + (2 * MINSECS) - $secondsafternowadd100], + [600, $now, $now + WEEKSECS + HOURSECS, $now + WEEKSECS - MINSECS - $secondsafternowaddweek], // Custom Pre-Signed URL expiration time and string-like 'Expires' header. - [0, $now, 'Thu, 01 Jan 1970 00:00:00 GMT', $now + MINSECS], - [600, $now, 'Thu, 01 Jan 1970 00:00:00 GMT', $now + 600 + MINSECS], - [600, $now, userdate($now - 100, '%a, %d %b %Y %H:%M:%S'), $now + MINSECS], - [600, $now, userdate($now + 30, '%a, %d %b %Y %H:%M:%S'), $now + MINSECS], - [600, $now, userdate($now + 100, '%a, %d %b %Y %H:%M:%S'), $now + 100], - [600, $now, userdate($now + WEEKSECS + HOURSECS, '%a, %d %b %Y %H:%M:%S'), $now + WEEKSECS - MINSECS], + [0, $now, 'Thu, 01 Jan 1970 00:00:00 GMT', $now + (2 * MINSECS) - $secondsafternow], + [600, $now, 'Thu, 01 Jan 1970 00:00:00 GMT', $now + 600 + MINSECS - $secondsafternow], + [600, $now, userdate($now - 100, '%a, %d %b %Y %H:%M:%S'), $now + (2 * MINSECS) - $secondsafternowsub100], + [600, $now, userdate($now + 30, '%a, %d %b %Y %H:%M:%S'), $now + (2 * MINSECS) - $secondsafternowadd30], + [600, $now, userdate($now + 100, '%a, %d %b %Y %H:%M:%S'), $now + (2 * MINSECS) - $secondsafternowadd100], + [600, $now, userdate($now + WEEKSECS + HOURSECS, '%a, %d %b %Y %H:%M:%S'), $now + WEEKSECS - MINSECS - $secondsafternowaddweek], ]; } diff --git a/version.php b/version.php index a0b44bb5..46e8690e 100644 --- a/version.php +++ b/version.php @@ -25,8 +25,8 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2021101100; // The current plugin version (Date: YYYYMMDDXX). -$plugin->release = 2021101100; // Same as version. +$plugin->version = 2021122300; // The current plugin version (Date: YYYYMMDDXX). +$plugin->release = 2021122300; // Same as version. $plugin->requires = 2013111811; // Requires Filesystem API. $plugin->component = "tool_objectfs"; $plugin->maturity = MATURITY_STABLE;