We tried to blacklist *.woff files when investigating #982188-3: Work around Same-Origin Policy: Cross-Origin Resource Sharing (CORS) support, but the blacklist had no effect. Looking closer, it seems that for resources in CSS files:

  • Black-/white-listing has no effect.
  • The resources in those files inherit the stylesheet's UFI and ignore the ones defined in CDN

To demonstrate, use the following Unique file identifier generation:

*.css|drupal_cache
*.png|mtime

All .png files from stylesheets will be served with a "drupal_cache:xxx" UFI instead of an "mtime:xxx" UFI.

(might be a documentation issue instead of a bug)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mr.baileys’s picture

Title: Far Future resources in CSS inherits stylesheets UFI. » Far Future resources in CSS inherit stylesheet UFI.

Typo in title...

Wim Leers’s picture

Status: Active » Postponed (maintainer needs more info)

This needs to be documented better.

The file URLs in CSS files are only rewritten if you have CSS aggregation enabled (otherwise file URLs in CSS files remain relative). If some CSS aggregations exist before you were using the CDN module (or with old CDN settings), then they may not be using CDN-altered file URLs.

Please confirm that this is indeed what you're seeing.

mr.baileys’s picture

Status: Postponed (maintainer needs more info) » Active

Nope, unfortunately, I'm seeing the same behavior regardless of whether the CSS files are aggregated or not.

Took the following steps (with the CDN UFI config from the OP):
a) Removed all aggregated css files from sites/default/files/css
b) Cleared the cache (via drush cc all to avoid regenerating any stylesheets before the cache is empty)

Expected behavior:
CSS files served with a "drupal-cache" UFI, *.png files within the stylesheet served with a "mtime" UFI.

Actual result:
Both the stylesheet as well as all *.png files served with the same "drupal-cache" UFI.

For example, the following snippet from system.base.css:

div.tree-child-last {
  background: url(../../misc/tree-bottom.png) no-repeat 11px center; /* LTR */
}

is aggregated in

/cdn/farfuture/drupal-cache:lyx71o/sites/default/files/css/css_Ga88rK8CNUh9LSfwBJ9v919hDQxfCWLXPg1IUgTLWjg.css

as:

div.tree-child-last{background:url(/cdn/farfuture/drupal-cache:lyx71o/misc/tree-bottom.png) no-repeat 11px center;}

I'm assuming that this will also affect cdn_pick_server(), not just the UFI on the files.

Wim Leers’s picture

I haven't found the time yet to dig deeper into this issue. But this is *very* strange for sure: I don't see how this could happen. Clearly, I must have missed an edge case.

Wim Leers’s picture

Are you possibly using another module that affects CSS aggregation, such as AdvAgg?

Because what you're seeing strongly contradicts with #1138410: Background images defined by CSS are not being run through CDN.

Wim Leers’s picture

The reason is in how CSS aggregation works: it uses drupal_build_css_cache(), which does the following:

        // Build the base URL of this CSS file: start with the full URL.
        $css_base_url = file_create_url($stylesheet['data']);
        // Move to the parent.
        $css_base_url = substr($css_base_url, 0, strrpos($css_base_url, '/'));
        // Simplify to a relative URL if the stylesheet URL starts with the
        // base URL of the website.
        if (substr($css_base_url, 0, strlen($GLOBALS['base_root'])) == $GLOBALS['base_root']) {
          $css_base_url = substr($css_base_url, strlen($GLOBALS['base_root']));
        }

        _drupal_build_css_path(NULL, $css_base_url . '/');
        // Anchor all paths in the CSS with its base URL, ignoring external and absolute paths.
        $data .= preg_replace_callback('/url\(\s*[\'"]?(?![a-z]+:|\/+)([^\'")]+)[\'"]?\s*\)/i', '_drupal_build_css_path', $contents);

So the culprit is Drupal core's overly simplistic CSS aggregation, which makes assumptions that are no longer valid when Far Future expiration is used.

Currently looking for a work-around. Or a hack to work around it…

Wim Leers’s picture

hook_element_info_alter() is going to allow us to override the CSS aggregation in D7, but in D6, things will likely get very ugly…

Wim Leers’s picture

Status: Active » Needs review
FileSize
8.94 KB
5.32 KB

Managed to get it working in both D7 and D6.

Wim Leers’s picture

Title: Far Future resources in CSS inherit stylesheet UFI. » Override CSS aggregation to ensure correct file URL altering for files referenced by CSS files
Assigned: Unassigned » Wim Leers
Category: bug » feature
Priority: Normal » Critical
Wim Leers’s picture

