Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken TemplateScriptingCompiler in combination with form builder #4444

Closed
Fighter456 opened this issue Aug 7, 2021 · 2 comments
Closed
Assignees

Comments

@Fighter456
Copy link
Contributor

Fighter456 commented Aug 7, 2021

Looks like the merged PR #4431 changes and broke the TemplateScriptingCompiler for constructs like the following in combination with the form builder.

RowFormContainer::create('rowContainer1')->appendChildren([
	RowFormContainer::create('rowContainer2')->appendChild(TitleFormField::create())
])

With the release version of Suite Core 5.4.2 it works as expected and the form gets rendered. But after synchronization of the latest changes from this repository it results in the following error:

Sat, 07 Aug 2021 13:53:52 +0000
Message: Undefined index: 7d4dd95e1ede1750992798f337579968d3503570
PHP version: 7.3.24
WoltLab Suite version: 5.4.2
Request URI: GET /index.php?pr4425/
Referrer: 
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1.1 Safari/605.1.15
Peak Memory Usage: 48173096/134217728
======
Error Class: wcf\system\exception\ErrorException
Error Message: Undefined index: 7d4dd95e1ede1750992798f337579968d3503570
Error Code: 0
File: /Users/dennis/Sites/localhost/products54/lib/system/WCF.class.php (344)
Extra Information: YToxOntpOjA7YToyOntpOjA7czoxNjoiVGVtcGxhdGUgQ29udGV4dCI7aToxO3M6NDEzOiIJPD9waHAgaWYgKCR0aGlzLT52WydjaGlsZCddLT5pc0F2YWlsYWJsZSgpKSB7ID8+CgkJPD89JHRoaXMtPnZbJ2NoaWxkJ10tPmdldEh0bWwoKTs/PgoJPD9waHAgfSA/Pgo8P3BocCB9Cj09PT0+IHVuc2V0KCR0aGlzLT52WydjaGlsZCddKTskdGhpcy0+dlsnY2hpbGQnXSA9ICR0aGlzLT5mb3JlYWNoVmFyc1snN2Q0ZGQ5NWUxZWRlMTc1MDk5Mjc5OGYzMzc1Nzk5NjhkMzUwMzU3MCddWydpdGVtJ107CnVuc2V0KCR0aGlzLT5mb3JlYWNoVmFyc1snN2Q0ZGQ5NWUxZWRlMTc1MDk5Mjc5OGYzMzc1Nzk5NjhkMzUwMzU3MCddKTsKIH0gPz4KPC9kaXY+Cgo8P3BocCBpZiAoICEgZW1wdHkoJHRoaXMtPnZbJ2NvbnRhaW5lciddLT5nZXREZXBlbmRlbmNpZXMoKSkpIHsgPz4KCTxzY3JpcHQgZGF0YS1yZWxvY2F0ZT0idHJ1ZSI+CiI7fX0=
Stack Trace: [{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/templates\/compiled\/0_wcf_1___rowFormContainer.php","line":41,"function":"handleError","class":"wcf\\system\\WCF","type":"::","args":[8,"Undefined index: 7d4dd95e1ede1750992798f337579968d3503570","\/Users\/dennis\/Sites\/localhost\/products54\/templates\/compiled\/0_wcf_1___rowFormContainer.php",41,{"templateName":"[redacted]","application":"[redacted]","sendHeaders":"[redacted]","sourceFilename":"[redacted]","compiledFilename":"[redacted]","metaDataFilename":"[redacted]","metaData":"[redacted]","_length030cc6805daccc6445929143f292b7c1b1ab4c17":"[redacted]","_i030cc6805daccc6445929143f292b7c1b1ab4c17":"[redacted]","_foreach_5d92dbc99cdafb0c5ffea74072591ad304e8a083":"[redacted]","_foreach_7d4dd95e1ede1750992798f337579968d3503570":"[redacted]"}]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/template\/TemplateEngine.class.php","line":346,"args":["[error_during_sanitization]"],"function":"include","class":"","type":""},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/template\/TemplateEngine.class.php","line":585,"function":"display","class":"wcf\\system\\template\\TemplateEngine","type":"->","args":["__rowFormContainer","wcf",false]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/form\/builder\/container\/FormContainer.class.php","line":63,"function":"fetch","class":"wcf\\system\\template\\TemplateEngine","type":"->","args":["__rowFormContainer","wcf",{"container":"[redacted]"},true]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/templates\/compiled\/0_wcf_1___form.php","line":94,"function":"getHtml","class":"wcf\\system\\form\\builder\\container\\FormContainer","type":"->","args":[]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/template\/TemplateEngine.class.php","line":346,"args":["[error_during_sanitization]"],"function":"include","class":"","type":""},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/template\/TemplateEngine.class.php","line":585,"function":"display","class":"wcf\\system\\template\\TemplateEngine","type":"->","args":["__form","wcf",false]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/form\/builder\/FormDocument.class.php","line":448,"function":"fetch","class":"wcf\\system\\template\\TemplateEngine","type":"->","args":["__form","wcf",{"form":"[redacted]"}]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/templates\/compiled\/0_wcf_1_pr4425.php","line":1695,"function":"getHtml","class":"wcf\\system\\form\\builder\\FormDocument","type":"->","args":[]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/template\/TemplateEngine.class.php","line":346,"args":["[error_during_sanitization]"],"function":"include","class":"","type":""},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/page\/AbstractPage.class.php","line":371,"function":"display","class":"wcf\\system\\template\\TemplateEngine","type":"->","args":["pr4425","wcf"]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/page\/AbstractPage.class.php","line":122,"function":"show","class":"wcf\\page\\AbstractPage","type":"->","args":[]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/request\/Request.class.php","line":89,"function":"__run","class":"wcf\\page\\AbstractPage","type":"->","args":[]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/lib\/system\/request\/RequestHandler.class.php","line":119,"function":"execute","class":"wcf\\system\\request\\Request","type":"->","args":[]},{"file":"\/Users\/dennis\/Sites\/localhost\/products54\/index.php","line":11,"function":"handle","class":"wcf\\system\\request\\RequestHandler","type":"->","args":["wcf"]}]

