Full disclosure: I originally reported this as an SI, but I got word back from Greg that it's more a configuration issue than a security issue. I agree, but I'm opening up discussion here to avoid headaches in the future.

I recently ran into an issue on a site where it was being mirrored from a junk domain via a CNAME record. Because of some subtle interplay between Views Cache and Page Cache (and the global $base_root not being set in settings.php), the primary RSS feed (run via Views), for a short time, pointed all links back to the junk domain rather than the real domain. (See scenario 2 here.)

I think this could be avoided by adding the base_url to the serialized portion of Views CIDs.

Again, I don't think this is crucial, but definitely worth discussing.

Patch attached. Would want something similar applied to the 6.x branch too.

CommentFileSizeAuthor
#10 views-add_base_url_to_cid_6x3x-1458504-10.patch996 bytesiamEAP
#10 views-add_base_url_to_cid_6x2x-1458504-10-do-not-test.patch982 bytesiamEAP
#7 views-add_base_url_to_cid-1458504-7.patch1.01 KBiamEAP
#5 views-add_base_url_to_cid-1458504-5.patch1 KBiamEAP
views-add_base_url_to_cid-0-0.patch1002 bytesiamEAP
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

One alternative would be to use relative paths.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review

Well relative paths for a rss feed isn't always what you want, maybe feed readers don't get that.

In general a view could depend on any kind of variable, and for very custom behaviors you might should better write a small cache plugin extension, thought for this problem is seems to make more sense to add this to views itself.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

Triggering the testbot.

Status: Needs review » Needs work

The last submitted patch, views-add_base_url_to_cid-0-0.patch, failed testing.

iamEAP’s picture

iamEAP CreditAttribution: iamEAP commented
Status: Needs work » Needs review
FileSize
views-add_base_url_to_cid-1458504-5.patch1 KB

Re-roll against latest dev.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Needs work 
+++ b/plugins/views_plugin_cache.incundefined
@@ -301,6 +302,7 @@ class views_plugin_cache extends views_plugin {
+        'base_url' => $GLOBALS['base_url']

missing trailing comma

iamEAP’s picture

iamEAP CreditAttribution: iamEAP commented
Status: Needs work » Needs review
FileSize
views-add_base_url_to_cid-1458504-7.patch1.01 KB

Just caught it. Thanks, Tim.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

Unless there is a good reason NOT to include this, I agree with dereine in #2.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Version: 7.x-3.x-dev » 6.x-3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks for adding this, it really makes sense in your cases.

iamEAP’s picture

iamEAP CreditAttribution: iamEAP commented
Status: Patch (to be ported) » Needs review
FileSize
views-add_base_url_to_cid_6x2x-1458504-10-do-not-test.patch982 bytes
views-add_base_url_to_cid_6x3x-1458504-10.patch996 bytes

Attached patches for both 6.x-3.x and 6.x-2.x branches.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

There aren't any tests for either 6.x branch anymore, but nice usage of the do-not-test suffix!

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Issue summary: View changes
Parent issue: » #2507839: Plan for Views v6.x-3.10 release
DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Related issues: +#2507847: Plan for Views v6.x-2.27 release

This should be added to 6.x-2.x too.

izmeez’s picture

izmeez CreditAttribution: izmeez commented

Patch in comment #10 applies to latest views-6.x-2.26 without difficulty and is already RTBC.

Chris Matthews’s picture

Chris Matthews CreditAttribution: Chris Matthews as a volunteer commented
Status: Reviewed & tested by the community » Closed (outdated)

The Drupal 6 branch is no longer supported, please check with the D6LTS project if you need further support. For more information as to why this issue was closed, please see issue #3030347: Plan to clean process issue queue