Note that the D6 version of the patch detects the AdvAgg module and *doesn't* do the CSS aggregation, but leaves it up to AdvAgg. I reported this in their issue queue (#1436360: AdvAgg + CDN module's CSS aggregation functionality), to make sure we do the same for the D7 version of the CDN module whenever the D7 port of the AdvAgg module is released.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

mr.baileys reported to me through another channel that he reviewed this patch on both D7 and D6, and on a very large production D7 site. It worked perfectly :)

Wim Leers’s picture

Wim Leers’s picture

Status: Fixed » Needs review
FileSize
9.67 KB
7.1 KB

Problems found with the changes introduced here; also reported in #1441988-4: File URLs broken after upgrade to version 2.3 (cache clearing was not documented).

These patches were tested on at least 3 local installs, one small and one very big production site. No problems.

The problems are very tricky:
1) The first problem doesn't occur when you only have CSS files that end up being aggregated; it only manifests when you have module and/or theme files that are marked to not be preprocessed. These CSS files are then indeed not preprocessed (also not by the custom CSS aggregator introduced here), but … this only works if the images referenced by the CSS files are accessible at the same path. Due to the secured Far Future URLs in version 2.3 (necessary to fix the security flaw), this is no longer true. Hence the images cannot load.
So, we do need to preprocess each file individually, and alter their file URLs so that the Far Future URLs to the images in the CSS file are also secured.

2) When you're using Drupal 6 and you're relying on the fallback mechanism, we cannot rely on file_create_url() to generate proper URLs. So we have to provide an alternative function in Drupal 6, that only calls file_create_url() when the core patch has been applied. Otherwise, e.g. sites/all/modules/admin_menu/admin_menu.css would be rewritten to sites/default/files/sites/all/modules/admin_menu/admin_menu.css, because Drupal core's file_create_url() function is so stupid.

3) Last but not least: we don't need to override Drupal's aggregation at all if the current page is not going to use a CDN anyway. To be able to detect that, I moved some of the logic in cdn_file_url_alter() into separate functions. This also helps improve testability and with #1413176: Provide API so other modules can hook in/depend on the CDN module.

Patches attached. Please test and report back.

nottaken’s picture

This patch is working for me when applied to 6.x-2.x-dev and doing local testing. I hope to push this live soon. I will continue to watching this issue for any updates. Thanks for the patch. -brian

Wim Leers’s picture

Now I just need D7 confirmation!

nottaken’s picture

using this in production now - only issue I see is on secure pages. The css files are pointing to images on non secure host. Maybe a cdn_secure.css adn a cnd_nonsecure.css would solve this problem? although, maybe I have misconfiguration something?

nottaken’s picture

I had css and js excluded from the cdn (using a aws pull) so the paths were only getting created for non secure pages. I see now that I needed to allow css and js for the cdn on both secure and non secure host, so it would pull versions for secure and non secure css files. The module is building a secure and non secure css file that have the correct paths. So, it's working as expected now.

nottaken’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
Category: feature » bug
Status: Needs review » Active

Not sure if this is related to this issue completely, but after running this patch for a few days to a week, I'm seeing secure pages use a css file with non secure paths in them. The css file name however is different for secure and non secure pages. Clearing the cache and loading the secure page seems to fix this, and non secure pages seem to be doing fine either way. Running 6.x-2.x-dev with the patch here and aws pull.

Ideas?

Wim Leers’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Active » Fixed

Even if the current patch may not be perfect (as #18 might indicate), fact is that it solves the problems for the majority of use cases. Since I seem to be unable to gather feedback for D7 from users, I'm just going to commit these patches as-is.

@nottaken: if you can reproduce the problem you've mentioned reliably, please open a new issue! :)

D7: http://drupalcode.org/project/cdn.git/commit/bf76200
D6: http://drupalcode.org/project/cdn.git/commit/dd9ab15

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Lux Delux’s picture

Status: Closed (fixed) » Active

Hello, I just installed the module on my D7 site.

CSS background images are not served through CDN, everything else seems to work fine.

Let me see if I set it up correctly in paths whitelist, for example when using the genesis theme

sites/all/themes/genesis/genesis_SUBTHEME/css/*.png

I entered this in the whitelist. I'm calling the .png files like this

background: url ('test.png')

Which points to the css folder

Any ideas? Cheers

Wim Leers’s picture

Status: Active » Closed (fixed)

Please report this in a new issue.

Wim Leers’s picture

JeremyFrench’s picture