Bug #75493

stdWrap properties executed twice (patch provided)

Added by Olaf Schmidt-Wischhöfer over 3 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Frontend
Target version:
Start date:
2016-04-11
Due date:
% Done:

100%

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

Description

The method stdWrap of class \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer includes a check to prevent duplicate execution of sub-properties for recursive stdWrap functions and cObjects. This check only words in some of the cases.

Example:

page = PAGE
page.10 = TEXT
page.10.stdWrap.append = TEXT

Result: “append” is printed twice in the TypoScript section of the admin panel.

Workaround:

page = PAGE
page.10 = TEXT
page.10.stdWrap =
page.10.stdWrap.append = TEXT

A patch is attached.

I tested the bug fix with version 6.2.19. I also compared the code of current HEAD in git.typo3.org and saw no change – so the fix should be applied to all branches.

ContentObjectRenderer.php.diff View - Bug fix (944 Bytes) Olaf Schmidt-Wischhöfer, 2016-04-11 11:13

2016-05-04_232104.jpg View (51.6 KB) Nicole Cordes, 2016-05-04 23:23

Associated revisions

Revision a4897da0 (diff)
Added by Olaf Schmidt-Wischhöfer over 3 years ago

[!!!][BUGFIX] Evaluate "boolean /stdWrap" properties correctly

stdWrap sub-properties on boolean properties have to be evaluated
correctly.

Resolves: #75493
Related: #60135
Releases: master
Change-Id: I7ab418a97228beb49458fa43e5d0ce50aef83877
Reviewed-on: https://review.typo3.org/48008
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>
Reviewed-by: Nicole Cordes <>
Tested-by: Nicole Cordes <>
Reviewed-by: Oliver Hader <>
Reviewed-by: Frank Naegler <>
Tested-by: Frank Naegler <>

History

#1 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

  • % Done changed from 0 to 80

#2 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

I could also have selected version 7.6 or version 8 – please feel free to change this as appropriate.

#3 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

  • Target version set to next-patchlevel

#4 Updated by Gerrit Code Review over 3 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#5 Updated by Nicole Cordes over 3 years ago

I don't see the issue that "append" is added twice to the TypoScript sections in Admin Panel with current master and the TypoScript you provided.

Can you please help me out to get a better understanding what is wrong here? Can you say which option in Admin Panel must be enabled and which output you get?

Thank you!

#6 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

Thanks for your response!

I have created a different test case to show the problem. I have verified it with Typo3 6.2.22, and will test it with current master either later today or on Monday.

Consider the following TypoScript setup:

page = PAGE
page.10 = TEXT
page.10.value.setCurrent = test1
page.20 = TEXT
page.20.value.current = 1
page.20.stdWrap.setCurrent = test2

The output is: test1

page = PAGE
page.10 = TEXT
page.10.value.setCurrent = test1
page.10.stdWrap.append = TEXT
page.10.stdWrap.append.value.current = 1
page.10.stdWrap.append.setCurrent = test2

The output is: test2

page = PAGE
page.10 = TEXT
page.10.value.setCurrent = test1
page.10.stdWrap =
page.10.stdWrap.append = TEXT
page.10.stdWrap.append.value.current = 1
page.10.stdWrap.append.setCurrent = test2

The output is: test1

#7 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

I can confirm that it was fixed in master with
https://git.typo3.org/Packages/TYPO3.CMS.git/commitdiff/23b63c2bbf87cd104fd42dca84cb861db0f55f47

The fix is also included in 7.6 and 8.1, but not in 6.2.22.

#8 Updated by Wouter Wolters over 3 years ago

http://review.typo3.org/32466 if you read the comments here it was decided to not backport this fix to the 6.2 branch. It was seen as a breaking change.

#9 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

Thanks, I can understand about the breaking change.

However, a close reading of the code shows that the the commit linked above fixed 1 symptom of the problem, but not a related problem:

page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc =
page.10.value.prioriCalc.wrap = 1
page.10.value.prioriCalc.required = 1

