Automatic HTML escaping in plugin views

Discussions and requests related to new CMSimple features, plugins, templates etc. and how to develop.
Please don't ask for support at this forums!
Post Reply
cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Automatic HTML escaping in plugin views

Post by cmb » Sun Nov 05, 2017 3:24 pm

In the light of recent events (namely https://github.com/cmsimple-xh/memberpages/issues/14 and https://github.com/cmsimple-xh/memberpages/issues/16), I want to antedate the announcement of some recent considerable changes to the view concept of PFW_XH, which is now supposed to provide automatic HTML escaping, at the price of way less flexible templates which might be regarded as a Good Thing[tm], though.

More detailed (but slightly out-dated) info regarding the new view concept is contained in the commit message.

An examplary use-case of this new concept can be found in the WidgetController of Poll_XH, and the respective view template. Also the respective unit test of Pfw_XH may provide some insights.

It seems to me that both Memberpages issues could have been prevented by using this view concept. I want to encourage plugin developers to try it out, and to provide feedback. To do so, you don't need any Controllers; actually you can stick with mostly procedural code. Just install the latest master of Pfw_XH, create and prepare a HtmlView object in a plugin function, put a respective xyz.php in your plugin's views/ folder, and play with it.
Christoph M. Becker – Plugins for CMSimple_XH

Holger
Site Admin
Posts: 3470
Joined: Mon May 19, 2008 7:10 pm
Location: Hessen, Germany

Re: Automatic HTML escaping in plugin views

Post by Holger » Sun Nov 05, 2017 4:41 pm

Ah nice that you keep on working on Pfw_XH :) .
cmb wrote:I want to antedate the announcement of some recent considerable changes to the view concept of PFW_XH, which is now supposed to provide automatic HTML escaping, at the price of way less flexible templates which might be regarded as a Good Thing[tm], though.
I have used the respective classes from Exchange_XH when developing CnC. I thought that I understood the concept. But it would be nice if you could take a look at my code.

In fact: I had some problems to create the "Cache-Info" view.
As you can see in CacheService.php - cacheInfo(), I create some HTML in that method. That's not correct if I'm right (or at least against the concept). And, to output the html, I have to use $view->info = new HtmlString($cache->cacheInfo()); in MainAdminController.php.

BTW: is / was the code from Exchange_XH up to date?

I do not know if I understood all the details but I can tell you that coding this way is much easier when it comes to generate HTML output!

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: Automatic HTML escaping in plugin views

Post by cmb » Sun Nov 05, 2017 6:14 pm

Holger wrote:Ah nice that you keep on working on Pfw_XH :) .
I have to – all this copy&paste leads to such problems as https://github.com/cmb69/twocents_xh/issues/48 (most likely 5 individual plugins are affected). :(
Holger wrote:BTW: is / was the code from Exchange_XH up to date?
Up-to-date regarding the view concept? Not really. Besides my rather old drafts, I've developed three view concepts which are relatively closely related. The first one was that of Pfw_XH 0.1.0, where view data have been injected into the templates as local variables (unescaped) or as view properties (escaped). I didn't like this manual escaping, so in several other plugins (amongst them Exchange_XH) I've refined this to inject the data as view properties (unescaped) and view methods (escaped). The templates could have been checked with a linter (actually, I've developed a draft) so that all echoing went to a call to a $this method, which guaranteed proper escaping. (A drawback was that callables didn't work anymore, but this feature was of somewhat limited use, anyway.) When turning back to Pfw_XH I've noticed that injecting view data as members of View was prone to BC issues (if I'd ever wanted to introduce a new property or method, even a private one, existing views/templates could cause havoc). Therefore I've decided (current Pfw_XH master) to inject the data as local variables, but wrapped in objects so automatic escaping would be possible. The additional benefit: no need to call $this->escape() anymore, which could amount to quite some additional code. However, I'm not really sure whether this view concept is actually viable – there are severe restrictions on what is possible in a template now.
cmb wrote:In fact: I had some problems to create the "Cache-Info" view.
As you can see in CacheService.php - cacheInfo(), I create some HTML in that method. That's not correct if I'm right (or at least against the concept). And, to output the html, I have to use $view->info = new HtmlString($cache->cacheInfo()); in MainAdminController.php.
Ah, I see. A simple improvement could be done by moving the $html stuff at the end of the method into a view/template, and let the method return the View object, like so:

Code: Select all

    public function cacheInfo() {
        // create $files as before
        $view = new View('cache-info');
        $view->count = $count;
        $view->bytes = $bytes;
        $view->files = new HtmlString($files);
        return $view;
    } 
MainAdminController::defaultAction() would not have to be modified. That would still require the HtmlString, but most of the simple HTML could be moved into a template.

A next step could be to tackle $files. Instead of precomputing $files, the respective DirectoryIterator could be passed to the template, where another loop could be made:

Code: Select all

<?php foreach ($this->files as $file):?>
<?php     if (!$fileInfo->isDot() && $fileInfo->isFile()):?>
    <?=$this->escape($this->file->getFilenameName())?><br>
<?php     endif?>
<?php endforeach?>
To avoid the additional if clause in the template (this would not be "allowed" in the new view concept, because it's too hard to read in a template file, in my opinion), you could build up $files like before, but instead of constructing string a simple array of the filenames would be constructed, and that would be passed to the template.

Another option would be to create a view helper class for this view/template, and to pass an instance of it to the template, so in the template you'd could access $helper->getFiles(), $helper->getSize() and $helper->getCount() (or similar). Seems to be overkill here, but still a valid option. And it might simplify unit testing.

Also consider to use generators instead of arrays. I've rarely used them so far, since they require PHP 5.5.0, but they appear to be a really nice feature. Therefore I've added explicit support for them in the new view concept (well, actually there is general support for Iterators, but I don't like to manually create Iterator classes, at least not until this could be done with anonymous classes, which would require PHP 7, though).

Lack of support for anonymous classes is also the reason why I wouldn't recommend to use a FilterIterator instead of the if clause of the DirectoryIterator loop. Having anonymous classes would allow to write:

Code: Select all

$files = (new class(new \DirectoryIterator('.')) extends FilterIterator {
    public function accept() {
        return !$this->current()->isDot() && $this->current()->isFile();
    }
}); 
That might be even more succinct than a respective generator:

Code: Select all

function files() {
    foreach (new \DirectoryIterator('.') as $fileInfo) {
        if (!$fileInfo->isDot() && $fileInfo->isFile()) {
            yield $fileInfo;
        }
    }
}
Holger wrote:I do not know if I understood all the details but I can tell you that coding this way is much easier when it comes to generate HTML output!
If you conclude that this concept simplifies the generation of HTML output, I think that you have understood the idea. :)
Christoph M. Becker – Plugins for CMSimple_XH

Post Reply