Namespace overriding.

[Michael Morris]

I've had some success with this, though I'm still a bit gun shy on it.

namespace PNL;
 
/**
 * Extend the root namespace function htmlentities to allow the first
 * argument to be an array or object.  If it is, recurse over it and apply
 * htmlentities to all members. KEYS WILL NOT CHANGE, so if they have html 
 * entities in them you still need to call htmlentities on the element.
 * 
 * @param mixed $mixed
 * @param unknown_type $flags
 * @param unknown_type $charset
 * @param unknown_type $double_encode
 */
function htmlentities($mixed, $flags=null, $charset=null, $double_encode=null) {
	if (is_array($mixed) || is_object($mixed)) {
		foreach ($mixed as &$var) {
			$var = htmlentities($var, $flags, $charset, $double_encode);
		}
 
		return $mixed;
	} else {
		return \htmlentities($mixed, $flags, $charset, $double_encode);
	}
}

Thoughts (either on the overriding in general or the function itself)?

[ScallioXTX]

1) It might be confusing, especially if your function is in long php files, that you're seemingly calling php's default htmlentities function which is actually your own. In this case it shouldn't really matter as the function is basically the same but extended, but if you drastically change a function people will have a hard time grasping why your code does what it does.

2) Do you only have the PNL namespace? I ask because from what I read (I'm not too familiar with functions in namespaces), this function will only override the global function in the PNL namespace, not in subnamespaces of PNL, which makes it less interesting imo.

3) I don't see the use of else in your code since the if part already returns something, which makes everything after the if body implicitly the else bodu, since it will only be executed when the if condition is not true.
I would write it like this:

namespace PNL;
 
/**
 * Extend the root namespace function htmlentities to allow the first
 * argument to be an array or object.  If it is, recurse over it and apply
 * htmlentities to all members. KEYS WILL NOT CHANGE, so if they have html 
 * entities in them you still need to call htmlentities on the element.
 * 
 * @param mixed $mixed
 * @param unknown_type $flags
 * @param unknown_type $charset
 * @param unknown_type $double_encode
 */
function htmlentities($mixed, $flags=null, $charset=null, $double_encode=null) {
	if (is_array($mixed) || is_object($mixed)) {
		foreach ($mixed as &$var) {
			$var = htmlentities($var, $flags, $charset, $double_encode);
		}
 
		return $mixed;
	}
        return \htmlentities($mixed, $flags, $charset, $double_encode);
}

That may just be a matter of taste of course, I don't know.

[Michael Morris]

1) It might be confusing, especially if your function is in long php files, that you're seemingly calling php's default htmlentities function which is actually your own. In this case it shouldn't really matter as the function is basically the same but extended, but if you drastically change a function people will have a hard time grasping why your code does what it does.

I'm aware of that, but that's the reason this is an override - it does what the original does and some more. If it changed things, even slightly, I'd choose a different name.

