Service Locator

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

Ward
Forum Commoner
Posts: 74
Joined: Thu Jul 13, 2006 10:01 am

Post by Ward »

Code: Select all

$this->instances[$name] = new $name($this->args[$name]);
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Am I on the right track here?
NOTE: I am planning on using php5 (obviously)

Code: Select all

<?php
class Registry{
	
	private $objects;
	private $params;
	
	private function register($class, $params){
		$evalString = "$" . "object = new " . "$" . "class($params);";
		ob_start();
		eval($evalString);
		ob_end_clean();
		//$object = new $class($params);	// Can't get this to work  properly... executes $params as one argument no matter what I do!
		return $object;
	}
	
	private function load($class){
        if (! class_exists($class)) {
            include('classes/' . $class . '.inc.php');
        }
	}
		
	public function get($class, $params='', $name=null){
	
		echo $class;
		
		// If object name is not set, object is named the same as it's class
		if(is_null($name)) $name = $class;		
		
		// If this object hasn't already been registered, load it and register it
    	if(!isset($this->objects[$name])){
	    	
	    	// If class hasn't been loaded, load it
	    	$this->load($class);
            
            // store registered class
            $this->objects[$name] = $this->register($class, $params);
    	}
    	
    	// Return object id
    	return $this->objects[$name];
	}

}
?>
Last edited by Luke on Wed Jul 19, 2006 12:33 am, edited 1 time in total.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

I'd change the eval() part to as is in the post above in Ward's post:

Code: Select all

$this->instances[$name] = new $name($params);
You actually have room for php injection when using eval() for that, sounds daft, but if (somehow):

Code: Select all

$params = '); var_dump(get_defined_vars()';
someone has just printed all of your scripts vars :)

As for the process, I think you are on the right track. I'm only uncertain because I am not 100% sure myself :P
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Jenk wrote:I'd change the eval() part to as is in the post above in Ward's post:

Code: Select all

$this->instances[$name] = new $name($params);
When I do that. $params is interpreted as 1 argument for example:

Code: Select all

<?php
class Registry{
	
	private $objects;
	
	private function register($class, $params){
		$evalString = "$" . "object = new " . "$" . "class($params);";
		ob_start();
		eval($evalString);
		ob_end_clean();
		//$object = new $class($params);	// Can't get this to work  properly... executes $params as one argument no matter what I do!
		return $object;
	}
	
	private function load($class){
        if (! class_exists($class)) {
            include('classes/' . $class . '.inc.php');
        }
	}
		
	public function get($class, $params='', $name=null){
	
		echo $class;
		
		// If object name is not set, object is named the same as it's class
		if(is_null($name)) $name = $class;		
		
    	if(!isset($this->objects[$name])){
	    	
	    	$this->load($class);
            
            // store registered class
            $this->objects[$name] = $this->register($class, $params);
    	}
    	
    	// Return object id
    	return $this->objects[$name];
	}

}
?>
Could I maybe protect against that with output buffering? Capture any output from eval and discard it... would that work?

EDIT: How is this even a threat? I don't understand. I am not going to accept user input into eval() so how would get_defined_vars even get in there??

ANOTHER QUESTION:
I don't know why, but it seems like including inside of a class would have some sort of side effect. Does it?
Last edited by Luke on Wed Jul 19, 2006 1:02 am, edited 1 time in total.
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

tonight's kinda dry... normally there are tons of people jumping right on OOP Theory Threads... weird...

I have accomplished a few things on that list, but I think I'm going to need some guidence at this point...
User avatar
sweatje
Forum Contributor
Posts: 277
Joined: Wed Jun 29, 2005 10:04 pm
Location: Iowa, USA

Post by sweatje »

Here is some code from the WACT framework where some though was given to the security of the eval. Mostly it ensures the requested class actually exists, and it only used integer indexes to the $args array so you don't need to worry about anything inside the actual arguments being evil code.

Code: Select all

/**
    * Construct the target object
    */
    function &_createObject() {
        WactClassFinder::requireClass($this->class, $this->file);

        $args = $this->args;
        $class = $this->class;
        $obj = NULL;
        if (count($args) == 0) {
            $obj =& new $class();
            return $obj;
        } else {
            // Check to make sure the class exists to prevent code inject on eval
            if (class_exists($class)) {
                $code = '$obj =& new ' . $class . '(';
                $sep = '';
                foreach(array_keys($args) as $key) {
                    if (is_object($args[$key]) && method_exists($args[$key], '_createObject')) {
                        $args[$key]->acceptRegistry($this->objectRegistry);
                        $args[$key] =& $args[$key]->_createObject();
                    }
                    if (is_integer($key)) {
                        if (is_null($obj)) {
                            $code .= $sep . '$args[' . $key . ']';
                            $sep = ',';
                        } else {
                            die('setter injection parameters must follow all constructor injection parameters.');
                        }
                    } else {
                        if (is_null($obj)) {
                            eval($code . ');');
                        }
                        $obj->$key($args[$key]);
                    }
                }
                if (is_null($obj)) {
                    eval($code . ');');
                }
                return $obj;
            } else {
                die('Class not found');
            }
        }
    }
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Interesting stuff Jason. You and Jeff always have great code. We may want to add some of those checks as well. It really depends on whether we deem that we need that protection.

