Problem/Motivation

The max_age_zero pattern of PerformanceAnalyzer::CACHE_PROBLEM_PATTERNS requires 'max-age' => 0 to come right after the opening bracket of the '#cache' array. Reproduced on current 1.x with drush audit:run performance:

$a = ['#cache' => ['max-age' => 0]];                               => CACHE_MAX_AGE_ZERO
$b = ['#cache' => ['contexts' => ['user'], 'max-age' => 0]];      => missed
$c = [
  '#cache' => [
    'tags' => ['node_list'],
    'max-age' => 0,                                                => missed
  ],
];

Declaring contexts or tags before max-age is common, so the rule misses a large share of the cases it is meant to find.

Proposed resolution

Match 'max-age' => 0 anywhere inside the '#cache' array, also after keys whose values are arrays one level deep (such as 'contexts' => ['user']) and across several lines, without leaving that array: a 'max-age' => 0 in a sibling render array key is not attributed to '#cache'. The alternatives of the new pattern cannot overlap, so it does not backtrack excessively. A test covers the three cases above, a non-zero max-age and a max-age 0 outside '#cache'; it fails without the fix.

Sites will see more CACHE_MAX_AGE_ZERO findings after this change; they are real cases that were previously missed.

The original report also proposed downgrading CACHE_MAX_AGE_ZERO to a notice when every route of a controller is an admin route. That is not needed any more: the admin report case is now covered by the inline audit-ignore marker (#3626576), e.g. "// audit-ignore: CACHE_MAX_AGE_ZERO -- admin report backed by a log table", and by the rule overrides setting (#3626580), e.g. "CACHE_MAX_AGE_ZERO */src/Controller/Admin* notice". Both are explicit and reviewable, whereas guessing from routing.yml files would add a heuristic that can be wrong both ways.

Remaining tasks

  • Review the merge request.

Comments

trebormc created an issue. See original summary.

  • trebormc committed 18d8ac94 on 1.x
    Issue #3626589 by trebormc: Detect CACHE_MAX_AGE_ZERO wherever 'max-age...
trebormc’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.