SSViewer anchor rewriting can be captured in cache blocks

[UndefinedOffset]

Affected Version

silverstripe/recipe-cms 4.3.3

Description

SSViewer's anchor/hash link rewritting can be captured in cache blocks in some instances due to how nested template rendering is performed. This should probably only be done after the cache block is loaded not when it is cached. As in some instances a link may have identifiable information for marketing purposes or other tracking information that is unique to the visitor. However with the rewriting being stored in the cache it's possible that the visitor with url parameters could re-generate the cache thus capturing their rewritten urls in the cache.

Steps to Reproduce

Given the following template:

<% cached 'cacheName', $SomeCacheVariables %>
    <% loop $Elements %>
        $Me
    <% end_loop %>
<% end_if %>

And this example element template:

<div class="element">
    <nav>
        <ul>
            <li><a href="#example-anchor-1">Example Anchor 1</a></li>
            <li><a href="#example-anchor-2">Example Anchor 2</a></li>
            <li><a href="#example-anchor-3">Example Anchor 3</a></li>
            <li><a href="#example-anchor-4">Example Anchor 4</a></li>
        </ul>
    </nav>
</div>

You could end up with something like the below in the actual rendered cache block, until the cache is regenerated.

<div class="element">
    <nav>
        <ul>
            <li><a href="/my-page/?cid=us:media:192090&amp;dclid=ofnisdonfdisofnheu#example-anchor-1">Example Anchor 1</a></li>
            <li><a href="/my-page/?cid=us:media:192090&amp;dclid=ofnisdonfdisofnheu#example-anchor-2">Example Anchor 2</a></li>
            <li><a href="/my-page/?cid=us:media:192090&amp;dclid=ofnisdonfdisofnheu#example-anchor-3">Example Anchor 3</a></li>
            <li><a href="/my-page/?cid=us:media:192090&amp;dclid=ofnisdonfdisofnheu#example-anchor-4">Example Anchor 4</a></li>
        </ul>
    </nav>
</div>

[maxime-rainville]

Managed to confirm behavior. I got flush token in my anchor link cached.

I reproduce this with these bits of code.

Element.php

<?php

use SilverStripe\View\ViewableData;

class Element extends ViewableData {

    private $no;

    public function __construct($no)
    {
        $this->no = $no;
    }

    public function forTemplate()
    {
        return $this->renderWith(self::class);
    }

    public function getNo()
    {
        return $this->no;
    }
}

Element.ss

<a href="home#example-anchor-$No">Example Anchor $No</a>

Add this snippet to PageController.php

        public function Elements()
        {
            return ArrayList::create([
                new Element(1),
                new Element(2),
                new Element(3),
                new Element(4),
                new Element(5),
            ]);
        }

        public function SomeCacheVariables()
        {
            return 'boom';
        }

Add this snippet to Page.ss

    <div class="element">
        <nav>
            <ul>
            <% cached 'cacheName', $SomeCacheVariables %>
            <% loop $Elements %>
                <li>$Me</li>
            <% end_loop %>
            <% end_cached %>
            </ul>
        </nav>
    </div>

[UndefinedOffset]

I wonder if hooking into the SSTemplateParser where the cache block is created (perhaps around here), then setting SSViewer::setRewriteHashLinksDefault(false), then resetting it back to the original value after the cache is set (around here)?

I might be able to open a pull request if you think this approach makes sense?

[UndefinedOffset]

Something like this? https://github.com/webbuilders-group/silverstripe-framework/commit/19d445037dfe309ab69cd9de78007facb8912199

[UndefinedOffset]

I opened pull #9044 with the fix from https://github.com/webbuilders-group/silverstripe-framework/commit/19d445037dfe309ab69cd9de78007facb8912199 that appears to work for resolving this issue. Basically I'm disabling the hash rewrite in cacheblocks completely (and restoring at the end of it), which shouldn't cause issues since the outer template parser instance would still rewrite the hashes. This way they just don't get caught in the actual cache.

Right now this is posing an issue for the analytics team on one of our projects so hence the bump ;) apologies.

[UndefinedOffset]

I changed my pull request to a draft, because as I mention there it's not a solid fix it still can be caught in the caches. Basically I think this requires a much more extensive rework of how hash rewriting works than at the time I was able to wrap my head around. It maybe better suited for who ever is most familiar with SSViewer and how templates are handled. I'll leave it up to the core team to decide if the pull should just be closed, it's pretty old and out dated. However I can confirm the issue does still happen in 4.5.2.

[UndefinedOffset]

I kind of wonder if it makes sense to make the hash rewriting a middleware instead of something SSViewer does? Does that make more sense? If so does it make more sense for me to close my pull and open a new one targeting the 4 branch, knowing something like that would likely not make sense as a bug fix in 4.5 (or 4.4).

[GuySartorelli]

Added to CMS 5 epic because the current proposed solution includes disabling (and probably removing) the existing mechanism for rewriting hash links in favour of a middleware.