Destramic Posted August 7, 2016 Share Posted August 7, 2016 hey guys i'm currently writing a head helper and probably down to my bad design i'm unable to return array stored in my placeholder here is my head helper: <?php namespace View\Helper; use Exception\Exception as Exception; class Head { private $_tags = array( 'script' => 'Helper\Head\Script', 'link' => 'Helper\Head\Link' ); public function __get($key) { if (isset($this->_tags[$key])) { if (is_object($this->_tags[$key])) { return $this->_tags[$key]; } else if (class_exists($this->_tags[$key])) { $this->_tags[$key] = new $this->_tags[$key]; return $this->_tags[$key]; } } return null; } public function __call($key, $arguments) { if (!is_null($this->$key)) { $this->$key->set($arguments[0]); return $this->$key; } return null; } } pre set scripts within bootstrap $view->head->script(array( 'src' => 'test' ))->prepend(array( 'src' => 'hello' )); append to the scripts witin controller $this->view->head->script->append(array( 'src' => 'bye' )); script class <?php namespace Helper\Head; use Exception\Exception as Exception; use Placeholder\Placeholder as Placeholder; use Head\View\Script as Script; class Script extends Placeholder { public function __toString() { print_r($this->get()); // $script = new Script(array('src' => 'hello')); // return $script->render(); return 'return'; } } when __toString is executed in my view, the array stored in my placeholder is empty. <?= $this->head->script; ?> Array() i know the problem all lies down to the head helper, because when using the __call or __get method it returns the script object which has not reference of previous appends/depends of the script tag. ummm i hope i explained myself enough and that there is a simple way of makig this work to how i want. previously i used to something like this but it looks ugly when setting scripts public function __call($method, $parameters) { if (preg_match('/^(?P<action>set|(ap|pre)pend)]*(_)*(?P<type>title|meta|script|link)$/', $method, $matches)) { $parameters = $parameters[0]; $tag_name = $matches['type']; $action = $matches['action']; $tag = $this->get_tag($tag_name); return $tag->$action($parameters); } } // $this->view->head->set_script()->append(); if you require to look at any code please let me know...thank you for your time Quote Link to comment Share on other sites More sharing options...
Solution Jacques1 Posted August 7, 2016 Solution Share Posted August 7, 2016 (edited) You need to narrow the problem down. Right now, there are way too many classes and methods involved, most of which we don't know and don't need to know. Where is the problem? In Head? Script? Placeholder? In a properly designed OOP infrastructure, objects can be tested individually. Do that, preferrably with automated unit tests. Your code is also bloated and relies too much on magic. Why on earth does the Head class need 20(!) lines of code to instantiate two classes? And is that all the class does, hold two unnecessarily hard-coded class references which make it impossible to use any other script class? The whole approach seems questionable. You're appearently trying to reinvent HTML in an object-oriented manner, and the only thing which sets you apart is that Destramic-HTML requires 10 times as many lines as plain HTML. Have you considered using an actual template engine like Twig? OOP is great for many tasks, but it's horrible for describing the structure of a document. That's what declarative languages like HTML are for. Edited August 7, 2016 by Jacques1 Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 7, 2016 Author Share Posted August 7, 2016 (edited) You need to narrow the problem down. Right now, there are way too many classes and methods involved, most of which we don't know and don't need to know. Where is the problem? In Head? Script? Placeholder? In a properly designed OOP infrastructure, objects can be tested individually. Do that, preferrably with automated unit tests. good point...ok i did just that use MVC\View\View as View; $view = new View; $view->head->script(array( 'src' => 'test4' ))->append(array( 'scr' => 'hi4' )); $view->head->script->append(array( 'src' => 'test3' ))->append(array( 'scr' => 'hi3' )); echo $view->head->script; use MVC\View\Helper as Helper; $helper = new Helper; $helper->head->script(array( 'src' => 'test4' ))->append(array( 'scr' => 'hi4' )); $helper->head->script->append(array( 'src' => 'test3' ))->append(array( 'scr' => 'hi3' )); echo $helper->head->script; use View\Helper\Head as Head; $head = new Head; $head->script(array( 'src' => 'test2' ))->append(array( 'scr' => 'hi2' )); $head->script->append(array( 'src' => 'test3' ))->append(array( 'scr' => 'hi3' )); echo $head->script; use Helper\Head\Script as Script; $script = new Script; $script->set(array( 'src' => 'test' ))->append(array( 'scr' => 'hi' )); echo $script; which returns : // from view Array ( [0] => Array ( [src] => test4 ) [1] => Array ( [scr] => hi4 ) [2] => Array ( [src] => test3 ) [3] => Array ( [scr] => hi3 ) ) // from helper Array ( [0] => Array ( [src] => test4 ) [1] => Array ( [scr] => hi4 ) [2] => Array ( [src] => test3 ) [3] => Array ( [scr] => hi3 ) ) // from head Array ( [0] => Array ( [src] => test2 ) [1] => Array ( [scr] => hi2 ) [2] => Array ( [src] => test3 ) [3] => Array ( [scr] => hi3 ) ) // from script Array ( [0] => Array ( [src] => test ) [1] => Array ( [scr] => hi ) ) well i didn't expect it all to return results as it did...now i'm scratchig my head a bit here why when called like so does it return an empty array // controller $this->view->head->script(array( 'src' => 'test' )); // view <?= $this->head->script; ?> Your code is also bloated and relies too much on magic. Why on earth does the Head class need 20(!) lines of code to instantiate two classes? And is that all the class does, hold two unnecessarily hard-coded class references which make it impossible to use any other script class? i want to use it for all head tags doctype, link, title etc...its how i see modern frameworks set thier head and then just call when needed in the view...i just asume that is the correct way as i have no real idea of what is the best practice other than what i see or unless you guys tell me different The whole approach seems questionable. You're appearently trying to reinvent HTML in an object-oriented manner, and the only thing which sets you apart is that Destramic-HTML requires 10 times as many lines as plain HTML. Have you considered using an actual template engine like Twig? OOP is great for many tasks, but it's horrible for describing the structure of a document. That's what declarative languages like HTML are for. i suppose i could just scrap the whole idea altogether and just put html where it needs to go, but i find with this method i could have all control of the head inside my controller....i did notice a post of yours metioning a template engine with thier mcv before, but its maybe something i coiuld look into if that is a good way to go but with my method i'm able to set all default links, scripts, title etc and just call in my view, with the option of appending inside the controller too. Edited August 7, 2016 by Destramic Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 7, 2016 Share Posted August 7, 2016 why when called like so does it return an empty array // controller $this->view->head->script(array( 'src' => 'test' )); // view <?= $this->head->script; ?> Are you sure you're referencing the same view object in both locations and running the code in the right order? i want to use it for all head tags doctype, link, title etc...its how i see modern frameworks set thier head and then just call when needed in the view...i just asume that is the correct way as i have no real idea of what is the best practice other than what i see or unless you guys tell me different Which frameworks? i suppose i could just scrap the whole idea altogether and just put html where it needs to go, but i find with this method i could have all control of the head inside my controller....i did notice a post of yours metioning a template engine with thier mcv before, but its maybe something i coiuld look into if that is a good way to go but with my method i'm able to set all default links, scripts, title etc and just call in my view, with the option of appending inside the controller too. Why should the controller choose JavaScript files? That's clearly a task of the presentation layer. But even if you somehow enncounter a scenario where the controller actually needs to make that decision, you could just pass an array of URLs to the view. I can't see the justification for spending such an enormous amount of time on those HTML helpers. They aren't very useful most of the time, they might actually distract you from better solutions, and they can always be emulated. Feel free to try this approach, especially when the framework is mostly for learning. But don't spend too much time on it. Instead of literally reimplementing the complete HTML standard, pick a few elements (scripts and styles make sense) and do a reality check before you continue. Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 7, 2016 Author Share Posted August 7, 2016 Are you sure you're referencing the same view object in both locations and running the code in the right order? i found the problem which was so stiupid of me...i hate to say what i did wrong sorry! Which frameworks? well i've never used a framework other than my own...i thought zend framrwork would be good so i read the files and manuel and based mine around that the helpers are only for the script, link, title and meta i wouldnt create helpers for every html element i use a decorator like so <?php namespace Head\View; use Exception\Exception as Exception; use HTML\Decorator_Abstract as Decorator_Abstract; use HTML\Decorator_Interface as Decorator_Interface; class Script Extends Decorator_Abstract implements Decorator_Interface { CONST ATTRIBUTES = array( 'charset', 'src', 'type' ); CONST BOOLEAN_ATTRIBUTES = array( 'async', 'defer' ); CONST ATTRIBUTE_VALUES = array( 'type' => array( 'text/javascript', 'text/ecmascript', 'application/ecmascript', 'application/javascript' ) ); protected function opening_tag() { return sprintf("<script%s>", $this->attributes()); } protected function closing_tag() { return '</script>'; } public function render() { return $this->opening_tag() . $this->closing_tag(); } } regarding using a template engine with the view, i don't really see the point i'm able to extract variables to the html like so $this->view->name = 'Destramic'; and use in my template like so $this->name; wouldn't using a template engine just over-complicate thing? with all the var, if, foreach syntax? thank for your help Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 7, 2016 Share Posted August 7, 2016 i'm able to extract variables to the html like so $this->view->name = 'Destramic'; and use in my template like so $this->name; Then your templates are full of cross-site scripting vulnerabilities. If you use plain PHP, you must HTML-escape every single variable, by hand. Forgetting it just once can compromise the entire application. Security is the biggest problem when misusing PHP as a template engine. A “PHP template” is technically a full-blown application which may do absolutely anything: issue shell commands, write files, communicate with other hosts. Why on earth should a template have this power? Its sole purpose is to generate HTML, so it shouldn't do anything other than that. PHP also lacks modern templating features like inheritance and custom syntax. You're stuck in the late 90s when PHP was a cute little language for your personal homepage. Last but not least, the PHP template syntax is just arcane. Don't tell me that you prefer this: <?php if (!$blog_posts): ?> no entries <?php else: foreach ($blog_posts as $blog_post): ?> <li><?= html_escape($blog_post['title']) ?></li> <?php endforeach; endif; ?> over this: {% for blog_post in blog_posts %} <li>{{ blog_post.title }}</li> {% else %} no entries {% endfor %} Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 9, 2016 Author Share Posted August 9, 2016 sorry for the late reply i have been doing some research regarding this xss...i didn't realise a web page could be so vulnerable in so many asspects of xxs. so every string must be encoded using htmlspecialchars($string, ENT_QUOTES, 'UTF-8'); so i need to encode the attributes set in my view helper?...that would imply that the design would xxs? i'm sorry i don't understand fully how a user browsing can inject code from the following example, unless the $id is changed inside the file...how is it possible to do it via the browser? <php $id = "test-id" ?> <div id="{$id}">this is a test</div> but i do understand that with user input such as my forum reply it needs to be encoded to ensure no xss {% for blog_post in blog_posts %} <li>{{ blog_post.title }}</li> {% else %} no entries {% endfor %} i'm not going to argue with you jacques, this looks alot easier on the eye thank you for your help Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 9, 2016 Author Share Posted August 9, 2016 sorry it should be: <php $id = "test-id"; ?> <div id="<?php echo $id; ?>">this is a test</div> Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted August 9, 2016 Share Posted August 9, 2016 You escape every variable within the template for its specific context. This usually means HTML-escaping with htmlspecialchars(), but not always. Some contexts require additional measures (e. g. the href attribute of an anchor due to the risk of JavaScript injection through javascript: and data: URLs), some contexts are inherently unsafe (like the content of a script element). Trying to skip the escaping for “safe” values is a bad idea. First off, escaping is not only about security. It's about making sure that the value won't interfere with the context. For example, the hard-coded name “O'Reilly” may be perfectly secure, but it will still blow up your application if you insert that name straight into a single-quoted SQL string. Secondly, trying to assess the risk of every single value is far too error-prone and expensive. Not only would you have to be a perfect programmer who never makes a wrong decision; you'd also have to re-evaluate the entire application on every change, because a value which used to be “safe” may now be unsafe. A far more realistic approach is to escape everything. Look at the context, choose the right escaping strategy, and then escape the value – regardless of where it happens to come from. Quote Link to comment Share on other sites More sharing options...
Destramic Posted August 11, 2016 Author Share Posted August 11, 2016 Thanks one again for the great advice jacques Well im going to scrap the head helper nonsence and create a template engine myself...but still keep the idea of setting an array of header tags so i can call all globals Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.