Contrib.social

DNT (Do Not Track) header detection not working when caching enabled

Open on Drupal.org →
Created on 14 October 2024, 6 months ago

Problem/Motivation

The eu_cookie_compliance_build_data() function looks for $_SERVER['HTTP_DNT'] to be set and truthy, and sets the method to "opt_in" if it is. However, the value returned from eu_cookie_compliance_build_data() is stored in the render cache bin with a cache id that doesn't vary on the presence of $_SERVER['HTTP_DNT']. It varies on these values:

  1. Language ID
  2. Domain ID
  3. Theme name

Steps to reproduce

Enable caching, and enable EU Cookie Compliance in auto mode. Then send requests to the site both with and without the DNT header. Notice that the behavior does not vary based on the presence or absence of the header.

Proposed resolution

I traced the use of this custom render cache entry to #3015612: [1.x][2.0.x] Better caching for cookie performance popup → which noted a performance hit when rendering the popup. I question using it in addition to the cache tags already being added to eu_cookie_compliance_page_attachments() 

$variables['#cache']['tags'] = Cache::mergeTags($cache_tags, $config->getCacheTags());

Each variation we need to account for will need to be added to $cid, whereas the #cache ability on hook_page_attachments() handles tags, contexts, and more.

So there's two ways to resolve this:

  1. In the short term, vary $cid by DNT header.
  2. More substantially, remove the custom cache and vary eu_cookie_compliance_page_attachments() by all pertinent values.

Remaining tasks

Choose a path and write a patch.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

🐛 Bug report
Status

Active

Version

1.0

Component

Code

Created by

🇺🇸United States pianomansam

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @pianomansam
  • Pipeline finished with Success
    6 months ago
    Total: 50s
    #309655
  • Merge request !150remove manual caching of eu_cookie_compliance_build_data, add headers:DNT cache context → (Open) created by pianomansam
  • Pipeline finished with Success
    6 months ago
    Total: 118s
    #309661
  • Pipeline finished with Success
    6 months ago
    Total: 81s
    #309665
  • Comment 6 months ago →
    🇺🇸United States pianomansam

    I have opened a MR that removes the custom caching of eu_cookie_compliance_build_data() and adds the DNT header to the cache context of eu_cookie_compliance_page_attachments(). Default services (default.services.yml) already has the language and theme cache contexts. And the Domain module suggests adding `url.site` to that list.

    I did not see any existing tests for this module, and even if I did, testing this seems pretty difficult. So I'll simply mark this for needing review and let others test it out.

  • Comment 5 months ago →
    🇳🇿New Zealand atowl

    Not that i'm an expert in caching, but we might have to put some of the caching context back.
    Keep the Language, and theme out, since it's done in the services file already, and leave in the domain module check? This will leave specific domain caching intact?

    Thoughts?

  • Pipeline finished with Success
    5 months ago
    Total: 51s
    #343597
  • Comment 5 months ago →
    🇺🇸United States pianomansam

    @atowl regarding the domain cache context, the domain module documentation suggests adding url.site to the services file. So users of that module should follow its directions, as doing so will resolve all sorts of things. If domain module users are experiencing issues with this module, it's not due to this module but rather not properly configuring their site for the domain module.

  • Pipeline finished with Success
    5 months ago
    Total: 112s
    #343616
  • Status changed to Needs work about 1 month ago
  • Comment about 1 month ago →
    🇳🇴Norway svenryen

    Hi!

    Thanks for helping.

    Could you please explain the reason for the removal of line 156-179?

  • Comment about 1 month ago →
    🇺🇸United States pianomansam

    @svenryen In summary, I'm questioning the work done on #3015612: [1.x][2.0.x] Better caching for cookie performance popup → and proposing that we undo it. I do not understand why that task says we cannot use the render cache. I will move my discussion to that task.

  • Comment about 1 month ago →
    🇨🇭Switzerland berdir Switzerland

    Side note: DNT is afaik dead: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/DNT. There seems to be a replacement that probably needs similar treatment though.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024