2) Do you only have the PNL namespace? I ask because from what I read (I'm not too familiar with functions in namespaces), this function will only override the global function in the PNL namespace, not in subnamespaces of PNL, which makes it less interesting imo.

I typically use one namespace for the core framework and one for the project. The functions live in a 'definitions.php' file inside the namespace.

Basically, I don't use the PHP recommendation for namespaces in my autoloader, which amounts to reflecting file structure in code structure, and will create major headaches for anyone using it, and I don't see it standing the test of time because it's ill thought out. Instead, classes live in files that share their name, and directories are ignored unless they have a .ns extension. The structure looks like this...

/php
  /PNL.ns
    Dispatcher.php
    definitions.php
    /arrays
      ReadOnlyArray.php
      StaticKeyArray.php
    /libraries
      TemplateLibrary.php
      TemplateLibraryFactory.php

  /TNT.ns
    Header.php
    definitions.php

This allows me to organize my classes into directories however I feel like without facing coding ramifications. I use namespaces when I *need* to start a new naming scope.

The autoloader is set up so that the first time it loads a class from a new namespace loads any definition files for that namespace. In the highly unlikely event a namespace function or constant must be referenced without loading any class from a namespace the definition load can be triggered with new \MyNameSpace\Definitions(); The autoloader will create an empty class to prevent an engine class. The unfortunate tradeoff is that "definitions" becomes unavailable as a class name in all namespaces.

I would write it like this:

namespace PNL;
 
function htmlentities($mixed, $flags=null, $charset=null, $double_encode=null) {
	if (is_array($mixed) || is_object($mixed)) {
		foreach ($mixed as &$var) {
			$var = htmlentities($var, $flags, $charset, $double_encode);
		}
 
		return $mixed;
	}
        return \htmlentities($mixed, $flags, $charset, $double_encode);
}

That may just be a matter of taste of course, I don't know.

EDIT - nevermind.

[Jeff Mott]

I'll grant you that this seems like a low risk intrusion into the global scope, but I still think the drawbacks outweigh the benefits. I think the most frequent drawback you'll encounter is ScallioXTX's #1: confusion. Other devs who try to read your code could be understandably confused when they see htmlentities behaving differently than the php.net docs say it should. Your custom escaper could instead be a method ($pnlTemplate->escape()) or even just a prefixed function (pnl_htmlentities()). I personally don't think either of those alternatives are so terrible to warrant monkey patching.

[Jeff Mott]

I've been dealing with a monkey patching issue today, and it made me think of this thread. (The issue I'm currently dealing with is in JavaScript, but the principle is the same.) The site I'm working on decided to "enhance" the global Date object. At the heart of my problem is the fact that it overrode the toString method. The enhanced toString method accepts a format parameter, and if no format parameter is provided, then it calls the original toString. Seems harmless... except another third-party script also decided to "enhance" the Date object's toString method in their own way, and the two aren't compatible.

Any time you alter the global environment, you risk conflicts with other code that might also alter the global environment, or code that might depend on the global environment behaving exactly like it's expected to.

I strongly recommend that you avoid at all costs any alterations to anything global.

[Michael Morris]

The \PNL namespace is not a global environment.

[Jeff Mott]

Ahh, my mistake. I thought you were overriding the global htmlentities. But you're right, that isn't what's happening. Feel free to ignore my posts.

[Michael Morris]

Your point isn't without merit, and I have decided to go ahead and attach that method to the Template class as #escape simply because I don't want to encourage html manipulation elsewhere. In PHP it isn't possible to force people to use a custom version of a function, but it could still be a problem for a maintenance programmer reading code within the namespace.

[Jeff Mott]

As a bonus, if you're interested, I like it when the behavior of a templating library is to escape by default. It makes it much harder to make a mistake and leave an XSS flaw somewhere. Here's a quick and simple way that could be done.

class Escaper
{
    private $value;
    
    public function __construct($value)
    {
        $this->value = $value;
    }
    
    public function __toString()
    {
        return htmlspecialchars($this->value, ENT_QUOTES);
    }
    
    public function getRaw()
    {
        return $this->value;
    }
} 

Escaper wraps some string value, and when an Escaper object is used in a string context, it returns the value escaped.

$taintedContent = '<b>Hello</b>, <i>World</i>';
$safeContent = new Escaper($taintedContent);

// Now we can use it in a string, and escaping happens automatically
echo '<p>' . $safeContent . '</p>';

// If we're sure we want the unescaped value, we can still get that
echo '<p>' . $safeContent->getRaw() . '</p>'; 

[Michael Morris]

My templates nest, that auto behavior would effectively lead to htmlentities(htmlentities()), which is counter productive.

[TomB]

Ignoring the topic of overriding inbuilt functions, looping through an array/object and applying htmlentities to each field seems like poor separation of concerns. The escaping should be done at the output stage, not when (presumably) you're assigning variables to the template.

The __toString() method shouldn't cause double escaping because it will only get called once, Jeff Mott's escaper function will work for arrays/objects with a little modification:

class Escaper 
{ 
    private $value; 
     
    public function __construct($value) 
    { 
        $this->value = $value; 
    } 
     
    public function __toString() 
    { 
        return htmlspecialchars($this->value, ENT_QUOTES); 
    } 
     
    public function getRaw() 
    { 
        return $this->value; 
    } 

   public function __get($name) {
       if (is_object($this->value)) return htmlentitites($this->value->$name);
       else if (is_array($this->value)) return htmlentitites($this->value[$name]);
   }
} 

<p>Your name is ' . <?=$user->firstname; ?> . ' ' . <?=$user->lastName;?></p>

There's no way that can lead to double escaping.