Result: 1+1

page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc.wrap = 1
page.10.value.prioriCalc.required = 1

Result: 2

The reason for this is that the following condition will always evaluate to true:

$functionType !== 'boolean' || $conf[$functionName]

in function TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer->stdWrap, line 2105

(If $conf[$functionName] is unset, then $functionType is always 'array'.)

Unfortunately, my commit also fails to fix this issue, since I accidentally committed the wrong revision. The new line needs to read:

$functionType = $this->stdWrapOrder[$functionName];

instead of:

$functionType = $this->stdWrapOrder[$stdWrapName];

Should I commit a corrected version of my bugfix?

And should I open a new issue, or is it somehow possible to rename this one?

If the bugfix is accepted, then the following check is no longer needed:

https://git.typo3.org/Packages/TYPO3.CMS.git/commitdiff/23b63c2bbf87cd104fd42dca84cb861db0f55f47

It might also be possible (not sure about this) to remove the “isExecuted” check in:

function TYPO3\CMS\Frontend\ContentObject\LoadRegisterContentObject->render

#10 Updated by Wouter Wolters over 3 years ago

Hi Olaf, I'm not so deep in this logic. In my opinion you can update your existing patch with your proposal. Discussion will probably follow after that. You can keep this ticket number.

#11 Updated by Gerrit Code Review over 3 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#12 Updated by Gerrit Code Review over 3 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#13 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

Simplified test-case:

page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc.wrap =

Result (without bug fix): 2

Workaround:

page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc =
page.10.value.prioriCalc.wrap =

Result: 1+1

#14 Updated by Gerrit Code Review over 3 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#15 Updated by Gerrit Code Review over 3 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#16 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

Another symptom of this bug:

page = PAGE
page.10 = TEXT
page.10.value.cObject.wrap = TE|XT
page.10.value.cObject.value = 2

Output: TE2XT

Please notice that page.10.value.cObject itself is nowhere defined.

The wrap TE|XT is evaluated twice. First it is used for the value of page.10.value.cObject (= TEXT), and then it is used for the output as well.

My bug fix solves this problem as well.

#17 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

Even stranger – the following example allows the website visitor to select the content element that will be rendered:

page = PAGE
page.10 = TEXT
page.10.value.cObject.stdWrap.dataWrap = |{gp:sword}
page.10.value.cObject.value = Other text<br>
page.10.value.cObject.template = TEXT
page.10.value.cObject.template.value = <f:form><f:form.textarea /></f:form>

If the URL argument sword=FLUIDTEMPLATE is given, then the output is: <textarea name=""></textarea>FLUIDTEMPLATE

If the URL argument sword=TEXT is given, then the output is: Other text<br>TEXT

If the argument sword is missing, then the output is empty.

The stdWrap properties of type functionName are immune to this bug, because $stdWrapOrder['postUserFunc.'] etc. (with dot) is not defined.

#18 Updated by Gerrit Code Review over 3 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#19 Updated by Gerrit Code Review over 3 years ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#20 Updated by Gerrit Code Review over 3 years ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#21 Updated by Gerrit Code Review over 3 years ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#22 Updated by Gerrit Code Review over 3 years ago

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#23 Updated by Gerrit Code Review over 3 years ago

Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#24 Updated by Gerrit Code Review over 3 years ago

Patch set 12 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#25 Updated by Gerrit Code Review over 3 years ago

Patch set 13 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#26 Updated by Gerrit Code Review over 3 years ago

Patch set 14 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#27 Updated by Gerrit Code Review over 3 years ago

Patch set 15 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#28 Updated by Gerrit Code Review over 3 years ago

Patch set 16 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#29 Updated by Gerrit Code Review over 3 years ago

Patch set 17 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#30 Updated by Gerrit Code Review over 3 years ago

Patch set 18 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008

#31 Updated by Olaf Schmidt-Wischhöfer over 3 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 80 to 100

#32 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF