From 5d8d6b3ac940019a51d6c0ef23a1867fa107edbe Mon Sep 17 00:00:00 2001 From: Wim Leers Date: Tue, 6 Sep 2016 01:11:18 +0200 Subject: [PATCH 1/9] farfuture wip --- cdn.admin.inc | 129 --------------------------- cdn_ui/config/optional/tour.tour.cdn-ui.yml | 8 ++ cdn_ui/js/summaries.js | 3 + cdn_ui/src/Form/CdnSettingsForm.php | 16 ++++ config/install/cdn.settings.yml | 35 ++++++++ config/schema/cdn.conditions.schema.yml | 26 ++++++ config/schema/cdn.data_types.schema.yml | 5 ++ config/schema/cdn.schema.yml | 18 ++++ help/admin-details-mode-pull-far-future.html | 16 ---- help/admin-details-mode-pull-ufi.html | 28 ------ src/CdnController.php | 0 src/cdn.routing.yml | 0 12 files changed, 111 insertions(+), 173 deletions(-) delete mode 100644 help/admin-details-mode-pull-ufi.html create mode 100644 src/CdnController.php create mode 100644 src/cdn.routing.yml diff --git a/cdn.admin.inc b/cdn.admin.inc index 24df591..2790b8a 100644 --- a/cdn.admin.inc +++ b/cdn.admin.inc @@ -27,138 +27,9 @@ function cdn_admin_details_form($form, &$form_state) { $form = array(); _cdn_settings_form_prepare($form, $form_state); - $form['settings'][CDN_BASIC_FARFUTURE_VARIABLE] = array( - '#type' => 'checkbox', - '#title' => t('Far Future expiration'), - '#description' => _cdn_help('admin-details-mode-pull-far-future') . - t('Mark all files served from the CDN to expire in the far future — - improves client-side cacheability.
Note: this - requires the !preprocess-css-link performance setting to be enabled (or - your site will break).
Note: only use Far Future - expiration when using a CDN or a reverse proxy.', - array( - '!preprocess-css-link' => l( - '"Aggregate and compress CSS files"', - 'admin/config/development/performance', - array('fragment' => 'edit-bandwidth-optimization') - ), - ) - ), - '#default_value' => variable_get(CDN_BASIC_FARFUTURE_VARIABLE, CDN_BASIC_FARFUTURE_DEFAULT), - '#states' => array( - 'visible' => array( - ':input[name="' . CDN_MODE_VARIABLE . '"]' => array('value' => CDN_MODE_BASIC), - ) - ), - ); - - $format_variables = array( - '@format-directory' => '<' . t('directory') . '>', - '@format-extensions' => '<' . t('extensions') . '>', - '@format-unique-identifier-method' => '<' . t('unique identifier method') . '>', - ); - - $methods = array(); - $ufi_info = module_invoke_all('cdn_unique_file_identifier_info'); - foreach ($ufi_info as $ufi_method => $ufi) { - $methods[] = $ufi['label'] - . ' (' . $ufi_method . '): ' - . $ufi['description']; - } - $format_variables['!ufi-methods'] = theme('item_list', array('items' => $methods)); - - $form['settings'][CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE] = array( - '#type' => 'textarea', - '#title' => t('Unique file identifier generation'), - '#description' => _cdn_help('admin-details-mode-pull-ufi') . t('Define how unique file identifiers (UFIs) are generated.'), - '#size' => 35, - '#default_value' => variable_get(CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE, CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_DEFAULT), - '#states' => array( - 'visible' => array( - ':input[name="' . CDN_MODE_VARIABLE . '"]' => array('value' => CDN_MODE_BASIC), - ':input[name="' . CDN_BASIC_FARFUTURE_VARIABLE . '"]' => array('checked' => TRUE), - ) - ), - ); - - $form['settings']['ufis'] = array( - '#type' => 'fieldset', - '#collapsible' => TRUE, - '#collapsed' => TRUE, - '#title' => t('Available UFI methods'), - '#input' => TRUE, - '#id' => 'ufi-fs-id', - '#prefix' => '
', - '#suffix' => '
', - '#states' => array( - 'visible' => array( - ':input[name="' . CDN_MODE_VARIABLE . '"]' => array('value' => CDN_MODE_BASIC), - ':input[name="' . CDN_BASIC_FARFUTURE_VARIABLE . '"]' => array('checked' => TRUE), - ), - ), - ); - - $form['settings']['ufis']['content'] = array( - '#markup' => t('Available UFI methods: !ufi-methods', $format_variables) . - '

' . t('Note that if no UFI method is specified for a file - (because no rule matches), the CDN module will fall - back to the mtime method.') . '

', - '#prefix' => '
', - '#suffix' => '
', - ); - return system_settings_form($form); } -/** - * Default validate callback for the details form. - */ -function cdn_admin_details_form_validate($form, &$form_state) { - - // When in Origin Pull mode, check the CDN mapping for CDNs/reverse proxies. - if ($form_state['values'][CDN_MODE_VARIABLE] == CDN_MODE_BASIC) { - $domains = cdn_get_domains(); - $token = md5(rand()); - variable_set('cdn_reverse_proxy_test', $token); - $yays = array(); - $nays = array(); - foreach ($domains as $domain) { - $url = 'http://' . $domain . base_path() . 'cdn/farfuture/reverse-proxy-test/' . $token; - $r1 = drupal_http_request($url); - $r2 = drupal_http_request($url); - unset($r1->headers); - unset($r2->headers); - $args = array('%domain' => $domain); - if ($r1 == $r2) { - $yays[] = t('%domain is a CDN or a reverse proxy.', $args); - } - else { - if ($r1->code == 404) { - $nays[] = t('%domain is a static file server.', $args); - } - else { - $nays[] = t('%domain uses the same web server as this Drupal site.', $args); - } - } - } - variable_set('cdn_reverse_proxy_test', FALSE); - if (!empty($yays)) { - drupal_set_message(t('Perfect domains: !yay-list', - array( - '!yay-list' => theme('item_list', array('items' => $yays)), - ) - )); - } - if (!empty($nays)) { - drupal_set_message(t('Potentially problematic domains: !nay-list', - array( - '!nay-list' => theme('item_list', array('items' => $nays)), - ) - ), 'warning'); - } - } -} - //---------------------------------------------------------------------------- // Private functions. diff --git a/cdn_ui/config/optional/tour.tour.cdn-ui.yml b/cdn_ui/config/optional/tour.tour.cdn-ui.yml index 3f9d5f2..8181d41 100644 --- a/cdn_ui/config/optional/tour.tour.cdn-ui.yml +++ b/cdn_ui/config/optional/tour.tour.cdn-ui.yml @@ -56,3 +56,11 @@ tips: weight: 6 attributes: data-id: edit-mapping-simple-extensions-condition-toggle + cdn-ui-farfuture: + id: cdn-ui-farfuture + plugin: text + label: 'Forever cacheable files' + body: 'Marks all files to expire in the far future, telling browsers to always use cached files, speeding up page loads. Lets Drupal serve files, but cached by the CDN, so server impact is negligible. You can opt out.' + weight: 7 + attributes: + data-id: edit-farfuture diff --git a/cdn_ui/js/summaries.js b/cdn_ui/js/summaries.js index 6397001..aedfb0a 100644 --- a/cdn_ui/js/summaries.js +++ b/cdn_ui/js/summaries.js @@ -30,6 +30,9 @@ } }); + $('[data-drupal-selector="edit-farfuture"]').drupalSetSummary(function () { + return document.querySelector('input[name="farfuture[status]"]').checked ? Drupal.t('Enabled') : Drupal.t('Disabled'); + }); } }; diff --git a/cdn_ui/src/Form/CdnSettingsForm.php b/cdn_ui/src/Form/CdnSettingsForm.php index 5ec3496..af0f777 100644 --- a/cdn_ui/src/Form/CdnSettingsForm.php +++ b/cdn_ui/src/Form/CdnSettingsForm.php @@ -149,6 +149,19 @@ class CdnSettingsForm extends ConfigFormBase { ], ]; + $form['farfuture'] = [ + '#type' => 'details', + '#title' => $this->t('Forever cacheable files'), + '#group' => 'cdn_settings', + '#tree' => TRUE, + ]; + $form['farfuture']['status'] = [ + '#type' => 'checkbox', + '#title' => $this->t('Make files cacheable forever'), + '#description' => $this->t('Better performance thanks to better caching of files by the visitor. When a file changes a different URL is used, to ensure instantaneous updates for your visitors.'), + '#default_value' => $config->get('farfuture.status'), + ]; + return parent::buildForm($form, $form_state); } @@ -182,6 +195,9 @@ class CdnSettingsForm extends ConfigFormBase { } } + // Vertical tab: 'Forever cacheable files'. + $config->set('farfuture.status', (bool) $form_state->getValue(['farfuture', 'status'])); + $config->save(); parent::submitForm($form, $form_state); diff --git a/config/install/cdn.settings.yml b/config/install/cdn.settings.yml index e9c4d7b..7e8c153 100644 --- a/config/install/cdn.settings.yml +++ b/config/install/cdn.settings.yml @@ -55,3 +55,38 @@ mapping: # - cdn-c.com # conditions: # extensions: [jpg, jpeg, png] + +farfuture: + status: true + rules: + # Shipped files: deployment identifier. + # Rationale: they cannot change, unless in a deployment. + - + conditions: + directories: + - core + - modules + - profiles + - themes + - sites/*/modules + - sites/*/themes + uniqueness: deployment_identifier + # Uploaded and generated files in general: mtime. + # Rationale: they may be modified. + - + conditions: + directories: + - files + - sites/*/files + uniqueness: mtime + # Uploaded audio and video files in specific: perpetual. + # Rationale: they are extremely unlikely to be modified. + - + conditions: + directories: + - files + - sites/*/files + mediatypes: + - audio + - video + uniqueness: perpetual diff --git a/config/schema/cdn.conditions.schema.yml b/config/schema/cdn.conditions.schema.yml index 2e1780f..6b1dad4 100644 --- a/config/schema/cdn.conditions.schema.yml +++ b/config/schema/cdn.conditions.schema.yml @@ -10,3 +10,29 @@ cdn.condition.extensions: sequence: type: string label: 'Allowed file extension' + +cdn.condition.directories: + type: sequence + label: 'Allowed directories' + sequence: + type: string + label: 'Allowed directory' + +# One of 'application', 'audio', 'example', 'image', 'message', 'model', +# 'multipart', 'text' or 'video'. +# See http://www.iana.org/assignments/media-types/media-types.xhtml. +cdn.condition.mediatypes: + type: sequence + label: 'Allowed media types' + sequence: + type: string + label: 'Allowed media type' + +# For example: 'application/javascript', 'image/png', 'video/mp4'. +# See http://www.iana.org/assignments/media-types/media-types.xhtml. +cdn.condition.mimetypes: + type: sequence + label: 'Allowed MIME types' + sequence: + type: string + label: 'Allowed MIME type' diff --git a/config/schema/cdn.data_types.schema.yml b/config/schema/cdn.data_types.schema.yml index a9c8cb9..29b46e8 100644 --- a/config/schema/cdn.data_types.schema.yml +++ b/config/schema/cdn.data_types.schema.yml @@ -4,3 +4,8 @@ cdn.domain: type: string label: 'Domain' +# A method that identifies the uniqueness of a file. Examples: md5_hash, mtime, +# perpetual, deployment_identifier. +cdn.unique_file_identifier_method: + type: string + label: 'Unique file identifier method' diff --git a/config/schema/cdn.schema.yml b/config/schema/cdn.schema.yml index 6399199..ab61b67 100644 --- a/config/schema/cdn.schema.yml +++ b/config/schema/cdn.schema.yml @@ -12,3 +12,21 @@ cdn.settings: mapping: label: 'File URL to CDN mapping' type: cdn.mapping.[type] + farfuture: + label: 'Far Future expiration configuration' + type: mapping + mapping: + status: + label: 'Far Future expiration status' + type: boolean + rules: + label: 'Ordered list of unique file identifier generation rules' + type: sequence + sequence: + label: 'Unique file identifier generation rule' + type: mapping + mapping: + conditions: + type: cdn.conditions + uniqueness: + type: cdn.unique_file_identifier_method diff --git a/help/admin-details-mode-pull-far-future.html b/help/admin-details-mode-pull-far-future.html index 322bcef..c76b7ee 100644 --- a/help/admin-details-mode-pull-far-future.html +++ b/help/admin-details-mode-pull-far-future.html @@ -1,20 +1,4 @@ -

Recommended for maximum performance boost!

-

Mark all files served from the CDN to expire in the far future (decades from now). This is the same as telling browsers to always use the cached files. This can significantly speed up page loads. Files are also automatically compressed (when the browser supports it), to speed up page loads even further.
-Finally, CORS (Cross-Origin Resource Sharing) is also enabled automatically for all these files, to prevent issues with fonts and JavaScript files being served from domains other than the origin domain.

-

Of course, you still want visitors to immediately get new versions of files when they change. That is why unique filenames are generated automatically.

-

CSS aggregation

It is necessary to enable CSS aggregation, if you don't enable CSS aggregation, files referenced by the CSS (such as images and fonts) files that are served from the CDN will not load. To prevent access to unauthorized files, every "Far Future URL" is signed with a security token.

Without CSS aggregation, the URLs in the CSS files continue to be relative, and consquently, these files will be loaded using the same security token. But since the token is based on the filename, this will result in a HTTP 403 response.

With CSS aggregation, the URLs in the CSS files will be rewritten. However, Drupal core's CSS aggregation is not very smart either, and it will in fact cause more or less the same problem. That's why the CDN module comes with an override of Drupal core's CSS aggregation, which correctly alters every file URL. As a result, this also enables us to e.g. serve the CSS file from one CDN, the images referenced by it from another and the fonts referenced by it from yet another.

- -

CDN or reverse proxy required

-

The Far Future expiration setting causes files to be served through PHP for with the optimal headers, and some files are compressed automatically. This causes a lot of overhead.

-

This implies that you must use a CDN or a reverse proxy such as Varnish (a CDN is actually just a very advanced and expensive reverse proxy). If you don't, very long load times will be the result — your site will actually become slower!

-

In fact, if you use the simple "subdomains point to main domain" trick to parallelize page loads, you'll even find that Far Future expiration will not work at all when you're using a separate web server just for static files (typically nginx or lighttpd). This is because they won't be able to find the files due to the Far Future URLs being signed with a security token.
-It will work when you're using the same web server for these alternative domains as you're using for serving the actual Drupal site. But in that case, very long load times will be the result.

-

Conclusion: only use Far Future expiration when using a CDN or a reverse proxy.

- - -

Detailed information for the experts

-

The following HTTP headers are set: Expires, Cache-Control, Last-Modified, Vary and Access-Control-Allow-Origin for files with one of the following extensions: css, js, svg, ico, gif, jpg, jpeg, png, otf, ttf, eot, woff, flv, swf and of these extensions, some will also be automatically compressed: css, js, ico, svg, eot, otf, ttf.

diff --git a/help/admin-details-mode-pull-ufi.html b/help/admin-details-mode-pull-ufi.html deleted file mode 100644 index 48c5293..0000000 --- a/help/admin-details-mode-pull-ufi.html +++ /dev/null @@ -1,28 +0,0 @@ -

What is this?

- -

To be able to use Far Future expiration, we need to make sure that the URL of a file changes whenever the file changes. Otherwise, visitors will continue to use the old version of the file, that they've cached.
-Depending on the type of file, it is usually better to use a different unique file identifier (UFI) method. Overall, you will want to minimize the number of times that the file system needs to be accessed.

- -

That's why the perpetual, drupal_version, drupal_cache and deployment_id methods are the most efficient: they don't touch the file system at all. However, perpetual will never detect file changes, so only use that if you're absolutely certain a file will never change (e.g. for video files). drupal_version should only be used for Drupal core files. drupal_cache is useful while doing development and for validating your cache stack. And deployment_id should probably only be used for files that are managed through version control.

- -

Format

-

Enter one rule per line, in the format <directory>[|<extensions>]|<unique file identifier (UFI) method>:

- - -

Note: To see which UFI methods are available, please consult the UI — this help system only allows for static content.

- - -

Examples

-

This would generate a unique identifier for Drupal core files based on the Drupal core version, files in the site directory would get unique identifiers based on the last time they were modified and movie files would not receive a unique identifier (they're so large browser can't cache them anyway):

-
misc/*:modules/*:themes/*|drupal_version
-sites/*|mtime
-sites/*|.avi .m4v .mov .mp4 .wmv .flv|perpetual
-

In this second example, we're dealing with a more high-traffic website, where it is too costly to access the filesystem for every served file. Therefor, this site defines a CDN_DEPLOYMENT_ID constant somewhere in its codebase. This constant changes whenever a module or theme changes. This is therefor far more efficient. See the last line:

-
misc/*:modules/*:themes/*|drupal_version
-sites/*|mtime
-sites/*|.avi .m4v .mov .mp4 .wmv .flv|perpetual
-sites/*/modules/*:sites/*/themes/*|deployment_id
\ No newline at end of file diff --git a/src/CdnController.php b/src/CdnController.php new file mode 100644 index 0000000..e69de29 diff --git a/src/cdn.routing.yml b/src/cdn.routing.yml new file mode 100644 index 0000000..e69de29 -- 2.9.0 From c641cd2b360e771d87ecc4d776729f11ef6ca921 Mon Sep 17 00:00:00 2001 From: Wim Leers Date: Wed, 7 Sep 2016 13:25:07 +0200 Subject: [PATCH 2/9] further wip --- cdn.basic.farfuture.inc | 280 --------------------------- cdn.constants.inc | 12 -- cdn.module | 41 ---- cdn.routing.yml | 6 + cdn.services.yml | 11 ++ src/CdnController.php | 70 +++++++ src/PathProcessor/PathProcessorFarFuture.php | 38 ++++ src/cdn.routing.yml | 0 8 files changed, 125 insertions(+), 333 deletions(-) create mode 100644 cdn.routing.yml create mode 100644 src/PathProcessor/PathProcessorFarFuture.php delete mode 100644 src/cdn.routing.yml diff --git a/cdn.basic.farfuture.inc b/cdn.basic.farfuture.inc index c6b5fab..e6d5a7b 100644 --- a/cdn.basic.farfuture.inc +++ b/cdn.basic.farfuture.inc @@ -6,169 +6,6 @@ */ -//---------------------------------------------------------------------------- -// Menu system callbacks. - -function cdn_basic_farfuture_download($token, $ufi, $path) { - // Validate the token to make sure this request originated from CDN. - $path_info = pathinfo($path); - $sec_token = drupal_hmac_base64($ufi . $path_info['filename'], drupal_get_private_key() . drupal_get_hash_salt()); - if ($token != $sec_token) { - header('HTTP/1.1 403 Forbidden'); - exit(); - } - - // Disallow downloading of files that are also not allowed to be downloaded - // by Drupal's .htaccess file. - if (preg_match("/\.(engine|inc|info|install|make|module|profile|test|po|sh|php([3-6])?|phtml|.*sql|theme|tpl(\.php)?|xtmpl)$|^(\..*|Entries.*|Repository|Root|Tag|Template)$/", $path)) { - header('HTTP/1.1 403 Forbidden'); - exit(); - } - - if (!file_exists($path)) { - watchdog( - 'cdn', - 'CDN Far Future 404: %file.', - array('%file' => $path), - WATCHDOG_ALERT - ); - header('HTTP/1.1 404 Not Found'); - exit(); - } - - // Remove some useless/unwanted headers. - $remove_headers = explode("\n", CDN_BASIC_FARFUTURE_REMOVE_HEADERS); - $current_headers = array(); - foreach (headers_list() as $header) { - $parts = explode(':', $header); - $current_headers[] = $parts[0]; - } - foreach ($remove_headers as $header) { - if (in_array($header, $current_headers)) { - // header_remove() only exists in PHP >=5.3 - if (function_exists('header_remove')) { - header_remove($header); - } - else { - // In PHP <5.3, we cannot remove headers. At least shorten them to save - // every byte possible and to stop leaking information needlessly. - header($header . ':'); - } - } - } - - // Remove all previously set Cache-Control headers, because we're going to - // override it. Since multiple Cache-Control headers might have been set, - // simply setting a new, overriding header isn't enough: that would only - // override the *last* Cache-Control header. Yay for PHP! - if (function_exists('header_remove')) { - header_remove('Cache-Control'); - } - else { - header("Cache-Control:"); - header("Cache-Control:"); - } - - // Default caching rules: no caching/immediate expiration. - header("Cache-Control: private, must-revalidate, proxy-revalidate"); - header("Expires: " . gmdate("D, d M Y H:i:s", time() - 86400) . "GMT"); - - // Instead of being powered by PHP, tell the world this resource was powered - // by the CDN module! - header("X-Powered-By: Drupal CDN module"); - // Determine the content type. - header("Content-Type: " . _cdn_basic_farfuture_get_mimetype(basename($path))); - // Support partial content requests. - header("Accept-Ranges: bytes"); - // Browsers that implement the W3C Access Control specification might refuse - // to use certain resources such as fonts if those resources violate the - // same-origin policy. Send a header to explicitly allow cross-domain use of - // those resources. (This is called Cross-Origin Resource Sharing, or CORS.) - header("Access-Control-Allow-Origin: *"); - - // If the extension of the file that's being served is one of the far future - // extensions (by default: images, fonts and flash content), then cache it - // in the far future. - $farfuture_extensions = variable_get(CDN_BASIC_FARFUTURE_EXTENSIONS_VARIABLE, CDN_BASIC_FARFUTURE_EXTENSIONS_DEFAULT); - $extension = drupal_strtolower(pathinfo($path, PATHINFO_EXTENSION)); - if (in_array($extension, explode("\n", $farfuture_extensions))) { - // Remove all previously set Cache-Control headers, because we're going to - // override it. Since multiple Cache-Control headers might have been set, - // simply setting a new, overriding header isn't enough: that would only - // override the *last* Cache-Control header. Yay for PHP! - if (function_exists('header_remove')) { - header_remove('Cache-Control'); - } - else { - header("Cache-Control:"); - header("Cache-Control:"); - } - // Set a far future Cache-Control header (480 weeks), which prevents - // intermediate caches from transforming the data and allows any - // intermediate cache to cache it, since it's marked as a public resource. - header("Cache-Control: max-age=290304000, no-transform, public"); - // Set a far future Expires header. The maximum UNIX timestamp is somewhere - // in 2038. Set it to a date in 2037, just to be safe. - header("Expires: Tue, 20 Jan 2037 04:20:42 GMT"); - // Pretend the file was last modified a long time ago in the past, this will - // prevent browsers that don't support Cache-Control nor Expires headers to - // still request a new version too soon (these browsers calculate a - // heuristic to determine when to request a new version, based on the last - // time the resource has been modified). - // Also see http://code.google.com/speed/page-speed/docs/caching.html. - header("Last-Modified: Wed, 20 Jan 1988 04:20:42 GMT"); - } - - // GET requests with an "Accept-Encoding" header that lists "gzip". - if (isset($_SERVER['HTTP_ACCEPT_ENCODING']) && strpos($_SERVER['HTTP_ACCEPT_ENCODING'], 'gzip') !== FALSE) { - // Only send gzipped files for some file extensions (it doesn't make sense - // to gzip images, for example). - if (in_array($extension, explode("\n", CDN_BASIC_FARFUTURE_GZIP_EXTENSIONS))) { - // Ensure a gzipped version of the file is stored on disk, instead of - // gzipping the file on every request. - $gzip_path = drupal_realpath('public://') . '/' . CDN_BASIC_FARFUTURE_GZIP_DIRECTORY . "/$path.$ufi.gz"; - if (!file_exists($gzip_path)) { - _cdn_basic_farfuture_create_directory_structure(dirname($gzip_path)); - file_put_contents($gzip_path, gzencode(file_get_contents($path), 9)); - } - // Make sure zlib.output_compression does not gzip our gzipped output. - ini_set('zlib.output_compression', '0'); - // Instruct intermediate HTTP caches to store both a compressed (gzipped) - // and uncompressed version of the resource. - header("Vary: Accept-Encoding"); - // Prepare for gzipped output. - header("Content-Encoding: gzip"); - $path = $gzip_path; - } - } - - // Conditional GET requests (i.e. with If-Modified-Since header). - if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { - // All files served by this function are designed to expire in the far - // future. Hence we can simply always tell the client the requested file - // was not modified. - header("HTTP/1.1 304 Not Modified"); - } - // "Normal" GET requests. - else { - _cdn_transfer_file($path); - } - - exit(); -} - -function cdn_basic_farfuture_reverseproxy_test($token) { - $reference_token = variable_get('cdn_reverse_proxy_test'); - if ($reference_token === FALSE || $token != $reference_token) { - header('HTTP/1.1 403 Forbidden'); - exit(); - } - - print REQUEST_TIME . '-' . md5(rand()); - exit(); -} - - //---------------------------------------------------------------------------- // Public functions. @@ -314,123 +151,6 @@ function _cdn_basic_farfuture_parse_raw_mapping($mapping_raw) { return $mapping; } -/** - * Variant of Drupal's file_transfer(), based on - * http://www.thomthom.net/blog/2007/09/php-resumable-download-server/ - * to support ranged requests as well. - * - * Note: ranged requests that request multiple ranges are not supported. They - * are responded to with a 416. See - * http://www.w3.org/Protocols/rfc2616/rfc2616-sec19.html - */ -function _cdn_transfer_file($path) { - $fp = @fopen($path, 'rb'); - - $size = filesize($path); // File size - $length = $size; // Content length - $start = 0; // Start byte - $end = $size - 1; // End byte - - // In case of a range request, seek within the file to the correct location. - if (isset($_SERVER['HTTP_RANGE'])) { - $c_start = $start; - $c_end = $end; - - // Extract the string containing the requested range. - list(, $range) = explode('=', $_SERVER['HTTP_RANGE'], 2); - - // If the client requested multiple ranges, repond with a 416. - if (strpos($range, ',') !== FALSE) { - header('HTTP/1.1 416 Requested Range Not Satisfiable'); - header("Content-Range: bytes $start-$end/$size"); - exit(); - } - - // Case "Range: -n": final n bytes are requested. - if ($range[0] == '-') { - $c_start = $size - substr($range, 1); - } - // Case "Range: m-n": bytes m through n are requested. When n is empty or - // non-numeric, n is the last byte. - else { - $range = explode('-', $range); - $c_start = intval($range[0]); - $c_end = (isset($range[1]) && is_numeric($range[1])) ? intval($range[1]) : $size; - } - // Minor normalization: end bytes can not be larger than $end. - $c_end = ($c_end > $end) ? $end : $c_end; - - // If the requested range is not valid, respond with a 416. - if ($c_start > $c_end || $c_start > $end || $c_end > $end) { - header('HTTP/1.1 416 Requested Range Not Satisfiable'); - header("Content-Range: bytes $start-$end/$size"); - exit(); - } - - $start = $c_start; - $end = $c_end; - $length = $end - $start + 1; - fseek($fp, $start); - - // The ranged request is valid and will be performed, respond with a 206. - header('HTTP/1.1 206 Partial Content'); - header("Content-Range: bytes $start-$end/$size"); - } - header("Content-Length: $length"); - - // Start buffered download. Prevent reading too far for a ranged request. - $buffer = 1024 * 8; - while (!feof($fp) && ($p = ftell($fp)) <= $end) { - if ($p + $buffer > $end) { - $buffer = $end - $p + 1; - } - set_time_limit(0); // Reset time limit for big files. - echo fread($fp, $buffer); - flush(); // Free up memory, to prevent triggering PHP's memory limit. - } - fclose($fp); -} - -/** - * Determine an Internet Media Type, or MIME type from a filename. - * Borrowed from Drupal 7. - * - * @param $path - * A string containing the file path. - * @return - * The internet media type registered for the extension or - * application/octet-stream for unknown extensions. - */ -function _cdn_basic_farfuture_get_mimetype($path) { - static $mapping; - if (!isset($mapping)) { - // The default file map, defined in file.mimetypes.inc is quite big. - // We only load it when necessary. - include_once DRUPAL_ROOT . '/includes/file.mimetypes.inc'; - $mapping = file_mimetype_mapping(); - } - - $extension = ''; - $file_parts = explode('.', basename($path)); - - // Remove the first part: a full filename should not match an extension. - array_shift($file_parts); - - // Iterate over the file parts, trying to find a match. - // For my.awesome.image.jpeg, we try: - // - jpeg - // - image.jpeg, and - // - awesome.image.jpeg - while ($additional_part = array_pop($file_parts)) { - $extension = drupal_strtolower($additional_part . ($extension ? '.' . $extension : '')); - if (isset($mapping['extensions'][$extension])) { - return $mapping['mimetypes'][$mapping['extensions'][$extension]]; - } - } - - return 'application/octet-stream'; -} - /** * Perform a nested HTTP request to generate a file. * diff --git a/cdn.constants.inc b/cdn.constants.inc index 03aea4b..0eb839f 100644 --- a/cdn.constants.inc +++ b/cdn.constants.inc @@ -7,15 +7,3 @@ // Variables and values. define('CDN_DRUPAL_ROOT_VARIABLE', 'cdn_drupal_root'); - -// Variables for basic mode. -define('CDN_BASIC_FARFUTURE_VARIABLE', 'cdn_farfuture_status'); -define('CDN_BASIC_FARFUTURE_DEFAULT', FALSE); -define('CDN_BASIC_FARFUTURE_EXTENSIONS_VARIABLE', "cdn_farfuture_extensions"); -define('CDN_BASIC_FARFUTURE_EXTENSIONS_DEFAULT', "css\njs\nsvg\nico\ngif\njpg\njpeg\npng\notf\nttf\neot\nwoff\nflv\nswf"); -define('CDN_BASIC_FARFUTURE_GZIP_EXTENSIONS', "css\njs\nico\nsvg\neot\notf\nttf"); -define('CDN_BASIC_FARFUTURE_GZIP_DIRECTORY', "cdn/farfuture/gzip"); -define('CDN_BASIC_FARFUTURE_REMOVE_HEADERS', "Set-Cookie\nETag"); -define('CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE', 'cdn_farfuture_unique_identifier_mapping'); -define('CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_DEFAULT', "misc/*:modules/*:themes/*|drupal_version\nsites/*|mtime\nsites/*|.avi .m4v .mov .mp4 .wmv .flv|perpetual"); -define('CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_DEFAULT', 'mtime'); diff --git a/cdn.module b/cdn.module index 49c0cc3..5caf8db 100644 --- a/cdn.module +++ b/cdn.module @@ -129,33 +129,6 @@ function cdn_cdn_unique_file_identifier_info() { ); } -/** - * Implements hook_menu(). - */ -function cdn_menu() { - // Origin Pull mode's Far Future expiration support. - $items['cdn/farfuture/%/%/%menu_tail'] = array( - 'title' => 'Download a far futured file', - 'access callback' => TRUE, - 'page callback' => 'cdn_basic_farfuture_download', - 'page arguments' => array(2, 3, 4), - 'type' => MENU_CALLBACK, - 'load arguments' => array('%map', '%index'), - 'file' => 'cdn.basic.farfuture.inc', - ); - $items['cdn/farfuture/reverse-proxy-test/%'] = array( - 'title' => 'Far Future reverse proxy test', - 'access callback' => TRUE, - 'page callback' => 'cdn_basic_farfuture_reverseproxy_test', - 'page arguments' => array(3), - 'type' => MENU_CALLBACK, - 'file' => 'cdn.basic.farfuture.inc', - ); - - return $items; -} - - //---------------------------------------------------------------------------- // Public functions. @@ -165,17 +138,3 @@ function cdn_menu() { function cdn_load_include($basename) { module_load_include('inc', 'cdn', "cdn.$basename"); } - - -//---------------------------------------------------------------------------- -// Private functions. - -/** - * Callback for generating a unique file identifier. - * - * @param $path - * The file path to the file for which to generate a unique identifier. - */ -function _cdn_ufi_deployment_id($path) { - return CDN_DEPLOYMENT_ID; -} diff --git a/cdn.routing.yml b/cdn.routing.yml new file mode 100644 index 0000000..9fb2ee4 --- /dev/null +++ b/cdn.routing.yml @@ -0,0 +1,6 @@ +cdn.farfuture: + path: '/cdn/farfuture/{token}/{unique_file_identifier_method}' + defaults: + _controller: '\Drupal\cdn\CdnController::farfuture' + requirements: + _access: 'TRUE' diff --git a/cdn.services.yml b/cdn.services.yml index 82a2385..d796600 100644 --- a/cdn.services.yml +++ b/cdn.services.yml @@ -18,3 +18,14 @@ services: arguments: ['@cdn.settings'] tags: - { name: event_subscriber } + + # Path processor for Far Future support, since the Drupal 8/Symfony routing + # system does not support "menu tail" or "slash in route parameter". + # See: + # - http://symfony.com/doc/2.8/routing/slash_in_parameter.html + # - http://drupal.stackexchange.com/questions/175758/slashes-in-single-route-parameter-or-other-ways-to-handle-a-menu-tail-with-dynam + # - https://api.drupal.org/api/drupal/includes%21menu.inc/function/menu_tail_to_arg/7.x + path_processor.cdn: + class: Drupal\cdn\PathProcessor\PathProcessorFarfuture + tags: + - { name: path_processor_inbound } diff --git a/src/CdnController.php b/src/CdnController.php index e69de29..ce9f6d7 100644 --- a/src/CdnController.php +++ b/src/CdnController.php @@ -0,0 +1,70 @@ +query->has('file')) { + throw new AccessDeniedException(); + } + + $file = $request->query->get('file'); + $calculated_token = Crypt::hmacBase64($unique_file_identifier_method . $file, \Drupal::service('private_key')->get() . Settings::getHashSalt()); + if ($token !== $calculated_token) { + throw new AccessDeniedException('Invalid token.'); + } + + if (!file_exists($file)) { +// watchdog( +// 'cdn', +// 'CDN Far Future 404: %file.', +// array('%file' => $path), +// WATCHDOG_ALERT +// ); + throw new NotFoundHttpException(); + } + + $farfuture_headers = [ + // Instead of being powered by PHP, tell the world this resource was powered + // by the CDN module! + 'X-Powered-By' => 'Drupal CDN module (https://www.drupal.org/project/cdn)', + // Browsers that implement the W3C Access Control specification might refuse + // to use certain resources such as fonts if those resources violate the + // same-origin policy. Send a header to explicitly allow cross-domain use of + // those resources. (This is called Cross-Origin Resource Sharing, or CORS.) + // The CDN module allows any domain to access it by default, which means + // hotlinking of these assets is possible. If you want to prevent this, + // implement a KernelEvents::RESPONSE subscriber that modifies this header + // for this route. + 'Access-Control-Allow-Origin' => '*', + 'Access-Control-Allow-Methods' => 'GET, HEAD', + // Set a far future Cache-Control header (480 weeks), which prevents + // intermediate caches from transforming the data and allows any + // intermediate cache to cache it, since it's marked as a public resource. + 'Cache-Control' => 'max-age=290304000, no-transform, public', + // Set a far future Expires header. The maximum UNIX timestamp is + // somewhere in 2038. Set it to a date in 2037, just to be safe. + 'Expires' => 'Tue, 20 Jan 2037 04:20:42 GMT', + // Pretend the file was last modified a long time ago in the past, this + // will prevent browsers that don't support Cache-Control nor Expires + // headers to still request a new version too soon (these browsers + // calculate a heuristic to determine when to request a new version, based + // on the last time the resource has been modified). + // Also see http://code.google.com/speed/page-speed/docs/caching.html. + 'Last-Modified' => 'Wed, 20 Jan 1988 04:20:42 GMT', + ]; + + $response = new BinaryFileResponse($file, 200, $farfuture_headers, TRUE, NULL, FALSE, FALSE); + $response->isNotModified($request); + return $response; + } + +} diff --git a/src/PathProcessor/PathProcessorFarFuture.php b/src/PathProcessor/PathProcessorFarFuture.php new file mode 100644 index 0000000..b537664 --- /dev/null +++ b/src/PathProcessor/PathProcessorFarFuture.php @@ -0,0 +1,38 @@ +query->set('file', $file); + + // Return the same path, but without the trailing file. + return "/cdn/farfuture/$token/$unique_file_identifier_method"; + } + +} diff --git a/src/cdn.routing.yml b/src/cdn.routing.yml deleted file mode 100644 index e69de29..0000000 -- 2.9.0 From 64f48d1fb4c7117f4c78084d28bd2b91d7d12023 Mon Sep 17 00:00:00 2001 From: Wim Leers Date: Wed, 7 Sep 2016 23:16:53 +0200 Subject: [PATCH 3/9] basically working! Just with hardcoded UFI for now. --- cdn.routing.yml | 2 +- src/CdnController.php | 36 ++++++++++------------------ src/CdnSettings.php | 7 ++++++ src/File/FileUrlGenerator.php | 12 ++++++++++ src/PathProcessor/PathProcessorFarFuture.php | 12 +++++----- 5 files changed, 39 insertions(+), 30 deletions(-) diff --git a/cdn.routing.yml b/cdn.routing.yml index 9fb2ee4..51561d2 100644 --- a/cdn.routing.yml +++ b/cdn.routing.yml @@ -1,5 +1,5 @@ cdn.farfuture: - path: '/cdn/farfuture/{token}/{unique_file_identifier_method}' + path: '/cdn/farfuture/{token}/{unique_file_identifier}' defaults: _controller: '\Drupal\cdn\CdnController::farfuture' requirements: diff --git a/src/CdnController.php b/src/CdnController.php index ce9f6d7..029f693 100644 --- a/src/CdnController.php +++ b/src/CdnController.php @@ -5,41 +5,31 @@ namespace Drupal\cdn; use Drupal\Component\Utility\Crypt; use Drupal\Core\Site\Settings; use Symfony\Component\HttpFoundation\BinaryFileResponse; -use Symfony\Component\HttpFoundation\File\Exception\AccessDeniedException; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; class CdnController { - public function farfuture(Request $request, $token, $unique_file_identifier_method) { - if (!$request->query->has('file')) { - throw new AccessDeniedException(); + public function farfuture(Request $request, $token, $unique_file_identifier) { + if (!$request->query->has('root_relative_file_url')) { + throw new AccessDeniedHttpException(); } - $file = $request->query->get('file'); - $calculated_token = Crypt::hmacBase64($unique_file_identifier_method . $file, \Drupal::service('private_key')->get() . Settings::getHashSalt()); + $root_relative_file_url = $request->query->get('root_relative_file_url'); + $calculated_token = Crypt::hmacBase64($unique_file_identifier . $root_relative_file_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); if ($token !== $calculated_token) { - throw new AccessDeniedException('Invalid token.'); - } - - if (!file_exists($file)) { -// watchdog( -// 'cdn', -// 'CDN Far Future 404: %file.', -// array('%file' => $path), -// WATCHDOG_ALERT -// ); - throw new NotFoundHttpException(); + throw new AccessDeniedHttpException('Invalid token.'); } $farfuture_headers = [ // Instead of being powered by PHP, tell the world this resource was powered // by the CDN module! 'X-Powered-By' => 'Drupal CDN module (https://www.drupal.org/project/cdn)', - // Browsers that implement the W3C Access Control specification might refuse - // to use certain resources such as fonts if those resources violate the - // same-origin policy. Send a header to explicitly allow cross-domain use of - // those resources. (This is called Cross-Origin Resource Sharing, or CORS.) + // Browsers that implement the W3C Access Control specification might + // refuse to use certain resources such as fonts if those resources + // violate the same-origin policy. Send a header to explicitly allow + // cross-domain use of those resources. (This is called Cross-Origin + // Resource Sharing, or CORS.) // The CDN module allows any domain to access it by default, which means // hotlinking of these assets is possible. If you want to prevent this, // implement a KernelEvents::RESPONSE subscriber that modifies this header @@ -62,7 +52,7 @@ class CdnController { 'Last-Modified' => 'Wed, 20 Jan 1988 04:20:42 GMT', ]; - $response = new BinaryFileResponse($file, 200, $farfuture_headers, TRUE, NULL, FALSE, FALSE); + $response = new BinaryFileResponse(substr($root_relative_file_url, 1), 200, $farfuture_headers, TRUE, NULL, FALSE, FALSE); $response->isNotModified($request); return $response; } diff --git a/src/CdnSettings.php b/src/CdnSettings.php index c434a25..17708c3 100644 --- a/src/CdnSettings.php +++ b/src/CdnSettings.php @@ -43,6 +43,13 @@ class CdnSettings { return $this->rawSettings->get('status') === 2; } + /** + * @return bool + */ + public function farfutureIsEnabled() { + return $this->rawSettings->get('farfuture.status'); + } + /** * Returns the lookup table. * diff --git a/src/File/FileUrlGenerator.php b/src/File/FileUrlGenerator.php index 378d849..ce163d0 100644 --- a/src/File/FileUrlGenerator.php +++ b/src/File/FileUrlGenerator.php @@ -3,8 +3,10 @@ namespace Drupal\cdn\File; use Drupal\cdn\CdnSettings; +use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Unicode; use Drupal\Core\File\FileSystemInterface; +use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -121,6 +123,16 @@ class FileUrlGenerator { $cdn_domain = $result; } + if ($this->settings->farfutureIsEnabled() && file_exists($uri)) { + $unique_file_identifier = 'perpetual'; + + // Generate a unique token to verify that the request was generated by + // CDN. We cannot use drupal_get_token() since it depends on the user + // session. + $calculated_token = Crypt::hmacBase64($unique_file_identifier . $root_relative_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); + return '//' . $cdn_domain . '/cdn/farfuture/' . $calculated_token . '/' . $unique_file_identifier . $root_relative_url; + } + return '//' . $cdn_domain . $root_relative_url; } diff --git a/src/PathProcessor/PathProcessorFarFuture.php b/src/PathProcessor/PathProcessorFarFuture.php index b537664..693fb7f 100644 --- a/src/PathProcessor/PathProcessorFarFuture.php +++ b/src/PathProcessor/PathProcessorFarFuture.php @@ -24,15 +24,15 @@ class PathProcessorFarFuture implements InboundPathProcessorInterface { return $path; } - // Parse the token, unique file identifier method and file. - $rest = preg_replace('|^/cdn/farfuture/|', '', $path); - list($token, $unique_file_identifier_method, $file) = explode('/', $rest, 3); + // Parse the token, unique file identifier method and root-relative file URL. + $tail = substr($path, strlen('/cdn/farfuture/')); + list($token, $unique_file_identifier, $root_relative_file_url) = explode('/', $tail, 3); - // Set the file as query parameter. - $request->query->set('file', $file); + // Set the root-relative file URL as query parameter. + $request->query->set('root_relative_file_url', '/' . $root_relative_file_url); // Return the same path, but without the trailing file. - return "/cdn/farfuture/$token/$unique_file_identifier_method"; + return "/cdn/farfuture/$token/$unique_file_identifier"; } } -- 2.9.0 From acbe78ae588a65d60d8118b83fd756d700f51763 Mon Sep 17 00:00:00 2001 From: Wim Leers Date: Thu, 8 Sep 2016 11:23:35 +0200 Subject: [PATCH 4/9] delete more --- cdn.basic.farfuture.inc | 81 ------------------------------------------------- cdn.module | 62 +------------------------------------ src/CdnController.php | 24 ++++++++++++++- 3 files changed, 24 insertions(+), 143 deletions(-) diff --git a/cdn.basic.farfuture.inc b/cdn.basic.farfuture.inc index e6d5a7b..9a9707f 100644 --- a/cdn.basic.farfuture.inc +++ b/cdn.basic.farfuture.inc @@ -151,84 +151,3 @@ function _cdn_basic_farfuture_parse_raw_mapping($mapping_raw) { return $mapping; } -/** - * Perform a nested HTTP request to generate a file. - * - * @param $path - * A path relative to the Drupal root (not urlencoded!) to the file that - * should be generated. - * @param $original_uri - * The original url if available. - * @return - * Whether the file was generated or not. - */ -function _cdn_basic_farfuture_generate_file($path, $original_uri = NULL) { - // Check if there's a file to generate in the first place! - if (!menu_get_item($path)) { - return FALSE; - } - - // While it should already be impossible to enter recursion because of the - // above menu system check, we still want to detect this just to be safe. - // This really can only happen if a file is missing and we try to generate - // it and the request to generate the file itself triggers a 404, which - // again references the file that is missing, and would thus again trigger a - // 404, etc. - // @see http://drupal.org/node/1417616#comment-5694960 - if (request_uri() == base_path() . $path) { - watchdog('cdn', 'Recursion detected for %file!', array('%file' => $path), WATCHDOG_ALERT); - header('HTTP/1.1 404 Not Found'); - exit(); - } - - $url = $GLOBALS['base_url'] . '/' . drupal_encode_path($path); - - // If this is an image style url ensure the image style token is set. Since we - // have just limited information to work with the evaluation is quite complex. - // The token query is added even if the 'image_allow_insecure_derivatives' - // variable is TRUE, so that the emitted links remain valid if it is changed - // back to the default FALSE. - if (($scheme = file_uri_scheme($original_uri)) && file_stream_wrapper_valid_scheme($scheme) && stripos($original_uri, $scheme . '://styles/') === 0) { - $parts = explode('/', $original_uri, 6); - $orinal_image_path = $scheme . '://' . $parts[5]; - $token_query = array(IMAGE_DERIVATIVE_TOKEN => image_style_path_token($parts[3], $orinal_image_path)); - $url .= (strpos($path, '?') !== FALSE ? '&' : '?') . drupal_http_build_query($token_query); - } - - $headers = array( - // Make sure we hit the server and do not end up with a stale - // cached version. - 'Cache-Control' => 'no-cache', - 'Pragma' => 'no-cache', - ); - drupal_http_request($url, array('headers' => $headers)); - - $exists = file_exists($path); - watchdog( - 'cdn', - 'Nested HTTP request to generate %file: %result (URL: %url, time: !time).', - array( - '!time' => (int) $_SERVER['REQUEST_TIME'], - '%file' => $path, - '%url' => $url, - '%result' => $exists ? 'success' : 'failure', - ), - $exists ? WATCHDOG_NOTICE : WATCHDOG_CRITICAL - ); - return $exists; -} - -/** - * file_check_directory() doesn't support creating directory trees. - */ -function _cdn_basic_farfuture_create_directory_structure($path) { - // Create the directory structure in which the file will be stored. Because - // it's nested, file_check_directory() can't do this in one run. - $parts = explode('/', $path); - for ($i = 0; $i < count($parts); $i++) { - $directory = implode('/', array_slice($parts, 0, $i + 1)); - if (!file_exists($directory)) { - file_prepare_directory($directory, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS); - } - } -} diff --git a/cdn.module b/cdn.module index 5caf8db..0558fbb 100644 --- a/cdn.module +++ b/cdn.module @@ -21,47 +21,7 @@ function cdn_file_url_alter(&$uri) { return; } - $result = \Drupal::service('cdn.file_url_generator')->generate($uri); - - if ($result) { - $uri = $result; - } - - return; - // @todo Far Future support - $farfuture = variable_get(CDN_BASIC_FARFUTURE_VARIABLE, CDN_BASIC_FARFUTURE_DEFAULT); - $maintenance_mode = variable_get('maintenance_mode', FALSE); - - if (cdn_status_is_enabled()) { - // Alter the file path when using Origin Pull mode and using that mode's - // Far Future setting. - if ($mode == CDN_MODE_BASIC && $farfuture && !$maintenance_mode) { - cdn_load_include('basic.farfuture'); - // We need the unescaped version of the URI to perform file operations. - $uri = urldecode($uri); - // If the file does not yet exist, perform a normal HTTP request to this - // file, to generate it. (E.g. when ImageCache is used, this will - // generate the derivative file.) When that fails, don't serve it from - // the CDN. - if (!file_exists($uri) && !_cdn_basic_farfuture_generate_file($uri, $original_uri)) { - $path = drupal_encode_path($uri); - return; - } - // Generate a unique file identifier (UFI). - $ufi = cdn_basic_farfuture_get_identifier($uri); - // Now that file operations have been performed, re-encode the URI. - $uri = drupal_encode_path($uri); - // Generate the new path. - $uri_before_farfuture = $uri; - - // Generate a unique token to verify that the request was generated by - // CDN. We cannot use drupal_get_token() since it depends on the user - // session. - $path_info = pathinfo(urldecode($uri)); - $token = drupal_hmac_base64($ufi . $path_info['filename'], drupal_get_private_key() . drupal_get_hash_salt()); - $uri = "cdn/farfuture/$token/$ufi/$uri"; - } - } + return \Drupal::service('cdn.file_url_generator')->generate($uri); } /** @@ -92,26 +52,6 @@ function cdn_cdn_unique_file_identifier_info() { 'filesystem' => FALSE, 'value' => 'forever', ), - 'drupal_version' => array( - 'label' => t('Drupal version'), - 'prefix' => 'drupal', - 'description' => t('Drupal core version — this should only be applied - to files that ship with Drupal core.'), - 'filesystem' => FALSE, - 'value' => VERSION, - ), - 'drupal_cache' => array( - 'label' => t('Drupal cache'), - 'prefix' => 'drupal-cache', - 'description' => t('Uses the current Drupal cache ID - (css_js_query_string). This ID is - updated automatically whenever the Drupal cache is - flushed (e.g. when you submit the modules form). Be - aware that this can change relatively often, forcing - redownloads by your visitors.'), - 'filesystem' => FALSE, - 'value' => variable_get('css_js_query_string', 0), - ), 'deployment_id' => array( 'label' => t('Deployment ID'), 'prefix' => 'deployment', diff --git a/src/CdnController.php b/src/CdnController.php index 029f693..6089d85 100644 --- a/src/CdnController.php +++ b/src/CdnController.php @@ -7,12 +7,34 @@ use Drupal\Core\Site\Settings; use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; class CdnController { + /** + * Serves the requested file with optimal Far Future expiration headers. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. $request->query must have root_relative_file_url, + * set by \Drupal\cdn\PathProcessor\PathProcessorFarFuture. + * @param string $token + * The token. + * @param string $unique_file_identifier + * The unique file identifier. + * + * @returns \Symfony\Component\HttpFoundation\BinaryFileResponse + * The response that will efficiently send the requested file. + * + * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException + * Thrown when the 'root_relative_file_url' query argument is not set, which + * can only happen in case of malicious requests or in case of a malfunction + * in \Drupal\cdn\PathProcessor\PathProcessorFarFuture. + * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException + * Thrown when an invalid token is provided. + */ public function farfuture(Request $request, $token, $unique_file_identifier) { if (!$request->query->has('root_relative_file_url')) { - throw new AccessDeniedHttpException(); + throw new BadRequestHttpException(); } $root_relative_file_url = $request->query->get('root_relative_file_url'); -- 2.9.0 From 77497227d1cdc53ae13eb57576256ae322f8357c Mon Sep 17 00:00:00 2001 From: Wim Leers Date: Wed, 14 Sep 2016 11:15:28 +0200 Subject: [PATCH 5/9] introduce CdnCondition plugins --- cdn.module | 8 --- cdn_ui/src/Form/CdnSettingsForm.php | 1 + config/install/cdn.settings.yml | 18 +++---- src/Annotation/CdnCondition.php | 57 ++++++++++++++++++++++ src/CdnConditionManager.php | 39 +++++++++++++++ src/Plugin/CdnCondition/CdnConditionInterface.php | 16 ++++++ src/Plugin/CdnCondition/Directory.php | 29 +++++++++++ src/Plugin/CdnCondition/Extension.php | 29 +++++++++++ src/Plugin/CdnCondition/MediaType.php | 30 ++++++++++++ src/Plugin/CdnCondition/MimeType.php | 29 +++++++++++ .../DeploymentIdentifier.php | 34 +++++++++++++ src/Plugin/CdnUniqueFileIdentifier/Perpetual.php | 21 ++++++++ 12 files changed, 294 insertions(+), 17 deletions(-) create mode 100644 src/Annotation/CdnCondition.php create mode 100644 src/CdnConditionManager.php create mode 100644 src/Plugin/CdnCondition/CdnConditionInterface.php create mode 100644 src/Plugin/CdnCondition/Directory.php create mode 100644 src/Plugin/CdnCondition/Extension.php create mode 100644 src/Plugin/CdnCondition/MediaType.php create mode 100644 src/Plugin/CdnCondition/MimeType.php create mode 100644 src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php create mode 100644 src/Plugin/CdnUniqueFileIdentifier/Perpetual.php diff --git a/cdn.module b/cdn.module index 0558fbb..229334b 100644 --- a/cdn.module +++ b/cdn.module @@ -44,14 +44,6 @@ function cdn_cdn_unique_file_identifier_info() { 'filesystem' => TRUE, 'callback' => 'filemtime', ), - 'perpetual' => array( - 'label' => t('Perpetual'), - 'prefix' => 'perpetual', - 'description' => t('Perpetual files never change (or are never cached - by the browser, e.g. video files).'), - 'filesystem' => FALSE, - 'value' => 'forever', - ), 'deployment_id' => array( 'label' => t('Deployment ID'), 'prefix' => 'deployment', diff --git a/cdn_ui/src/Form/CdnSettingsForm.php b/cdn_ui/src/Form/CdnSettingsForm.php index af0f777..58f03df 100644 --- a/cdn_ui/src/Form/CdnSettingsForm.php +++ b/cdn_ui/src/Form/CdnSettingsForm.php @@ -8,6 +8,7 @@ namespace Drupal\cdn_ui\Form; use Drupal\Core\Form\ConfigFormBase; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Site\Settings; use Drupal\Core\Url; /** diff --git a/config/install/cdn.settings.yml b/config/install/cdn.settings.yml index 7e8c153..c77c8a7 100644 --- a/config/install/cdn.settings.yml +++ b/config/install/cdn.settings.yml @@ -18,7 +18,7 @@ mapping: # type: simple # domain: cdn-a.com # conditions: -# extensions: [jpg, jpeg, png] +# extension: [jpg, jpeg, png] # Serve CSS & image files from CDN A, downloads from B, everything else from C: # @@ -30,12 +30,12 @@ mapping: # type: simple # domain: cdn-a.com # conditions: -# extensions: [css, jpg, jpeg, png] +# extension: [css, jpg, jpeg, png] # - # type: simple # domain: cdn-b.com # conditions: -# extensions: [zip] +# extension: [zip] # Serve CSS & JS files from CDN A, images from either B or C and nothing else: # @@ -47,14 +47,14 @@ mapping: # type: simple # domain: cdn-a.com # conditions: -# extensions: [css, js] +# extension: [css, js] # - # type: auto-balanced # domains: # - cdn-b.com # - cdn-c.com # conditions: -# extensions: [jpg, jpeg, png] +# extension: [jpg, jpeg, png] farfuture: status: true @@ -63,7 +63,7 @@ farfuture: # Rationale: they cannot change, unless in a deployment. - conditions: - directories: + directory: - core - modules - profiles @@ -75,7 +75,7 @@ farfuture: # Rationale: they may be modified. - conditions: - directories: + directory: - files - sites/*/files uniqueness: mtime @@ -83,10 +83,10 @@ farfuture: # Rationale: they are extremely unlikely to be modified. - conditions: - directories: + directory: - files - sites/*/files - mediatypes: + media_type: - audio - video uniqueness: perpetual diff --git a/src/Annotation/CdnCondition.php b/src/Annotation/CdnCondition.php new file mode 100644 index 0000000..d579f3e --- /dev/null +++ b/src/Annotation/CdnCondition.php @@ -0,0 +1,57 @@ +setCacheBackend($cache_backend, 'cdn_conditions'); + } + +} diff --git a/src/Plugin/CdnCondition/CdnConditionInterface.php b/src/Plugin/CdnCondition/CdnConditionInterface.php new file mode 100644 index 0000000..a1bf7e9 --- /dev/null +++ b/src/Plugin/CdnCondition/CdnConditionInterface.php @@ -0,0 +1,16 @@ +data-align attribute on <img> tags to align images.") + * ) + */ +class Directory { + + /** + * {@inheritdoc} + */ + public function matches($root_relative_file_url, array $conditions) { + $file_extension = Unicode::strtolower(pathinfo($root_relative_file_url, PATHINFO_EXTENSION)); + return in_array($file_extension, $conditions, TRUE); + } + +} diff --git a/src/Plugin/CdnCondition/Extension.php b/src/Plugin/CdnCondition/Extension.php new file mode 100644 index 0000000..2d33636 --- /dev/null +++ b/src/Plugin/CdnCondition/Extension.php @@ -0,0 +1,29 @@ +data-align attribute on <img> tags to align images.") + * ) + */ +class Extension { + + /** + * {@inheritdoc} + */ + public function matches($root_relative_file_url, array $conditions) { + $file_extension = Unicode::strtolower(pathinfo($root_relative_file_url, PATHINFO_EXTENSION)); + return in_array($file_extension, $conditions, TRUE); + } + +} diff --git a/src/Plugin/CdnCondition/MediaType.php b/src/Plugin/CdnCondition/MediaType.php new file mode 100644 index 0000000..b6475df --- /dev/null +++ b/src/Plugin/CdnCondition/MediaType.php @@ -0,0 +1,30 @@ +data-align attribute on <img> tags to align images.") + * ) + */ +class MediaType { + + /** + * {@inheritdoc} + */ + public function matches($root_relative_file_url, array $conditions) { + $mime_type = \Drupal::service('file.mime_type.guesser')->guess($root_relative_file_url); + $media_type = explode('/', $mime_type, 1); + return in_array($media_type, $conditions, TRUE); + } + +} diff --git a/src/Plugin/CdnCondition/MimeType.php b/src/Plugin/CdnCondition/MimeType.php new file mode 100644 index 0000000..67bd964 --- /dev/null +++ b/src/Plugin/CdnCondition/MimeType.php @@ -0,0 +1,29 @@ +data-align attribute on <img> tags to align images.") + * ) + */ +class MimeType { + + /** + * {@inheritdoc} + */ + public function matches($root_relative_file_url, array $conditions) { + $mime_type = \Drupal::service('file.mime_type.guesser')->guess($root_relative_file_url); + return in_array($mime_type, $conditions, TRUE); + } + +} diff --git a/src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php b/src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php new file mode 100644 index 0000000..80038d3 --- /dev/null +++ b/src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php @@ -0,0 +1,34 @@ + t('A developer-defined deployment ID. Can be an +arbitrary string or number, as long as it uniquely +identifies deployments and therefore the affected +files.
+Define this deployment ID in any enabled module or +in settings.php as the +CDN_DEPLOYMENT_ID +constant, and it will be picked up instantaneously.'), + + * ) + */ +class MimeType { + + /** + * {@inheritdoc} + */ + public function getUniqueFileIdentifier($root_relative_file_url) { + return Settings::get('deployment_identifier'); + } + +} diff --git a/src/Plugin/CdnUniqueFileIdentifier/Perpetual.php b/src/Plugin/CdnUniqueFileIdentifier/Perpetual.php new file mode 100644 index 0000000..5dfc4e2 --- /dev/null +++ b/src/Plugin/CdnUniqueFileIdentifier/Perpetual.php @@ -0,0 +1,21 @@ + Date: Wed, 14 Sep 2016 11:40:43 +0200 Subject: [PATCH 6/9] Remove CdnCondition plugins, remove Far Future expiration mapping altogether: always use mtime. --- cdn.basic.farfuture.inc | 153 --------------------- cdn.module | 38 +---- cdn.routing.yml | 3 +- config/install/cdn.settings.yml | 32 ----- src/Annotation/CdnCondition.php | 57 -------- src/CdnController.php | 10 +- src/File/FileUrlGenerator.php | 8 +- src/Plugin/CdnCondition/CdnConditionInterface.php | 16 --- src/Plugin/CdnCondition/Directory.php | 29 ---- src/Plugin/CdnCondition/Extension.php | 29 ---- src/Plugin/CdnCondition/MediaType.php | 30 ---- src/Plugin/CdnCondition/MimeType.php | 29 ---- .../DeploymentIdentifier.php | 34 ----- src/Plugin/CdnUniqueFileIdentifier/Perpetual.php | 21 --- 14 files changed, 13 insertions(+), 476 deletions(-) delete mode 100644 cdn.basic.farfuture.inc delete mode 100644 src/Annotation/CdnCondition.php delete mode 100644 src/Plugin/CdnCondition/CdnConditionInterface.php delete mode 100644 src/Plugin/CdnCondition/Directory.php delete mode 100644 src/Plugin/CdnCondition/Extension.php delete mode 100644 src/Plugin/CdnCondition/MediaType.php delete mode 100644 src/Plugin/CdnCondition/MimeType.php delete mode 100644 src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php delete mode 100644 src/Plugin/CdnUniqueFileIdentifier/Perpetual.php diff --git a/cdn.basic.farfuture.inc b/cdn.basic.farfuture.inc deleted file mode 100644 index 9a9707f..0000000 --- a/cdn.basic.farfuture.inc +++ /dev/null @@ -1,153 +0,0 @@ - $current_specificity) { - $ufi_method = $mapping[$directory][$extension]['ufi method']; - $current_specificity = $specificity; - } - } - } - } - - // Fall back to the default UFI method in case no UFI method is defined by - // the user. - if ($ufi_method === FALSE) { - $ufi_method = CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_DEFAULT; - } - - return $ufi_method; -} - -/** - * Get the UFI (Unique File Identifier) for the file at a path. - * - * @param $path - * The path to get a UFI for. - */ -function cdn_basic_farfuture_get_identifier($path) { - static $ufi_info; - static $mapping; - - // Gather all unique file identifier info. - if (!isset($ufi_info)) { - $ufi_info = module_invoke_all('cdn_unique_file_identifier_info'); - } - - // We only need to parse the textual CDN mapping once into a lookup table. - if (!isset($mapping)) { - $mapping = _cdn_basic_farfuture_parse_raw_mapping(variable_get(CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE, CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_DEFAULT)); - } - - $ufi_method = cdn_basic_farfuture_get_ufi_method($path, $mapping); - - $prefix = $ufi_info[$ufi_method]['prefix']; - if (isset($ufi_info[$ufi_method]['value'])) { - $value = $ufi_info[$ufi_method]['value']; - } - else { - $callback = $ufi_info[$ufi_method]['callback']; - $value = call_user_func_array($callback, array($path)); - } - - return "$prefix:$value"; -} - - -//---------------------------------------------------------------------------- -// Private functions. - -/** - * Parse the raw (textual) mapping into a lookup table, where the key is the - * file extension and the value is a list of CDN URLs that serve the file. - * - * @param $mapping_raw - * A raw (textual) mapping. - * @return - * The corresponding mapping lookup table. - */ -function _cdn_basic_farfuture_parse_raw_mapping($mapping_raw) { - $mapping = array(); - - if (!empty($mapping_raw)) { - $lines = preg_split("/[\n\r]+/", $mapping_raw, -1, PREG_SPLIT_NO_EMPTY); - foreach ($lines as $line) { - // Parse this line. It may or may not limit the CDN URL to a list of - // file extensions. - $parts = explode('|', $line); - $directories = explode(':', $parts[0]); - $specificity = 0; - - // There may be 2 or 3 parts: - // - part 1: directories - // - part 2: file extensions (optional) - // - part 3: unique file identifier method - if (count($parts) == 2) { - $extensions = array('*'); // Use the asterisk as a wildcard. - $ufi_method = drupal_strtolower(trim($parts[1])); - } - elseif (count($parts) == 3) { - // Convert to lower case, remove periods, whitespace and split on ' '. - $extensions = explode(' ', trim(str_replace('.', '', drupal_strtolower($parts[1])))); - $ufi_method = drupal_strtolower(trim($parts[2])); - } - - // Create the mapping lookup table. - foreach ($directories as $directory) { - $directory_specificity = 10 * count(explode('/', $directory)); - foreach ($extensions as $extension) { - $extension_specificity = ($extension == '*') ? 0 : 1; - - $mapping[$directory][$extension] = array( - 'ufi method' => $ufi_method, - 'specificity' => $directory_specificity + $extension_specificity, - ); - } - } - } - } - - return $mapping; -} - diff --git a/cdn.module b/cdn.module index 229334b..c33d97d 100644 --- a/cdn.module +++ b/cdn.module @@ -21,45 +21,9 @@ function cdn_file_url_alter(&$uri) { return; } - return \Drupal::service('cdn.file_url_generator')->generate($uri); + $uri = \Drupal::service('cdn.file_url_generator')->generate($uri); } -/** - * Implementation of hook_cdn_unique_file_identifier_info(). - */ -function cdn_cdn_unique_file_identifier_info() { - // Keys are machine names. - return array( - 'md5_hash' => array( - 'label' => t('MD5 hash'), - 'prefix' => 'md5', - 'description' => t('MD5 hash of the file.'), - 'filesystem' => TRUE, - 'callback' => 'md5_file', - ), - 'mtime' => array( - 'label' => t('Last modification time'), - 'prefix' => 'mtime', - 'description' => t('Last modification time of the file.'), - 'filesystem' => TRUE, - 'callback' => 'filemtime', - ), - 'deployment_id' => array( - 'label' => t('Deployment ID'), - 'prefix' => 'deployment', - 'description' => t('A developer-defined deployment ID. Can be an - arbitrary string or number, as long as it uniquely - identifies deployments and therefore the affected - files.
- Define this deployment ID in any enabled module or - in settings.php as the - CDN_DEPLOYMENT_ID - constant, and it will be picked up instantaneously.'), - 'filesystem' => FALSE, - 'callback' => '_cdn_ufi_deployment_id', - ), - ); -} //---------------------------------------------------------------------------- // Public functions. diff --git a/cdn.routing.yml b/cdn.routing.yml index 51561d2..f6b5678 100644 --- a/cdn.routing.yml +++ b/cdn.routing.yml @@ -1,6 +1,7 @@ cdn.farfuture: - path: '/cdn/farfuture/{token}/{unique_file_identifier}' + path: '/cdn/farfuture/{token}/{mtime}' defaults: _controller: '\Drupal\cdn\CdnController::farfuture' requirements: _access: 'TRUE' + mtime: \d+ diff --git a/config/install/cdn.settings.yml b/config/install/cdn.settings.yml index c77c8a7..766480c 100644 --- a/config/install/cdn.settings.yml +++ b/config/install/cdn.settings.yml @@ -58,35 +58,3 @@ mapping: farfuture: status: true - rules: - # Shipped files: deployment identifier. - # Rationale: they cannot change, unless in a deployment. - - - conditions: - directory: - - core - - modules - - profiles - - themes - - sites/*/modules - - sites/*/themes - uniqueness: deployment_identifier - # Uploaded and generated files in general: mtime. - # Rationale: they may be modified. - - - conditions: - directory: - - files - - sites/*/files - uniqueness: mtime - # Uploaded audio and video files in specific: perpetual. - # Rationale: they are extremely unlikely to be modified. - - - conditions: - directory: - - files - - sites/*/files - media_type: - - audio - - video - uniqueness: perpetual diff --git a/src/Annotation/CdnCondition.php b/src/Annotation/CdnCondition.php deleted file mode 100644 index d579f3e..0000000 --- a/src/Annotation/CdnCondition.php +++ /dev/null @@ -1,57 +0,0 @@ -query must have root_relative_file_url, * set by \Drupal\cdn\PathProcessor\PathProcessorFarFuture. * @param string $token - * The token. - * @param string $unique_file_identifier - * The unique file identifier. + * The token. Ensures that users cannot simply request just any file. + * @param int $mtime + * The file's mtime. * * @returns \Symfony\Component\HttpFoundation\BinaryFileResponse * The response that will efficiently send the requested file. @@ -32,13 +32,13 @@ class CdnController { * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * Thrown when an invalid token is provided. */ - public function farfuture(Request $request, $token, $unique_file_identifier) { + public function farfuture(Request $request, $token, $mtime) { if (!$request->query->has('root_relative_file_url')) { throw new BadRequestHttpException(); } $root_relative_file_url = $request->query->get('root_relative_file_url'); - $calculated_token = Crypt::hmacBase64($unique_file_identifier . $root_relative_file_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); + $calculated_token = Crypt::hmacBase64($mtime . $root_relative_file_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); if ($token !== $calculated_token) { throw new AccessDeniedHttpException('Invalid token.'); } diff --git a/src/File/FileUrlGenerator.php b/src/File/FileUrlGenerator.php index ce163d0..dc4cb05 100644 --- a/src/File/FileUrlGenerator.php +++ b/src/File/FileUrlGenerator.php @@ -123,14 +123,16 @@ class FileUrlGenerator { $cdn_domain = $result; } + // When Far Future expiration is enabled, rewrite the file URL to let Drupal + // serve the file with optimal headers. Only possible if the file exists. if ($this->settings->farfutureIsEnabled() && file_exists($uri)) { - $unique_file_identifier = 'perpetual'; + $mtime = filemtime($uri); // Generate a unique token to verify that the request was generated by // CDN. We cannot use drupal_get_token() since it depends on the user // session. - $calculated_token = Crypt::hmacBase64($unique_file_identifier . $root_relative_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); - return '//' . $cdn_domain . '/cdn/farfuture/' . $calculated_token . '/' . $unique_file_identifier . $root_relative_url; + $calculated_token = Crypt::hmacBase64($mtime . $root_relative_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); + return '//' . $cdn_domain . '/cdn/farfuture/' . $calculated_token . '/' . $mtime . $root_relative_url; } return '//' . $cdn_domain . $root_relative_url; diff --git a/src/Plugin/CdnCondition/CdnConditionInterface.php b/src/Plugin/CdnCondition/CdnConditionInterface.php deleted file mode 100644 index a1bf7e9..0000000 --- a/src/Plugin/CdnCondition/CdnConditionInterface.php +++ /dev/null @@ -1,16 +0,0 @@ -data-align attribute on <img> tags to align images.") - * ) - */ -class Directory { - - /** - * {@inheritdoc} - */ - public function matches($root_relative_file_url, array $conditions) { - $file_extension = Unicode::strtolower(pathinfo($root_relative_file_url, PATHINFO_EXTENSION)); - return in_array($file_extension, $conditions, TRUE); - } - -} diff --git a/src/Plugin/CdnCondition/Extension.php b/src/Plugin/CdnCondition/Extension.php deleted file mode 100644 index 2d33636..0000000 --- a/src/Plugin/CdnCondition/Extension.php +++ /dev/null @@ -1,29 +0,0 @@ -data-align attribute on <img> tags to align images.") - * ) - */ -class Extension { - - /** - * {@inheritdoc} - */ - public function matches($root_relative_file_url, array $conditions) { - $file_extension = Unicode::strtolower(pathinfo($root_relative_file_url, PATHINFO_EXTENSION)); - return in_array($file_extension, $conditions, TRUE); - } - -} diff --git a/src/Plugin/CdnCondition/MediaType.php b/src/Plugin/CdnCondition/MediaType.php deleted file mode 100644 index b6475df..0000000 --- a/src/Plugin/CdnCondition/MediaType.php +++ /dev/null @@ -1,30 +0,0 @@ -data-align attribute on <img> tags to align images.") - * ) - */ -class MediaType { - - /** - * {@inheritdoc} - */ - public function matches($root_relative_file_url, array $conditions) { - $mime_type = \Drupal::service('file.mime_type.guesser')->guess($root_relative_file_url); - $media_type = explode('/', $mime_type, 1); - return in_array($media_type, $conditions, TRUE); - } - -} diff --git a/src/Plugin/CdnCondition/MimeType.php b/src/Plugin/CdnCondition/MimeType.php deleted file mode 100644 index 67bd964..0000000 --- a/src/Plugin/CdnCondition/MimeType.php +++ /dev/null @@ -1,29 +0,0 @@ -data-align attribute on <img> tags to align images.") - * ) - */ -class MimeType { - - /** - * {@inheritdoc} - */ - public function matches($root_relative_file_url, array $conditions) { - $mime_type = \Drupal::service('file.mime_type.guesser')->guess($root_relative_file_url); - return in_array($mime_type, $conditions, TRUE); - } - -} diff --git a/src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php b/src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php deleted file mode 100644 index 80038d3..0000000 --- a/src/Plugin/CdnUniqueFileIdentifier/DeploymentIdentifier.php +++ /dev/null @@ -1,34 +0,0 @@ - t('A developer-defined deployment ID. Can be an -arbitrary string or number, as long as it uniquely -identifies deployments and therefore the affected -files.
-Define this deployment ID in any enabled module or -in settings.php as the -CDN_DEPLOYMENT_ID -constant, and it will be picked up instantaneously.'), - - * ) - */ -class MimeType { - - /** - * {@inheritdoc} - */ - public function getUniqueFileIdentifier($root_relative_file_url) { - return Settings::get('deployment_identifier'); - } - -} diff --git a/src/Plugin/CdnUniqueFileIdentifier/Perpetual.php b/src/Plugin/CdnUniqueFileIdentifier/Perpetual.php deleted file mode 100644 index 5dfc4e2..0000000 --- a/src/Plugin/CdnUniqueFileIdentifier/Perpetual.php +++ /dev/null @@ -1,21 +0,0 @@ - Date: Wed, 14 Sep 2016 14:27:34 +0200 Subject: [PATCH 7/9] clean-up + test coverage --- cdn.routing.yml | 2 +- cdn.services.yml | 4 +- config/install/cdn.settings.yml | 10 +- config/schema/cdn.conditions.schema.yml | 26 ----- config/schema/cdn.data_types.schema.yml | 6 - config/schema/cdn.schema.yml | 11 -- help/admin-details-mode-pull-far-future.html | 4 - src/CdnConditionManager.php | 39 ------- src/CdnController.php | 47 ++++++-- src/EventSubscriber/ConfigSubscriber.php | 34 +++++- src/File/FileUrlGenerator.php | 41 +++++-- tests/cdn.test | 161 --------------------------- tests/src/Unit/File/FileUrlGeneratorTest.php | 100 ++++++++++++++++- 13 files changed, 210 insertions(+), 275 deletions(-) delete mode 100644 help/admin-details-mode-pull-far-future.html delete mode 100644 src/CdnConditionManager.php diff --git a/cdn.routing.yml b/cdn.routing.yml index f6b5678..50ac3d1 100644 --- a/cdn.routing.yml +++ b/cdn.routing.yml @@ -1,5 +1,5 @@ cdn.farfuture: - path: '/cdn/farfuture/{token}/{mtime}' + path: '/cdn/farfuture/{security_token}/{mtime}' defaults: _controller: '\Drupal\cdn\CdnController::farfuture' requirements: diff --git a/cdn.services.yml b/cdn.services.yml index d796600..4b8411e 100644 --- a/cdn.services.yml +++ b/cdn.services.yml @@ -5,12 +5,12 @@ services: cdn.file_url_generator: class: Drupal\cdn\File\FileUrlGenerator - arguments: ['@file_system', '@stream_wrapper_manager', '@request_stack', '@cdn.settings'] + arguments: ['@app.root', '@file_system', '@stream_wrapper_manager', '@request_stack', '@private_key', '@cdn.settings'] # Event subscribers. cdn.config_subscriber: class: Drupal\cdn\EventSubscriber\ConfigSubscriber - arguments: ['@cache_tags.invalidator', '@kernel'] + arguments: ['@cache_tags.invalidator', '@kernel', '@config.factory'] tags: - { name: event_subscriber } cdn.html_response_subscriber: diff --git a/config/install/cdn.settings.yml b/config/install/cdn.settings.yml index 766480c..c4ece55 100644 --- a/config/install/cdn.settings.yml +++ b/config/install/cdn.settings.yml @@ -18,7 +18,7 @@ mapping: # type: simple # domain: cdn-a.com # conditions: -# extension: [jpg, jpeg, png] +# extensions: [jpg, jpeg, png] # Serve CSS & image files from CDN A, downloads from B, everything else from C: # @@ -30,12 +30,12 @@ mapping: # type: simple # domain: cdn-a.com # conditions: -# extension: [css, jpg, jpeg, png] +# extensions: [css, jpg, jpeg, png] # - # type: simple # domain: cdn-b.com # conditions: -# extension: [zip] +# extensions: [zip] # Serve CSS & JS files from CDN A, images from either B or C and nothing else: # @@ -47,14 +47,14 @@ mapping: # type: simple # domain: cdn-a.com # conditions: -# extension: [css, js] +# extensions: [css, js] # - # type: auto-balanced # domains: # - cdn-b.com # - cdn-c.com # conditions: -# extension: [jpg, jpeg, png] +# extensions: [jpg, jpeg, png] farfuture: status: true diff --git a/config/schema/cdn.conditions.schema.yml b/config/schema/cdn.conditions.schema.yml index 6b1dad4..2e1780f 100644 --- a/config/schema/cdn.conditions.schema.yml +++ b/config/schema/cdn.conditions.schema.yml @@ -10,29 +10,3 @@ cdn.condition.extensions: sequence: type: string label: 'Allowed file extension' - -cdn.condition.directories: - type: sequence - label: 'Allowed directories' - sequence: - type: string - label: 'Allowed directory' - -# One of 'application', 'audio', 'example', 'image', 'message', 'model', -# 'multipart', 'text' or 'video'. -# See http://www.iana.org/assignments/media-types/media-types.xhtml. -cdn.condition.mediatypes: - type: sequence - label: 'Allowed media types' - sequence: - type: string - label: 'Allowed media type' - -# For example: 'application/javascript', 'image/png', 'video/mp4'. -# See http://www.iana.org/assignments/media-types/media-types.xhtml. -cdn.condition.mimetypes: - type: sequence - label: 'Allowed MIME types' - sequence: - type: string - label: 'Allowed MIME type' diff --git a/config/schema/cdn.data_types.schema.yml b/config/schema/cdn.data_types.schema.yml index 29b46e8..e106a86 100644 --- a/config/schema/cdn.data_types.schema.yml +++ b/config/schema/cdn.data_types.schema.yml @@ -3,9 +3,3 @@ cdn.domain: type: string label: 'Domain' - -# A method that identifies the uniqueness of a file. Examples: md5_hash, mtime, -# perpetual, deployment_identifier. -cdn.unique_file_identifier_method: - type: string - label: 'Unique file identifier method' diff --git a/config/schema/cdn.schema.yml b/config/schema/cdn.schema.yml index ab61b67..f64261d 100644 --- a/config/schema/cdn.schema.yml +++ b/config/schema/cdn.schema.yml @@ -19,14 +19,3 @@ cdn.settings: status: label: 'Far Future expiration status' type: boolean - rules: - label: 'Ordered list of unique file identifier generation rules' - type: sequence - sequence: - label: 'Unique file identifier generation rule' - type: mapping - mapping: - conditions: - type: cdn.conditions - uniqueness: - type: cdn.unique_file_identifier_method diff --git a/help/admin-details-mode-pull-far-future.html b/help/admin-details-mode-pull-far-future.html deleted file mode 100644 index c76b7ee..0000000 --- a/help/admin-details-mode-pull-far-future.html +++ /dev/null @@ -1,4 +0,0 @@ -

CSS aggregation

-

It is necessary to enable CSS aggregation, if you don't enable CSS aggregation, files referenced by the CSS (such as images and fonts) files that are served from the CDN will not load. To prevent access to unauthorized files, every "Far Future URL" is signed with a security token.

-

Without CSS aggregation, the URLs in the CSS files continue to be relative, and consquently, these files will be loaded using the same security token. But since the token is based on the filename, this will result in a HTTP 403 response.

-

With CSS aggregation, the URLs in the CSS files will be rewritten. However, Drupal core's CSS aggregation is not very smart either, and it will in fact cause more or less the same problem. That's why the CDN module comes with an override of Drupal core's CSS aggregation, which correctly alters every file URL. As a result, this also enables us to e.g. serve the CSS file from one CDN, the images referenced by it from another and the fonts referenced by it from yet another.

diff --git a/src/CdnConditionManager.php b/src/CdnConditionManager.php deleted file mode 100644 index 165a3f1..0000000 --- a/src/CdnConditionManager.php +++ /dev/null @@ -1,39 +0,0 @@ -setCacheBackend($cache_backend, 'cdn_conditions'); - } - -} diff --git a/src/CdnController.php b/src/CdnController.php index 213fd08..965fc8d 100644 --- a/src/CdnController.php +++ b/src/CdnController.php @@ -3,13 +3,40 @@ namespace Drupal\cdn; use Drupal\Component\Utility\Crypt; +use Drupal\Core\DependencyInjection\ContainerInjectionInterface; +use Drupal\Core\PrivateKey; use Drupal\Core\Site\Settings; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; -class CdnController { +class CdnController implements ContainerInjectionInterface { + + /** + * The private key service. + * + * @var \Drupal\Core\PrivateKey + */ + protected $privateKey; + + /** + * @param \Drupal\Core\PrivateKey $private_key + * The private key service. + */ + public function __construct(PrivateKey $private_key) { + $this->privateKey = $private_key; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('private_key') + ); + } /** * Serves the requested file with optimal Far Future expiration headers. @@ -17,8 +44,10 @@ class CdnController { * @param \Symfony\Component\HttpFoundation\Request $request * The current request. $request->query must have root_relative_file_url, * set by \Drupal\cdn\PathProcessor\PathProcessorFarFuture. - * @param string $token - * The token. Ensures that users cannot simply request just any file. + * @param string $security_token + * The security token. Ensures that users can not request any file they want + * by manipulating the URL (they could otherwise request settings.php for + * example). See https://www.drupal.org/node/1441502. * @param int $mtime * The file's mtime. * @@ -30,17 +59,19 @@ class CdnController { * can only happen in case of malicious requests or in case of a malfunction * in \Drupal\cdn\PathProcessor\PathProcessorFarFuture. * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException - * Thrown when an invalid token is provided. + * Thrown when an invalid security token is provided. */ - public function farfuture(Request $request, $token, $mtime) { + public function farfuture(Request $request, $security_token, $mtime) { + // Ensure \Drupal\cdn\PathProcessor\PathProcessorFarFuture did its job. if (!$request->query->has('root_relative_file_url')) { throw new BadRequestHttpException(); } + // Validate security token. $root_relative_file_url = $request->query->get('root_relative_file_url'); - $calculated_token = Crypt::hmacBase64($mtime . $root_relative_file_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); - if ($token !== $calculated_token) { - throw new AccessDeniedHttpException('Invalid token.'); + $calculated_token = Crypt::hmacBase64($mtime . $root_relative_file_url, $this->privateKey->get() . Settings::getHashSalt()); + if ($security_token !== $calculated_token) { + throw new AccessDeniedHttpException('Invalid security token.'); } $farfuture_headers = [ diff --git a/src/EventSubscriber/ConfigSubscriber.php b/src/EventSubscriber/ConfigSubscriber.php index fa0bbd3..c97483c 100644 --- a/src/EventSubscriber/ConfigSubscriber.php +++ b/src/EventSubscriber/ConfigSubscriber.php @@ -3,17 +3,22 @@ namespace Drupal\cdn\EventSubscriber; use Drupal\Core\Cache\CacheTagsInvalidatorInterface; +use Drupal\Core\Config\Config; use Drupal\Core\Config\ConfigCrudEvent; use Drupal\Core\Config\ConfigEvents; +use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\DrupalKernelInterface; +use Drupal\Core\StringTranslation\StringTranslationTrait; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; /** - * A subscriber invalidating cache tags when CDN config is saved. + * Invalidates cache tags & enables CSS aggregation when CDN config is saved. */ class ConfigSubscriber implements EventSubscriberInterface { + use StringTranslationTrait; + /** * The cache tags invalidator. * @@ -28,17 +33,27 @@ class ConfigSubscriber implements EventSubscriberInterface { */ protected $drupalKernel; + /** + * The config factory. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; + /** * Constructs a ConfigSubscriber object. * * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tags_invalidator * The cache tags invalidator. - * @param @var \Drupal\Core\DrupalKernelInterface $drupal_kernel + * @param \Drupal\Core\DrupalKernelInterface $drupal_kernel * The Drupal kernel. + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * A config factory for retrieving config objects. */ - public function __construct(CacheTagsInvalidatorInterface $cache_tags_invalidator, DrupalKernelInterface $drupal_kernel) { + public function __construct(CacheTagsInvalidatorInterface $cache_tags_invalidator, DrupalKernelInterface $drupal_kernel, ConfigFactoryInterface $config_factory) { $this->cacheTagsInvalidator = $cache_tags_invalidator; $this->drupalKernel = $drupal_kernel; + $this->configFactory = $config_factory; } /** @@ -49,6 +64,19 @@ class ConfigSubscriber implements EventSubscriberInterface { */ public function onSave(ConfigCrudEvent $event) { if ($event->getConfig()->getName() === 'cdn.settings') { + // If Far Future expiration was just enabled, then we must enable CSS + // aggregation. Otherwise files referenced by the CSS (images, fonts …) + // will not load, because they will reuse the security token of the + // referencing CSS file. By enabling CSS aggregation, we make it possible + // to run all those referenced files through file_create_url() as well, + // hence giving them their own security tokens. + if ($event->getConfig()->get('farfuture.status') === TRUE && $event->isChanged('farfuture.status') && !$this->configFactory->get('system.performance')->get('css.preprocess')) { + $this->configFactory->getEditable('system.performance') + ->set('css.preprocess', TRUE) + ->save(); + drupal_set_message($this->t('Automatically enabled CSS aggregation — necessary for Far Future expiration support.'), 'warning'); + } + $this->cacheTagsInvalidator->invalidateTags([ // Rendered output that is cached. (HTML containing URLs.) 'rendered', diff --git a/src/File/FileUrlGenerator.php b/src/File/FileUrlGenerator.php index dc4cb05..15cc216 100644 --- a/src/File/FileUrlGenerator.php +++ b/src/File/FileUrlGenerator.php @@ -6,6 +6,7 @@ use Drupal\cdn\CdnSettings; use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Unicode; use Drupal\Core\File\FileSystemInterface; +use Drupal\Core\PrivateKey; use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\StreamWrapperInterface; use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; @@ -18,6 +19,13 @@ use Symfony\Component\HttpFoundation\RequestStack; */ class FileUrlGenerator { + /** + * The app root. + * + * @var string + */ + protected $root; + /** * The file system service. * @@ -39,6 +47,13 @@ class FileUrlGenerator { */ protected $requestStack; + /** + * The private key service. + * + * @var \Drupal\Core\PrivateKey + */ + protected $privateKey; + /** * The CDN settings service. * @@ -49,19 +64,25 @@ class FileUrlGenerator { /** * Constructs a new CDN file URL generator object. * + * @param string $root + * The app root. * @param \Drupal\Core\File\FileSystemInterface * The file system service. * @param \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface * The stream wrapper manager. * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. + * @param \Drupal\Core\PrivateKey $private_key + * The private key service. * @param \Drupal\cdn\CdnSettings $cdn_settings * The CDN settings service. */ - public function __construct(FileSystemInterface $file_system, StreamWrapperManagerInterface $stream_wrapper_manager, RequestStack $request_stack, CdnSettings $cdn_settings) { + public function __construct($root, FileSystemInterface $file_system, StreamWrapperManagerInterface $stream_wrapper_manager, RequestStack $request_stack, PrivateKey $private_key, CdnSettings $cdn_settings) { + $this->root = $root; $this->fileSystem = $file_system; $this->streamWrapperManager = $stream_wrapper_manager; $this->requestStack = $request_stack; + $this->privateKey = $private_key; $this->settings = $cdn_settings; } @@ -125,13 +146,17 @@ class FileUrlGenerator { // When Far Future expiration is enabled, rewrite the file URL to let Drupal // serve the file with optimal headers. Only possible if the file exists. - if ($this->settings->farfutureIsEnabled() && file_exists($uri)) { - $mtime = filemtime($uri); - - // Generate a unique token to verify that the request was generated by - // CDN. We cannot use drupal_get_token() since it depends on the user - // session. - $calculated_token = Crypt::hmacBase64($mtime . $root_relative_url, \Drupal::service('private_key')->get() . Settings::getHashSalt()); + $absolute_file_path = $this->root . $root_relative_url; + if ($this->settings->farfutureIsEnabled() && file_exists($absolute_file_path)) { + // We do the filemtime() call separately, because a failed filemtime() + // will cause a PHP warning to be written to the log, which would remove + // any performance gain achieved by removing the file_exists() call. + $mtime = filemtime($absolute_file_path); + + // Generate a security token. Ensures that users can not request any file + // they want by manipulating the URL (they could otherwise request + // settings.php for example). See https://www.drupal.org/node/1441502. + $calculated_token = Crypt::hmacBase64($mtime . $root_relative_url, $this->privateKey->get() . Settings::getHashSalt()); return '//' . $cdn_domain . '/cdn/farfuture/' . $calculated_token . '/' . $mtime . $root_relative_url; } diff --git a/tests/cdn.test b/tests/cdn.test index f787076..ecbca9f 100644 --- a/tests/cdn.test +++ b/tests/cdn.test @@ -202,164 +202,3 @@ class CDNGeneralTestCase extends CDNUnitTestCase { $this->assertEqual(TRUE, cdn_request_is_https(), 'HTTPS request detected.'); } } - -class CDNOriginPullFarFutureTestCase extends CDNUnitTestCase { - public static function getInfo() { - return array( - 'name' => 'Origin Pull mode — Far Future expiration', - 'description' => 'Verify Origin Pull mode\'s Far Future expiration functionality.', - 'group' => 'CDN', - ); - } - - function setUp() { - parent::setUp(); - $this->loadFile('cdn.basic.inc'); - $this->loadFile('cdn.basic.farfuture.inc'); - $this->variableSet(CDN_MODE_VARIABLE, CDN_MODE_BASIC); - $this->variableSet(CDN_BASIC_FARFUTURE_VARIABLE, TRUE); - } - - /** - * Assert a UFI mapping (and optionally set a mapping). - * - * @param $mapping - * The mapping to set; if FALSE, no new mapping will be set. - * @param $parsed_reference - * The reference the parsed mapping will be compared to. - * @param $message - */ - function assertUFIMapping($mapping, $parsed_reference, $message = NULL) { - if (is_null($message)) { - $message = 'UFI mapping parsed correctly.'; - } - - if ($mapping !== FALSE) { - $this->variableSet(CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE, $mapping); - } - $parsed = _cdn_basic_farfuture_parse_raw_mapping(variable_get(CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE, CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_DEFAULT)); - $this->assertEqual($parsed_reference, $parsed, $message); - } - - /** - * Assert a UFI method. Must be run after a UFI mapping is asserted (and - * set) by assertUFIMapping(). - * - * @param $path - * The path to get a UFI for. - * @param $ufi_method_reference - * The reference the resulting UFI method will be compared to. - */ - function assertUFIMethod($path, $ufi_method_reference) { - $ufi_mapping = _cdn_basic_farfuture_parse_raw_mapping(variable_get(CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE, CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_DEFAULT)); - $this->assertEqual( - $ufi_method_reference, - cdn_basic_farfuture_get_ufi_method($path, $ufi_mapping), - 'Correct UFI method applied.' - ); - } - - function testUFIMapping() { - $default = CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_DEFAULT; - $parsed_mapping = _cdn_basic_farfuture_parse_raw_mapping($default); - $this->assertUFIMapping( - FALSE, - $parsed_mapping, - 'The default UFI mapping is set to sensible defaults.' - ); - - // Growing complexity for a single-directory UFI. - $this->assertUFIMapping( - "sites/*|mtime", - array( - 'sites/*' => array( - '*' => array( - 'ufi method' => 'mtime', - 'specificity' => 20, // two directories (2*10), no extension (0) - ), - ), - ), - 'Simple single-directory UFI mapping (step 1).' - ); - $this->assertUFIMethod('sites/foo', 'mtime'); - $this->assertUFIMapping( - "sites/*|mtime\nsites/*|.woff .ttf|md5_hash", - array( - 'sites/*' => array( - '*' => array( - 'ufi method' => 'mtime', - 'specificity' => 20, // two directories (2*10), no extension (0) - ), - 'woff' => array( - 'ufi method' => 'md5_hash', - 'specificity' => 21, // two directories (2*10), one extension (1) - ), - 'ttf' => array( - 'ufi method' => 'md5_hash', - 'specificity' => 21, // two directories (2*10), one extension (1) - ), - ), - ), - 'Simple single-directory UFI mapping (step 2).' - ); - $this->assertUFIMethod('sites/foo', 'mtime'); - $this->assertUFIMethod('sites/foobambamb.woff', 'md5_hash'); - $this->assertUFIMethod('sites/foo/bar/baz.ttf', 'md5_hash'); - $this->assertUFIMapping( - "sites/*|mtime\nsites/*|.woff .ttf|md5_hash\nsites/*|.mov .mp4|perpetual", - array( - 'sites/*' => array( - '*' => array( - 'ufi method' => 'mtime', - 'specificity' => 20, // two directories (2*10), no extension (0) - ), - 'woff' => array( - 'ufi method' => 'md5_hash', - 'specificity' => 21, // two directories (2*10), one extension (1) - ), - 'ttf' => array( - 'ufi method' => 'md5_hash', - 'specificity' => 21, // two directories (2*10), one extension (1) - ), - 'mov' => array( - 'ufi method' => 'perpetual', - 'specificity' => 21, // two directories (2*10), one extension (1) - ), - 'mp4' => array( - 'ufi method' => 'perpetual', - 'specificity' => 21, // two directories (2*10), one extension (1) - ), - ), - ), - 'Simple single-directory UFI mapping (step 2).' - ); - $this->assertUFIMethod('sites/foo', 'mtime'); - $this->assertUFIMethod('sites/foobambamb.woff', 'md5_hash'); - $this->assertUFIMethod('sites/foo/bar/baz.ttf', 'md5_hash'); - $this->assertUFIMethod('sites/movies/foo.mov', 'perpetual'); - $this->assertUFIMethod('sites/movies/bar.mp4', 'perpetual'); - } - - function testFileUrlAlterHook() { - // We don't want to test the UFI functionality here. - $this->variableSet(CDN_BASIC_FARFUTURE_UNIQUE_IDENTIFIER_MAPPING_VARIABLE, '*|perpetual'); - // Provide a very basic CDN mapping. - $this->variableSet(CDN_BASIC_MAPPING_VARIABLE, 'http://cdn-a.com'); - - $filename = 'újjáépítésérol — 100% in B&W.jpg'; - $uri = "public://$filename"; - $this->touchFile($uri); - - cdn_file_url_alter($uri); - - $path_info = pathinfo($filename); - $expected = implode('/', array( - 'http://cdn-a.com' . base_path() . 'cdn/farfuture', - drupal_hmac_base64('perpetual:forever' . $path_info['filename'], drupal_get_private_key() . drupal_get_hash_salt()), - 'perpetual:forever', - variable_get('file_public_path', conf_path() . '/files'), - drupal_encode_path($filename) - )); - $this->assertIdentical($uri, $expected, 'cdn_file_url_alter() works correctly.'); - } -} diff --git a/tests/src/Unit/File/FileUrlGeneratorTest.php b/tests/src/Unit/File/FileUrlGeneratorTest.php index 08f7e82..f65b945 100644 --- a/tests/src/Unit/File/FileUrlGeneratorTest.php +++ b/tests/src/Unit/File/FileUrlGeneratorTest.php @@ -4,7 +4,10 @@ namespace Drupal\Tests\cdn\Unit\File; use Drupal\cdn\CdnSettings; use Drupal\cdn\File\FileUrlGenerator; +use Drupal\Component\Utility\Crypt; +use Drupal\Component\Utility\NestedArray; use Drupal\Core\File\FileSystem; +use Drupal\Core\PrivateKey; use Drupal\Core\Site\Settings; use Drupal\Core\StreamWrapper\PublicStream; use Drupal\Core\StreamWrapper\StreamWrapperInterface; @@ -21,6 +24,20 @@ use Symfony\Component\HttpFoundation\RequestStack; */ class FileUrlGeneratorTest extends UnitTestCase { + static protected $privateKey = 'super secret key that really is just some string'; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + + $settings = [ + 'hash_salt' => $this->randomMachineName(), + ]; + new Settings($settings); + } + /** * @covers ::generate * @dataProvider urlProvider @@ -50,6 +67,9 @@ class FileUrlGeneratorTest extends UnitTestCase { ], ] ], + 'farfuture' => [ + 'status' => FALSE, + ], ], ]); $this->assertSame($expected_result, $gen->generate($uri)); @@ -99,6 +119,28 @@ class FileUrlGeneratorTest extends UnitTestCase { return $cases; } + /** + * @covers ::generate + */ + public function testGenerateFarFuture() { + $gen = $this->createFileUrlGenerator('', [ + 'status' => 2, + 'mapping' => [ + 'type' => 'simple', + 'domain' => 'cdn.example.com', + 'conditions' => [], + ], + 'farfuture' => [ + 'status' => TRUE, + ], + ]); + + $this->assertSame('//cdn.example.com/core/misc/does-not-exist.js', $gen->generate('core/misc/does-not-exist.js')); + $drupal_js_mtime = filemtime($this->root . '/core/misc/drupal.js'); + $drupal_js_calculated_token = Crypt::hmacBase64($drupal_js_mtime. '/core/misc/drupal.js', static::$privateKey . Settings::getHashSalt()); + $this->assertSame('//cdn.example.com/cdn/farfuture/' . $drupal_js_calculated_token . '/' . $drupal_js_mtime . '/core/misc/drupal.js', $gen->generate('core/misc/drupal.js')); + } + /** * Creates a FileUrlGenerator with mostly dummies. * @@ -141,17 +183,73 @@ class FileUrlGeneratorTest extends UnitTestCase { $current_uri = $args[0]; return $s; }); + $private_key = $this->prophesize(PrivateKey::class); + $private_key->get() + ->willReturn(static::$privateKey); return new FileUrlGenerator( + $this->root, new FileSystem( $this->prophesize(StreamWrapperManagerInterface::class)->reveal(), - new Settings([]), + Settings::getInstance(), $this->prophesize(LoggerInterface::class)->reveal() ), $stream_wrapper_manager->reveal(), $request_stack->reveal(), + $private_key->reveal(), new CdnSettings($this->getConfigFactoryStub(['cdn.settings' => $raw_config])) ); } + /** + * {@inheritdoc} + * + * Overridden, because the way ImmutableConfig::get() is mocked, does not + * match the actual implementation, which then causes tests to fail. + */ + public function getConfigFactoryStub(array $configs = array()) { + $config_get_map = array(); + $config_editable_map = array(); + // Construct the desired configuration object stubs, each with its own + // desired return map. + foreach ($configs as $config_name => $map) { + $get = function ($key) use ($map) { + $parts = explode('.', $key); + if (count($parts) == 1) { + return isset($map[$key]) ? $map[$key] : NULL; + } + else { + $value = NestedArray::getValue($map, $parts, $key_exists); + return $key_exists ? $value : NULL; + } + }; + + $immutable_config_object = $this->getMockBuilder('Drupal\Core\Config\ImmutableConfig') + ->disableOriginalConstructor() + ->getMock(); + $immutable_config_object->expects($this->any()) + ->method('get') + ->willReturnCallback($get); + $config_get_map[] = array($config_name, $immutable_config_object); + + $mutable_config_object = $this->getMockBuilder('Drupal\Core\Config\Config') + ->disableOriginalConstructor() + ->getMock(); + $mutable_config_object->expects($this->any()) + ->method('get') + ->willReturnCallback($get); + $config_editable_map[] = array($config_name, $mutable_config_object); + } + // Construct a config factory with the array of configuration object stubs + // as its return map. + $config_factory = $this->getMock('Drupal\Core\Config\ConfigFactoryInterface'); + $config_factory->expects($this->any()) + ->method('get') + ->will($this->returnValueMap($config_get_map)); + $config_factory->expects($this->any()) + ->method('getEditable') + ->will($this->returnValueMap($config_editable_map)); + return $config_factory; + } + } -- 2.9.0 From 2e2b113852df8756cac031028a6386ba0901fa9b Mon Sep 17 00:00:00 2001 From: Wim Leers Date: Thu, 15 Sep 2016 00:39:19 +0200 Subject: [PATCH 8/9] consistently use "farfuture", more clean-up", integration test --- README.txt | 3 +- cdn.routing.yml | 4 +-- cdn.services.yml | 8 +++--- ...dnController.php => CdnFarfutureController.php} | 12 ++++---- ...FarFuture.php => CdnFarfuturePathProcessor.php} | 6 ++-- ...CdnUninstallTest.php => CdnIntegrationTest.php} | 32 +++++++++++++++++++--- tests/src/Unit/File/FileUrlGeneratorTest.php | 2 +- 7 files changed, 45 insertions(+), 22 deletions(-) rename src/{CdnController.php => CdnFarfutureController.php} (91%) rename src/PathProcessor/{PathProcessorFarFuture.php => CdnFarfuturePathProcessor.php} (81%) rename src/Tests/{CdnUninstallTest.php => CdnIntegrationTest.php} (61%) diff --git a/README.txt b/README.txt index 39e46f3..c0f41a9 100644 --- a/README.txt +++ b/README.txt @@ -15,14 +15,13 @@ pretty much every CDN is an Origin Pull CDN (2015 and later). The CDN module aims to do only one thing and do it well: altering URLs to point to files on CDNs. It supports: • Any sort of CDN mapping + • optimal far future expiration • DNS prefetching • CSS aggregation • auto-balance files over multiple CDNs (http://drupal.org/node/1452092) • SEO: prevent CDN from serving HTML and REST responses, only allow assets • … and many more details that are taken care of automatically -Not yet ported: - • optimal Far Future expiration Installation ------------ diff --git a/cdn.routing.yml b/cdn.routing.yml index 50ac3d1..7dec5ad 100644 --- a/cdn.routing.yml +++ b/cdn.routing.yml @@ -1,7 +1,7 @@ -cdn.farfuture: +cdn.farfuture.download: path: '/cdn/farfuture/{security_token}/{mtime}' defaults: - _controller: '\Drupal\cdn\CdnController::farfuture' + _controller: '\Drupal\cdn\CdnFarfutureController::download' requirements: _access: 'TRUE' mtime: \d+ diff --git a/cdn.services.yml b/cdn.services.yml index 4b8411e..2560a0f 100644 --- a/cdn.services.yml +++ b/cdn.services.yml @@ -19,13 +19,13 @@ services: tags: - { name: event_subscriber } - # Path processor for Far Future support, since the Drupal 8/Symfony routing - # system does not support "menu tail" or "slash in route parameter". - # See: + # Inbound path processor for the cdn.farfuture.download route, since the + # Drupal 8/Symfony routing system does not support "menu tail" or "slash in + # route parameter". See: # - http://symfony.com/doc/2.8/routing/slash_in_parameter.html # - http://drupal.stackexchange.com/questions/175758/slashes-in-single-route-parameter-or-other-ways-to-handle-a-menu-tail-with-dynam # - https://api.drupal.org/api/drupal/includes%21menu.inc/function/menu_tail_to_arg/7.x path_processor.cdn: - class: Drupal\cdn\PathProcessor\PathProcessorFarfuture + class: Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor tags: - { name: path_processor_inbound } diff --git a/src/CdnController.php b/src/CdnFarfutureController.php similarity index 91% rename from src/CdnController.php rename to src/CdnFarfutureController.php index 965fc8d..dc5ace9 100644 --- a/src/CdnController.php +++ b/src/CdnFarfutureController.php @@ -12,7 +12,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; -class CdnController implements ContainerInjectionInterface { +class CdnFarfutureController implements ContainerInjectionInterface { /** * The private key service. @@ -39,11 +39,11 @@ class CdnController implements ContainerInjectionInterface { } /** - * Serves the requested file with optimal Far Future expiration headers. + * Serves the requested file with optimal far future expiration headers. * * @param \Symfony\Component\HttpFoundation\Request $request * The current request. $request->query must have root_relative_file_url, - * set by \Drupal\cdn\PathProcessor\PathProcessorFarFuture. + * set by \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor. * @param string $security_token * The security token. Ensures that users can not request any file they want * by manipulating the URL (they could otherwise request settings.php for @@ -57,12 +57,12 @@ class CdnController implements ContainerInjectionInterface { * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException * Thrown when the 'root_relative_file_url' query argument is not set, which * can only happen in case of malicious requests or in case of a malfunction - * in \Drupal\cdn\PathProcessor\PathProcessorFarFuture. + * in \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor. * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * Thrown when an invalid security token is provided. */ - public function farfuture(Request $request, $security_token, $mtime) { - // Ensure \Drupal\cdn\PathProcessor\PathProcessorFarFuture did its job. + public function download(Request $request, $security_token, $mtime) { + // Ensure \Drupal\cdn\PathProcessor\CdnFarfuturePathProcessor did its job. if (!$request->query->has('root_relative_file_url')) { throw new BadRequestHttpException(); } diff --git a/src/PathProcessor/PathProcessorFarFuture.php b/src/PathProcessor/CdnFarfuturePathProcessor.php similarity index 81% rename from src/PathProcessor/PathProcessorFarFuture.php rename to src/PathProcessor/CdnFarfuturePathProcessor.php index 693fb7f..3493134 100644 --- a/src/PathProcessor/PathProcessorFarFuture.php +++ b/src/PathProcessor/CdnFarfuturePathProcessor.php @@ -14,7 +14,7 @@ use Symfony\Component\HttpFoundation\Request; * * @see \Drupal\image\PathProcessor\PathProcessorImageStyles */ -class PathProcessorFarFuture implements InboundPathProcessorInterface { +class CdnFarfuturePathProcessor implements InboundPathProcessorInterface { /** * {@inheritdoc} @@ -26,13 +26,13 @@ class PathProcessorFarFuture implements InboundPathProcessorInterface { // Parse the token, unique file identifier method and root-relative file URL. $tail = substr($path, strlen('/cdn/farfuture/')); - list($token, $unique_file_identifier, $root_relative_file_url) = explode('/', $tail, 3); + list($security_token, $mtime, $root_relative_file_url) = explode('/', $tail, 3); // Set the root-relative file URL as query parameter. $request->query->set('root_relative_file_url', '/' . $root_relative_file_url); // Return the same path, but without the trailing file. - return "/cdn/farfuture/$token/$unique_file_identifier"; + return "/cdn/farfuture/$security_token/$mtime"; } } diff --git a/src/Tests/CdnUninstallTest.php b/src/Tests/CdnIntegrationTest.php similarity index 61% rename from src/Tests/CdnUninstallTest.php rename to src/Tests/CdnIntegrationTest.php index 6b0ae5b..d06f98a 100644 --- a/src/Tests/CdnUninstallTest.php +++ b/src/Tests/CdnIntegrationTest.php @@ -2,16 +2,16 @@ namespace Drupal\cdn\Tests; +use Drupal\Component\Utility\Crypt; +use Drupal\Core\Site\Settings; use Drupal\file\Entity\File; use Drupal\filter\Entity\FilterFormat; use Drupal\Tests\BrowserTestBase; /** - * Tests that uninstalling the CDN module causes CDN file URLs to disappear. - * * @group cdn */ -class CdnUninstallTest extends BrowserTestBase { +class CdnIntegrationTest extends BrowserTestBase { /** * Modules to enable. @@ -65,10 +65,15 @@ class CdnUninstallTest extends BrowserTestBase { $this->config('cdn.settings') ->set('mapping', ['type' => 'simple', 'domain' => 'cdn']) ->set('status', 2) + // Disable the farfuture functionality to simplify testing. + ->set('farfuture', ['status' => FALSE]) ->save(); } - public function testUninstall() { + /** + * Tests that uninstalling the CDN module causes CDN file URLs to disappear. + */ + public function atestUninstall() { $session = $this->getSession(); $this->drupalGet('/node/1'); @@ -85,4 +90,23 @@ class CdnUninstallTest extends BrowserTestBase { $this->assertSession()->responseContains('src="' . base_path() . $this->siteDirectory . '/files/druplicon.png"'); } + /** + * Tests that the cdn.farfuture.download route/controller work as expected. + */ + public function testFarfuture() { + $drupal_js_mtime = filemtime(DRUPAL_ROOT . '/core/misc/drupal.js'); + $drupal_js_security_token = Crypt::hmacBase64($drupal_js_mtime. '/core/misc/drupal.js', \Drupal::service('private_key')->get() . Settings::getHashSalt()); + + $this->drupalGet('/cdn/farfuture/' . $drupal_js_security_token . '/' . $drupal_js_mtime . '/core/misc/drupal.js'); + $this->assertSession()->statusCodeEquals(200); + // Assert presence of headers that \Drupal\cdn\CdnFarfutureController sets. + $this->assertSame('Wed, 20 Jan 1988 04:20:42 GMT', $this->getSession()->getResponseHeader('Last-Modified')); + // Assert presence of headers that Symfony's BinaryFileResponse sets. + $this->assertSame('bytes', $this->getSession()->getResponseHeader('Accept-Ranges')); + + // Any chance to the security token should cause a 403. + $this->drupalGet('/cdn/farfuture/' . substr($drupal_js_security_token, 1) . '/' . $drupal_js_mtime . '/core/misc/drupal.js'); + $this->assertSession()->statusCodeEquals(403); + } + } diff --git a/tests/src/Unit/File/FileUrlGeneratorTest.php b/tests/src/Unit/File/FileUrlGeneratorTest.php index f65b945..8ded7c3 100644 --- a/tests/src/Unit/File/FileUrlGeneratorTest.php +++ b/tests/src/Unit/File/FileUrlGeneratorTest.php @@ -122,7 +122,7 @@ class FileUrlGeneratorTest extends UnitTestCase { /** * @covers ::generate */ - public function testGenerateFarFuture() { + public function testGenerateFarfuture() { $gen = $this->createFileUrlGenerator('', [ 'status' => 2, 'mapping' => [ -- 2.9.0 From 50d752293fa3a035054c39082a838f396fd673df Mon Sep 17 00:00:00 2001 From: Wim Leers Date: Thu, 15 Sep 2016 00:50:51 +0200 Subject: [PATCH 9/9] Fix nitpicks, and re-enable the uninstallation test coverage. --- config/schema/cdn.schema.yml | 4 ++-- src/Tests/CdnIntegrationTest.php | 4 ++-- tests/src/Unit/File/FileUrlGeneratorTest.php | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/schema/cdn.schema.yml b/config/schema/cdn.schema.yml index f64261d..fcb5565 100644 --- a/config/schema/cdn.schema.yml +++ b/config/schema/cdn.schema.yml @@ -13,9 +13,9 @@ cdn.settings: label: 'File URL to CDN mapping' type: cdn.mapping.[type] farfuture: - label: 'Far Future expiration configuration' + label: 'Far future expiration configuration' type: mapping mapping: status: - label: 'Far Future expiration status' + label: 'Far future expiration status' type: boolean diff --git a/src/Tests/CdnIntegrationTest.php b/src/Tests/CdnIntegrationTest.php index d06f98a..2b7c8d9 100644 --- a/src/Tests/CdnIntegrationTest.php +++ b/src/Tests/CdnIntegrationTest.php @@ -65,7 +65,7 @@ class CdnIntegrationTest extends BrowserTestBase { $this->config('cdn.settings') ->set('mapping', ['type' => 'simple', 'domain' => 'cdn']) ->set('status', 2) - // Disable the farfuture functionality to simplify testing. + // Disable the farfuture functionality: simpler file URL assertions. ->set('farfuture', ['status' => FALSE]) ->save(); } @@ -73,7 +73,7 @@ class CdnIntegrationTest extends BrowserTestBase { /** * Tests that uninstalling the CDN module causes CDN file URLs to disappear. */ - public function atestUninstall() { + public function testUninstall() { $session = $this->getSession(); $this->drupalGet('/node/1'); diff --git a/tests/src/Unit/File/FileUrlGeneratorTest.php b/tests/src/Unit/File/FileUrlGeneratorTest.php index 8ded7c3..3538e42 100644 --- a/tests/src/Unit/File/FileUrlGeneratorTest.php +++ b/tests/src/Unit/File/FileUrlGeneratorTest.php @@ -137,8 +137,8 @@ class FileUrlGeneratorTest extends UnitTestCase { $this->assertSame('//cdn.example.com/core/misc/does-not-exist.js', $gen->generate('core/misc/does-not-exist.js')); $drupal_js_mtime = filemtime($this->root . '/core/misc/drupal.js'); - $drupal_js_calculated_token = Crypt::hmacBase64($drupal_js_mtime. '/core/misc/drupal.js', static::$privateKey . Settings::getHashSalt()); - $this->assertSame('//cdn.example.com/cdn/farfuture/' . $drupal_js_calculated_token . '/' . $drupal_js_mtime . '/core/misc/drupal.js', $gen->generate('core/misc/drupal.js')); + $drupal_js_security_token = Crypt::hmacBase64($drupal_js_mtime. '/core/misc/drupal.js', static::$privateKey . Settings::getHashSalt()); + $this->assertSame('//cdn.example.com/cdn/farfuture/' . $drupal_js_security_token . '/' . $drupal_js_mtime . '/core/misc/drupal.js', $gen->generate('core/misc/drupal.js')); } /** -- 2.9.0