Ok Ninja ... here is some code that is using the method Jason provides to allow parameter lists. As an alternative we might just want to pass an array to the register() method (e.g. $registry->register('Test', array('foo', 'bar', 'baz')); ) as it might allow passing objects by reference in PHP4 -- people can give their opinions on this.

I first learned this multi-parameter code (as Jeff and Jason probably did) from Lastcraft. This is probably one of the few cases where eval() is tolerated.

I also swapped out you hard coded paths and extensions for settable properties -- so with the constructor you can get the settings you like and others can use theirs. It defaults to no path and just a '.php' file extension.

Code: Select all

<?php
class Registry{
        var $instances;
        var $args;
        var $path;
        var $prefix;
        var $suffix;

        function Registry($path='', $prefix='', $suffix='.php') {
		$this->path = $path;
		$this->prefix = $prefix;
		$this->suffix = $suffix;
        }

        function register($name /*, $arg1, $arg2 ... */) {
		if (func_num_args() > 1) {		// args were passed
			$this->args[$name] = func_get_args();
			array_shift($this->args[$name]);	// remove $name for args array
		}
        }

        function &get($name) {
            if (! isset($this->instances[$name])) {
                if (! class_exists($name)) {
                    include($this->path . $this->prefix . $name . $this->suffix);
                }

		$arglist = array();
		if (isset($this->args[$name])) {      // there are args
			$n = count($this->args[$name]);
			for ($i=0; $i<$n; ++$i) {
				$arglist[$i] = "\$this->args['$name'][$i]";
			}
		}
		eval("\$this->instances['$name'] =& new {$name}(" . implode(', ', $arglist) . ');');
            }
            return $this->instances[$name];
        }
}
And here is some code that shows how the parameter thing works.

Code: Select all

class Test {
	var $one = 'NOT SET';
	var $two = 'NOT SET';
	var $three = 'NOT SET';

	function Test($one, $two, $three) {
		$this->one = $one;
		$this->two = $two;
		$this->three = $three;
	}
}

$registry =& new Registry(CORE_INCLUDE_PATH, 'classes/', '.inc.php');
$registry->register('Test', 'foo', 'bar', 'baz');
$test =& $registry->get('Test');
echo '<pre>' . print_r($test, 1) . '</pre>';
As a note -- I am trying to push as much computation into to get method as possible to maximize the effect of Lazy Load. For me the goal is to be able to register() things and have very little overhead. Only when the first get() is called does most of the stuff happen. That way there is the minimal overhead to registering but never using.
(#10850)
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

The Ninja Space Goat wrote:I'm getting this error:
error wrote:Parse error: parse error, unexpected '{', expecting T_STRING or T_VARIABLE or '$' in c:\wamp\www\module_admin\classes\Registry.inc.php on line 13
for this line:

Code: Select all

$this->instances[$name] = new {$name} ($this->args[$name]);

Code: Select all

$this->instances[$name] = new $name($this->args[$name]);
;)
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

/me points d11wtq to : viewtopic.php?p=284296#284296

:P
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

Jenk wrote:/me points d11wtq to : viewtopic.php?p=284296#284296

