Bug #68728

cHash check in AbstractPlugin is wrong

Added by Andreas Allacher about 4 years ago. Updated 22 days ago.

Status:
New
Priority:
Should have
Assignee:
-
Category:
Link Handling, Site Handling & Routing
Target version:
-
Start date:
2015-08-04
Due date:
% Done:

0%

TYPO3 Version:
6.2
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

Currently AbstractPlugin does the following cHash check:

$this->piVars = GeneralUtility::_GPmerged($this->prefixId);
if ($this->pi_checkCHash && count($this->piVars)) {
    $GLOBALS['TSFE']->reqCHash();
}

Which is wrong for the following reason:

1.) It ignores cHash exclude parameters and various other cHash settings
2.) It checks for POST & GET variables. However, usually cHash only consists of GET variables.
Of course, that might be why one wants to check for cHash values. However, in the case of wrong POST variables I think it should only disable the caching instead of resulting in pageNotFound error.
And providing the correct cHash for POST variables is nearly impossible. Actually if a form is sent via GET variables it would also be an issue which is why this is usually handled by manually disabling caching in the plugin itself.

In that regard:
Is that check even necessary as the cHash should already be verified by the TypoScriptFrontendController? Of course, it wouldn't check for POST variables.

History

#1 Updated by Andreas Allacher about 4 years ago

I know there are similar tickets to this but they are not exactly the same.

#2 Updated by Riccardo De Contardi 4 months ago

Is this issue still valid? As far as I see the code has changed (at least on latest master) where I found:

frontend/Classes/Plugin/AbstractPlugin.php line 251 and followings:

        if ($this->prefixId) {
            $this->piVars = GeneralUtility::_GPmerged($this->prefixId);
            // cHash mode check
            // IMPORTANT FOR CACHED PLUGINS (USER cObject): As soon as you generate cached plugin output which
            // depends on parameters (eg. seeing the details of a news item) you MUST check if a cHash value is set.
            // Background: The function call will check if a cHash parameter was sent with the URL because only if
            // it was the page may be cached. If no cHash was found the function will simply disable caching to
            // avoid unpredictable caching behaviour. In any case your plugin can generate the expected output and
            // the only risk is that the content may not be cached. A missing cHash value is considered a mistake
            // in the URL resulting from either URL manipulation, "realurl" "grayzones" etc. The problem is rare
            // (more frequent with "realurl") but when it occurs it is very puzzling!
            if ($this->pi_checkCHash && !empty($this->piVars)) {
                $this->frontendController->reqCHash();
            }
        }

#3 Updated by Andreas Allacher 4 months ago

I don't think anything has changed, has it? _GPmerged still delivers GET and POST variables, however, only GET variables are relevant für cHash. It doesn't seem to take exclude arguments into account.
However, it is possible that the issue was fixed due to other changes. I haven't tested it in quite some time.

#4 Updated by Riccardo De Contardi 22 days ago

  • Category set to Link Handling, Site Handling & Routing

Also available in: Atom PDF