Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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)
Comment | File | Size | Author |
---|---|---|---|
#13 | 1428530_follow-up_13_D7.patch | 7.1 KB | Wim Leers |
#13 | 1428530_follow-up_13_D6.patch | 9.67 KB | Wim Leers |
#8 | 1428530-8_D7.patch | 5.32 KB | Wim Leers |
#8 | 1428530-8_D6.patch | 8.94 KB | Wim Leers |
Comments
Comment #1
mr.baileysTypo in title...
Comment #2
Wim LeersThis 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.
Comment #3
mr.baileysNope, 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:
is aggregated in
as:
I'm assuming that this will also affect cdn_pick_server(), not just the UFI on the files.
Comment #4
Wim LeersI 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.
Comment #5
Wim LeersAre 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.
Comment #6
Wim LeersThe reason is in how CSS aggregation works: it uses
drupal_build_css_cache()
, which does the following: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…
Comment #7
Wim Leershook_element_info_alter() is going to allow us to override the CSS aggregation in D7, but in D6, things will likely get very ugly…
Comment #8
Wim LeersManaged to get it working in both D7 and D6.
Comment #9
Wim LeersComment #10
Wim LeersNote 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.
Comment #11
Wim Leersmr.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 :)
Comment #12
Wim LeersD7: http://drupalcode.org/project/cdn.git/commit/73e601b
D6: http://drupalcode.org/project/cdn.git/commit/aea6004
Comment #13
Wim LeersProblems 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 tosites/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.
Comment #14
nottaken CreditAttribution: nottaken commentedThis 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
Comment #15
Wim LeersNow I just need D7 confirmation!
Comment #16
nottaken CreditAttribution: nottaken commentedusing 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?
Comment #17
nottaken CreditAttribution: nottaken commentedI 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.
Comment #18
nottaken CreditAttribution: nottaken commentedNot 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?
Comment #19
Wim LeersEven 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
Comment #21
Lux Delux CreditAttribution: Lux Delux commentedHello, 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
Comment #22
Wim LeersPlease report this in a new issue.
Comment #23
Wim Leers#18: fixed in #1212184: Create different CSS aggregation files for HTTP and HTTPS.
Comment #24
JeremyFrench CreditAttribution: JeremyFrench commentedsee #1942786: Mixed content warning in safari on https page