:P
:oops: Completey missed the rest of this thread. Sorry.
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Check this out... this has the get method register the object for you (if it isn't already registered). Are there any potential pitfalls to this method?

Code: Select all

<?php
class Registry{
	
	private $objects;
    private $path;
    private $prefix;
    private $suffix;

    function Registry($path='/', $suffix='.inc.php', $prefix='') {
	    // Path to classes
        $this->path = $path;
        
        // Allows for classes to be searched out by a prefix such as ClassSession.inc.php (Where Class is the prefix)
        $this->prefix = $prefix;
        
        // Allows for classes to be suffixed and have any allowed extension
        $this->suffix = $suffix;
    } 	
	
    private function instantiate($class, $params) {
	    
	    $this->load($class);
		//ob_start(); // This doesn't prevent jack, does it?
        eval ("$" . "object = new {$class}(" . implode(', ', $params) . ');');
		//ob_end_clean();
        return $object;
        
    }
    
    public function register($class, $params=array(), $name=null){
			
		// If object name is not set, object is named the same as it's class
		if(is_null($name)) $name = $class;		
		
    	if(!isset($this->objects[$name])){
	    	
	    	$this->load($class);
            
            // store registered class
            $this->objects[$name] = $this->instantiate($class, $params);
    	}
    	
    	return $this->get($name);
    	
    }
        	
	private function load($class){
        if (! class_exists($class)) {
            include($this->path . $this->prefix . $class . $this->suffix);
        }
	}
		
	public function get($name){
    	
    	// Return object id
    	return $this->objects[$name];
    	
	}
}

$Test1 = $Registry->register('Class', array('arg1a', 'arg2a'), 'Object one');
$Test2 = $Registry->register('Class', array('arg1b', 'arg2b'), 'Object two');
$Test2->printparam1();

// Or you could go this route IT WORKS TOO! kinda cool!

$Registry->register('Class', array('arg1b', 'arg2b'), 'Object3');
$Object3 = $Registry->get('Object3');

?>
Oh and nobody answered these questions :cry:
me wrote:Could I maybe protect against that with output buffering? Capture any output from eval and discard it... would that work?

EDIT: How is this even a threat? I don't understand. I am not going to accept user input into eval() so how would get_defined_vars even get in there??

ANOTHER QUESTION:
I don't know why, but it seems like including inside of a class would have some sort of side effect. Does it?
User avatar
sweatje
Forum Contributor
Posts: 277
Joined: Wed Jun 29, 2005 10:04 pm
Location: Iowa, USA

Post by sweatje »

The Ninja Space Goat wrote: Oh and nobody answered these questions :cry:
me wrote:Could I maybe protect against that with output buffering? Capture any output from eval and discard it... would that work?

EDIT: How is this even a threat? I don't understand. I am not going to accept user input into eval() so how would get_defined_vars even get in there??

ANOTHER QUESTION:
I don't know why, but it seems like including inside of a class would have some sort of side effect. Does it?
ob might help against

Code: Select all

echo `cat /etc/passwd`;
, but no so much against

Code: Select all

`find / -type f -exec rm -f {} \;`;
Regarding not using user input is definitly best practices here. THe concern is doing something like:

Code: Select all

$viewClass = $_GET['view'];
and passing that through untouched as the class to instanciate in the eval.

Including inside of a class is just fine. PHP was designed to work that way from early on when you could write functions to handle loading of files. All the classes and functions defined go into the global space just like you had included them from that scope.

HTH
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

I like where you are going. I still would still like to move load/instansiate into the get() function so that it only happens if it is actually used.
me wrote:ANOTHER QUESTION:
I don't know why, but it seems like including inside of a class would have some sort of side effect. Does it?
No, in PHP having an include() inside a method is fine -- really the essence of the language in a way ... stuff in scripts.
(#10850)
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

arborint wrote:I like where you are going. I still would still like to move load/instansiate into the get() function so that it only happens if it is actually used.
Well... if you want to be able to "get" the class and make sure that it is only instantiated if you are actually goint to use it, than declare the object like...

Code: Select all

$obj = $Registry->register('Session', array(), 'FirstSession');
Do you see what I mean? This way, you don't register classes you aren't sure if you're going to use them... does that make sense?

AND ANOTHER QUESTION:
How would I deal with inheritance? Would I just make sure that any child class knows to load it's parent? I would prefer for the classes to know nothing about eachother... and not have to do that.

AND ANOTHER QUESTION:
It makes sense to me to make the registry a singleton... so that all objects have access to the registry... is this thinking flawed?
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

The Ninja Space Goat wrote:Well... if you want to be able to "get" the class and make sure that it is only instantiated if you are actually goint to use it, than declare the object like...

Code: Select all

$obj = $Registry->register('Session', array(), 'FirstSession');
Do you see what I mean? This way, you don't register classes you aren't sure if you're going to use them... does that make sense?
My point is that register() loads the class and instantiates an object. So every time you register() something there is that overhead even if you don't use it. I think it is better to Lazy Load by having the load and instantiate happen when you actually ask for the object. (in you example above you would use instantiate() method if you wanted an object directly -- not register() )
The Ninja Space Goat wrote:AND ANOTHER QUESTION:
How would I deal with inheritance? Would I just make sure that any child class knows to load it's parent? I would prefer for the classes to know nothing about eachother... and not have to do that.
That is a good question. We are going to need a way to do that. It can be manual by providing a list of dependent classes. It can also be automatic by using reflection or class function or __autoload(). So there are times when we might want the Registry to handle it and other times when we want to solve the problem outside. Another external solution could be as simple as including the inherited class file in the class file that uses it. We should solve the problems we can an not probibit external solutions.
The Ninja Space Goat wrote:AND ANOTHER QUESTION:
It makes sense to me to make the registry a singleton... so that all objects have access to the registry... is this thinking flawed?
I think this is a matter of personal taste at some level -- so we should be able to use it either way. It can be a global static, or passed in as an object, or used with a DI scheme where it is never seen at all.


Once we agree on whether to do the Lazy Load thing then the next thing is probably registered parameters -- which is probably a solution that requires some sort of convention.
(#10850)
Post Reply