Bug #17251
closedtslib_fe::makeCacheHash does not work if no cHash
0%
Description
this function checks the cHash validity and turn the TSFE in no_cache mode if cHash is not valid...
But if no cHash is set, it doesn't do the check !
@see patch
(issue imported from #M5531)
Files
Updated by Michael Stucki over 17 years ago
Please attach patches, not PHP code!
Updated by Popy no-lastname-given over 17 years ago
Is this your way to say "Thanks", or this is particular for me ?
I don't really have the time to make patchs wich will probably never been included... maybe I should not POST solutions ? or reporting bugs ?
Sorry for my bad mood
Updated by Michael Stucki over 17 years ago
Yes it seems you're in a really bad mood... I'm sorry for being too demanding, or whatever you'd call it.
The reason why I'm asking for patches is because I want to see what you have changed inside the code, and not how the modified code looks like.
Counter question: I don't really have the time to make patches that probably introduce some kind of misconception. Do you expect anyone in here to do so?
(But well, let's not dispute about that, after all I am willing to commit that patch, if it makes sense...)
Cheers! :-)
Updated by Michael Stucki over 17 years ago
And just one additional hint: Once you have set up Subversion which is only 5 or 10 minutes away from you, it would be even quicker to post patches instead of creating PHP files manually.
Just to let you know about that...
Updated by Popy no-lastname-given over 17 years ago
Also, I'll try to make a patch this afternoon.
I understand the problem, you know.
But you know, the code is perfet, you can commit immediatly ;-)
Updated by Popy no-lastname-given over 17 years ago
Patch added. As you see, many things change :)
Updated by Oliver Hader over 17 years ago
Thanks for adding the patch file.
Your changes require to have a cHash on every GET call, even if this is addressing a USER_INT object, doesn't it?
Furthermore: "strcmp($cHash_calc, $this->cHash)" vs. "$cHash_calc !== $this->cHash"
Updated by Popy no-lastname-given over 17 years ago
Yes, ever a valid cHash
I have heard somewhere that using set_no_cache and &no_cache=1is bad...
Also USER plugin's should have another way to disable caching when POST data should be inserted (for example). Also if a missing cHash is the only good solution to disable caching... it should work ! Or I need another solution.
Of course, it means that USER_INT plugins will disable caching in every links...
But using USER_INT for a whole plugin is bad, isn't it ?
But, what's the problem with strcmp ? As I remmember, it is recommended in Typo3 CGL...
A single coffee and mood is better, isn't it ?
Updated by Popy no-lastname-given over 17 years ago
I have another idea to solve theses USER / USER_INT / set_no_cache / &no_cache problems, but it is complex...
The idea is : as we have parts of the page wich are not cached, we should have part of the url wich corresponds to theses uncached parts. And here the cHash has 2 roles : validating the "cached" parameters, and separating the "cached" parameters and "uncached" parameters
Also urls will seems like this :
<core params><cached params><chash><uncached params>
Where core params ar id, type, MP, ...
Then, using querystring, the makeCacheHash can determine "cached params" and "uncached params", and check the cHash validity only with "cached params"
Note that linkVars must be included in "cached params"
With this system, every works :
a USER_INT plugin make links with linkVars, a cHash corresponding to these linkVars, and its own GET vars
a USER plugin will work as now, but build a wrong / empty cHash param to turn in no_cache mode
If the idea is approved, I can provide the new makeCacheHash function, and a new proprety will be needed in typolink, but nothing else needs to be modified (except pibase linking functions, of course).
Updated by Oliver Hader over 17 years ago
Yes, I made a similar patch some weeks ago. I called it cHashTunnel, that allows to use a proper cHash for the URL and "tunnel" parameters for USER_INT plugins around the cHash validation (and the storage in cache for those individual parameters).
I'm going to create a new feature request for this and relate it to this issue.
Updated by Popy no-lastname-given over 17 years ago
I've found a little mistake in the patch I did post :
I did write "$cHash_calc = t3lib_div::shortMD5(serialize($cHash_array));"
instead of "$cHash_calc = t3lib_div::shortMD5(serialize($this->cHash_array));"
@Oliver Hader : do you want I attach the makeCacheHash function with cached/uncached params support ?
Updated by Elmar Hinz about 17 years ago
@Popy no-lastname-given: You want to send POST data to a USER plugin. You should always use a USER_INT for dynamic plugins.
Updated by Elmar Hinz about 17 years ago
@Oliver:
You write: Your changes require to have a cHash on every GET call, even if this is addressing a USER_INT object, doesn't it?
This sentence points to the misconception: The cHash applies to a whole page, but the check of the cHash is triggert by single plugins. If you want to address a USER_INT, there can still be a USER object in the same page triggering the check.
Because you want to address a USER_INT you don't send a cHash. But a USER plugin in the same page triggers the checking and the checking fails, because of the missing cHash. For the configuration pageNotFoundOnCHashError = 1 we run into trouble.
My suggestion: Remove this triggering of the the check.
Instead: Always check, whenever a cHash has been send. If the check is passed, cache the page.
This may sound dangerous on the first glance, but it isn't. Valid cHashes are only generated on your own machine, so you have it under control. The extension developer makes the decision to cache in one point only, in the moment the cHash is created.
To additionally configure triggering on the other end makes the matters complicated and confusing. Confusing things are always less secure. I suggest to make it simple. Remove the triggering. Control caching in one single point.
Updated by Alexander Opitz over 11 years ago
- Category deleted (
Communication) - Status changed from New to Needs Feedback
- Target version deleted (
0)
The issue is very old, does this issue exists in newer versions of TYPO3 CMS (4.5 or 6.1)?
Updated by Popy no-lastname-given over 11 years ago
Alexander Opitz wrote:
The issue is very old, does this issue exists in newer versions of TYPO3 CMS (4.5 or 6.1)?
Seems yes. Unless CacheHashCalculator::$requireCacheHashPresenceParameters is filled with every used parameter name.
Updated by Alexander Opitz over 11 years ago
Can you tell us the version with which you tested?
Updated by Popy no-lastname-given over 11 years ago
Can you tell us the version with which you tested?
None, i just red the 6.0 makeCacheHash code, and the only validation step done if no cHash is provided is to look in CacheHashCalculator::$requireCacheHashPresenceParameters, which I highly doubt is filled with every possible parameter name.
Updated by Alexander Opitz almost 11 years ago
- Status changed from Needs Feedback to New
- Is Regression set to No
Updated by Mathias Schreiber over 9 years ago
- Category set to Frontend
- Target version set to 7.2 (Frontend)
Updated by Benni Mack about 9 years ago
- Target version changed from 7.2 (Frontend) to 7.4 (Backend)
Updated by Susanne Moog about 9 years ago
- Target version changed from 7.4 (Backend) to 7.5
Updated by Benni Mack almost 9 years ago
- Status changed from New to Needs Feedback
- Target version deleted (
7.5)
seems not that trivial. care to create a new patch? maybe with test cases to it?
Updated by Alexander Opitz about 8 years ago
Hi, what is the state of this issue?
Updated by Alexander Opitz almost 8 years ago
- Status changed from Needs Feedback to Closed
No feedback within the last 90 days => closing this issue.
If you think that this is the wrong decision or experience this issue again, then please write to the mailing list typo3.teams.bugs with issue number and an explanation or open a new ticket and add a relation to this ticket number.