I have created a little plugin for easily reproduction of the error. Please check out this repository: https://github.com/Fighter456/de.dennis-kraffczyk.wsc.pr4425

@dtdesign
Copy link
Member

dtdesign commented Aug 8, 2021

After digging through the code flow I figured out #4431 did not really broke anything. Instead it revealed a fatal flaw in the inner workings of the form builder which does not use sandboxing, eventually causing conflicts due to colliding identifiers.

@dtdesign
Copy link
Member

dtdesign commented Aug 8, 2021

I stand corrected, the form builder uses the sandbox, but the sandbox itself is flawed. It does restore the variable, but does not take of $foreachVars, causing nested calls with the same internal identifier to impact the outer loop. For illustration, this is what actually happens in the example code above:

$this->foreachVars['70cbf93b4a0d3a8c9c2b9dc986b75130e0c308fd'] = [];
// …
foreach ($_foreach_70cbf93b4a0d3a8c9c2b9dc986b75130e0c308fd as $this->v['child']) {
  // invoke `getHtml()`, the following is what happens inside the call!
  $this->foreachVars['70cbf93b4a0d3a8c9c2b9dc986b75130e0c308fd'] = [];
  // ^-- Collision caused by using the identical identifier.
  // …
  foreach ($_foreach_70cbf93b4a0d3a8c9c2b9dc986b75130e0c308fd as $this->v['child']) {
    // …
  }
  // end of `getHtml()`
} 

dtdesign added a commit that referenced this issue Sep 8, 2021
Nesting the same template inside a `foreach` loop that is also accessed inside the nested call will overwrite the values from the outer template due to identical identifiers being used.

The sandbox did not protected `$this->foreachVars` despite being stateful.

See #4431
Fixes #4444
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants