Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid return type in ContentCacheSegmentAspect #168

Closed
MCStreetguy opened this issue May 30, 2022 · 1 comment · Fixed by #169
Closed

Invalid return type in ContentCacheSegmentAspect #168

MCStreetguy opened this issue May 30, 2022 · 1 comment · Fixed by #169

Comments

@MCStreetguy
Copy link
Contributor

Currently there is an error in the ContentCacheSegmentAspect when a cached segment results in a non-string value (e.g. through being nulled by an @if expression):

Return value of t3n\Neos\Debug\Aspect\ContentCacheSegmentAspect_Original::renderCacheInfoIntoSegment() must be of the type string, null returned

As far as I can tell, this is due to the fact, that the renderCacheInfoIntoSegment method declares a non-nullable return type of string, but may return non-string values, as the $segment variable also might be of any type, just like stated in the belonging docblock:

/**
 * @param mixed $segment This is mixed as the RuntimeContentCache might also return none string values
 * @param mixed[] $info
 */
protected function renderCacheInfoIntoSegment($segment, array $info): string
{
    // ...
    // Add debug data only to html output
    $segmentFormat = $info['entryIdentifier']['format'] ?? null;

    if ($segmentFormat !== 'html') {
        return $segment;
    }

    // ...

Therefore, in case of any non-string value such as integers, objects or like in my initial case null, this throws the above error.
You can reproduce this behaviour with the following short fusion code i used for testing this bug:

prototype(Neos.Neos:Page) {
  body = Neos.Fusion:Value {
    value = ${null}
    //or: value = 25
    //or: value = ${node}

    @cache {
      mode = 'uncached'
      context {
        1 = 'node'
      }
    }
  }
}

The quickest solution I can think of would be to remove the return types alltogether as the return type can be nearly anything.
I am happy to create an appropriate merge request, provided there is no disagreement with my proposed solution.

@Sebobo
Copy link
Collaborator

Sebobo commented Jun 2, 2022

Your proposal sounds good, thx :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants