[TYPO3-core] Announcing TYPO3 4.5.18, 4.6.11 and 4.7.3
Markus Klein
klein.t3 at mfc-linz.at
Thu Aug 9 01:18:46 CEST 2012
Hello guys!
After spending more time on this "regression" I think this is not a regression.
(I agree with Helmut that we might never be able to catch all possible select parts.)
Let me give a short history of how this code evolved:
Before 4.6 the fields uid and pid were always added to the selectfields.
With [1] Björn added a patch for 4.6 which should refine this, by
"Do not add uid,pid,... to field list if the field list already contains a '*' or the repective field is in the list."
Until patch set 9 the code was:
if (isset($GLOBALS['TCA'][$table]) && !strstr($selectPart, '*')) {
As Benjamin Mack correctly commented there: "strpos() !== FALSE is faster than strstr(), please modify"
Unfortunately this was "fixed" in patch set 10 with:
if (isset($GLOBALS['TCA'][$table]) && strpos($selectPart, '*')!== FALSE) {
This is WRONG as one negation was omitted! [Correct would be && strpos($selectPart, '*') === FALSE]
Patch set 11 was merged. (I wonder how the tests were working?)
Therefore the bug was introduced "again" and uid was added whenever * was part of selectfields.
The current version from [2] is IMHO correct and fixes the bug introduced in 4.6 [1] patch set 10/11.
Kind regards
Markus
[1] https://review.typo3.org/4974
[2] https://review.typo3.org/9158
> Hi,
>
> On 08.08.12 21:39, Oliver Hader wrote:
>
> > can you please create a new report on Forge and post the values of
> > $selectPart that are used to call tslib_cObj::sanitizeSelectPart()?
>
> I fear the regex used for tslib_cObj::sanitizeSelectPart() is ureliable and, that we will never catch all possible cases without doing a real
> SQL parsing, which imho too much overhead for fixing the original issue.
> Regex can not do this job properly[1]
>
> We must either live with these regression for the next several releases (it's the third release, which introduced a different kind of
> regression in the same area) or just remove this method and add to the documentation that in case wokrspace preview is needed,
> the respective fields need to be added manually.
>
> I would prefer a working tslib_cObj::sanitizeSelectPart() of course, but have no idea to get this done reliably. That is why I would vote
> vor reverting it completely.
>
> Kind regards,
> Helmut
>
> [1]http://stackoverflow.com/questions/139926/regular-expression-to-match-common-sql-syntax
>
> --
> Helmut Hummel
> Release Manager TYPO3 6.0
> TYPO3 Core Developer, TYPO3 Security Team Leader
>
> TYPO3 .... inspiring people to share!
> Get involved: typo3.org
> _______________________________________________
> Before posting to this list, please have a look to the posting rules on the following websites:
>
> http://typo3.org/teams/core/core-mailinglist-rules/
> http://typo3.org/development/bug-fixing/diff-and-patch/
> _______________________________________________
> TYPO3-team-core mailing list
> TYPO3-team-core at lists.typo3.org
> http://lists.typo3.org/cgi-bin/mailman/listinfo/typo3-team-core
More information about the TYPO3-team-core
mailing list