[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