URI parsing and handling class

Question

I wrote a simple class to deal with parsing and handling URIs for the project I'm currently working on, since it relies a lot on that functionality (eg the Router class and some HTML helpers). Here is the code already commented:

<?php # URI class class uri { # URI scheme public $scheme; # URI host public $host; # URI user public $user; # URI password public $password; # URI port public $port; # URI path public $path; # URI query public $query; # URI fragment public $fragment; # Default constructor for the URI public function __construct($uri = null) { # No URI is set, the current request URI will be used if($uri == null) { # Predefine the empty parts array $uri_parts = array(); # Extract the scheme part of the URI and add it the to array if https if(array_key_exists('HTTPS', $_SERVER)) array_push($uri_parts, 'https://'); # Extract the scheme part of the URI and add it the to array if http if(!array_key_exists('HTTPS', $_SERVER)) array_push($uri_parts, 'http://'); # Extract the host part of the URI and add it to the array if https if(array_key_exists('SERVER_NAME', $_SERVER)) array_push($uri_parts, $_SERVER['SERVER_NAME']); # Extract the request URI itself and add it to the array if(array_key_exists('REQUEST_URI', $_SERVER)) array_push($uri_parts, $_SERVER['REQUEST_URI']); # Set the URI string to be processed later $uri = implode($uri_parts); } # An URI is provided in string form if($uri != null and !is_array($uri)) { # Regular Expression from RFC 2396 (appendix B) preg_match('"^(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?"', $uri, $matches); # Extract the URI scheme from the string if(array_key_exists(2, $matches)) $this->scheme = $matches[2]; # Extract the URI authority from the string if(array_key_exists(4, $matches)) $authority = $matches[4]; # Extract the URI path from the string if(array_key_exists(5, $matches)) $this->path = $matches[5]; # Extract the URI query from the string if(array_key_exists(7, $matches)) $this->query = $matches[7]; # Extract the URI fragment from the string if(array_key_exists(9, $matches)) $this->fragment = $matches[9]; # Extract username, password, host and port from authority preg_match('"(([^:@]*)(:([^:@]*))?@)?([^:]*)(:(.*))?"', $authority, $matches); # Extract the URI username from the authority part if(array_key_exists(2, $matches)) $this->user = $matches[2]; # Extract the URI password from the authority part if(array_key_exists(4, $matches)) $this->password = $matches[4]; # Extract the URI hostname from the authority part if(array_key_exists(5, $matches)) $this->host = $matches[5]; # Extract the URI port from the authority part if(array_key_exists(7, $matches)) $this->port = $matches[7]; # Get the rid of the leading path slash if its present in the string if(substr($this->path, 0, 1) == '/') $this->path = substr($this->path, 1); # Get the rid of the ending path slash if its present in the string if(substr($this->path, -1, 1) == '/') $this->path = substr($this->path, 0, -1); # Set the path to an array by exploding it with the default delimiter $this->path = explode('/', $this->path); # Make sure that the array is really empty if the path was empty from the beginning if(array_key_exists(0, $this->path) and empty($this->path[0])) $this->path = array(); # Split the query string into an array parse_str($this->query, $query); $this->query = $query; } # An URI is provided in array form if($uri != null and is_array($uri)) { # Set the scheme of the URI if(array_key_exists('scheme', $uri)) $this->scheme = $uri['scheme']; # Set the hostname of the URI if(array_key_exists('host', $uri)) $this->host = $uri['host']; # Set the user of the URI if(array_key_exists('user', $uri)) $this->user = $uri['user']; # Set the password of the URI if the pass key is used if(array_key_exists('pass', $uri)) $this->scheme = $uri['pass']; # Set the password of the URI if the password key is used if(array_key_exists('password', $uri)) $this->password = $uri['password']; # Set the port of the URI if(array_key_exists('port', $uri)) $this->port = $uri['port']; # Set the path of the URI if(array_key_exists('path', $uri)) $this->path = $uri['path']; # Set the query of the URI if(array_key_exists('query', $uri)) $this->path = $uri['query']; # Set the fragment of the URI if the anchor key is used if(array_key_exists('anchor', $uri)) $this->fragment = $uri['anchor']; # Set the fragment of the URI if the fragment key is used if(array_key_exists('fragment', $uri)) $this->fragment = $uri['fragment']; } } # Returns the URI as a string # You can specify what parts should be built public function build(array $parts = array('scheme', 'user', 'password', 'host', 'path', 'query', 'fragment')) { # Predefine an array of parts $uri_parts = array(); # Attach the URI scheme to the parts array if not empty if(in_array('scheme', $parts) and !empty($this->scheme)) array_push($uri_parts, $this->scheme . '://'); # Attach the URI user to the parts array if not empty if(in_array('user', $parts) and !empty($this->user)) array_push($uri_parts, $this->user); # Attach the URI password to the parts array if not empty if(in_array('password', $parts) and !empty($this->password)) array_push($uri_parts, ':' . $this->password); # Attach the URI user prefix to the parts array if not empty if(in_array('user', $parts) and !empty($this->user)) array_push($uri_parts, '@'); # Attach the URI host to the parts array if not empty if(in_array('host', $parts) and !empty($this->host)) array_push($uri_parts, $this->host.'/'); # Attach the URI path to the parts array if not empty if(in_array('path', $parts) and count($this->path) > 0) array_push($uri_parts, implode('/', $this->path)); # Attach the URI query to the parts array if not empty if(in_array('query', $parts) and count($this->query) > 0) array_push($uri_parts, '?' . http_build_query($this->query, '', '&')); # Attach the URI fragment to the parts array if not empty if(in_array('fragment', $parts) and !empty($this->fragment)) array_push($uri_parts, '#' . $this->fragment); # Return the URI string return implode($uri_parts); } # Default convertion to a string, returns the complete URI public function __toString() { return $this->build(); } } 

The most simple usage of the class would be something like that

$uri = new uri; $uri->path = array('some', 'real', 'path'); echo $uri; 

which would output http://localhost/some/real/path given that you test it on localhost.

1. Are there any problems you see with the possible usage of that class?
2. Is there any way you would improve its functionality?

EDIT: Please notice that leading and trailing slashes are stripped from the path string on purpose, because they are unwanted in my current environment and should never appear in the URI.

Answer 1

The big one is all your properties (variables) are public! This is really bad, because it means that an instance of this object has no real control whatsoever over what its own state is. Any external agent can inject anything it wants into public properties and it can all too easily result, through accident of malicious intent, in the internal state of the class being corrupted.

You need tighter control over your class state. There are two methods involved with doing this. The first is simply to set your instance variables protected instead of public. Now they can't be messed with from outside.

Of course now you also don't have any legitimate access to the instance variables either, but this is where the second technique comes in. For each instance variable you want to make accessible to the outside in some way, you need to create a pair of methods for getting and setting its value.

Example:

class Uri { protected $protocol = ''; // Getter public function getProtocol () { return ($this -> protocol); } // Setter public function setProtocol ($newProto) { $this -> protocol = $newProto; return (true); } } 

At first glance this might seem like unnecessary extra work, but because this is the most basic example of getters and setters it doesn't illustrate the benefits that such an approach bestows on the programmer.

First, you can now have read-only/write-only properties. If you need a read-only property, for example, simply don't implement a setter for the property in question. Now it can't be modified from outside.

More importantly, it allows a lot of control over what happens when an instance variable has changed. It can be used to restrict their values to valid states, notify some other internal part of the class that a property has changed, and so on.

Say you want to limit the protocol to http and https in my above example class:

public function setProtocol ($newProto) { switch ((string) $newProto) // Casting to the expected data type is a good idea here because PHP type juggling can cause serious screwups otherwise { case 'http' : case 'https' : $this -> protocol = $newProto; return (true); break; default : throw new InvalidArgumentException ('Only HTTP or HTTPS allowed'); break; } } 

As you can see, it is now impossible to set the protocol variable to any state other than the ones you allow.

This isn't the extent of the power of getters and setters, the benefits they confer are huge. You can, for example, use a getter to do lazy loading of instance variables. Say you have one instance variable that isn't used that often and requires an expensive operation (a DB lookup, a file load and parse, connection to a potentially slow external server, etc), you can write your class to only load the data on demand, and have the getter do the actual data fetch the first time its called. It can then cache the result and skip the data fetching state on subsequent calls. Doing this improves performance because the expensive operation is only done if its results are actually needed, and it's only done once per object instance.

Answer 2

Information Hiding

Your class contains all public properties and methods.

This means that users of the class are exposed to all of the internal properties, causing the following problems:

1. Confusion - With so many publicly accessible properties and methods the user doesn't know which property or method to interact with first. There are more choices for them to look at.
2. Accident Prone - The user has full power over the public properties. This could lead them to modify them in harmful ways (accidentally). Without restricting the access to only tested public methods we cannot limit the scope of possible errors (accidents) that can be created.

Work in the Constructor

Your constructor does real work rather than just configuring the object.

Doing real work in the constructor makes it harder for you to test and extend from this class. Miško Hevery says it better than i could here.

Suggested Interface

I would try to keep the interface as simple as possible. What I suggest below is mainly to take the work away from the constructor. You can modify it to your own needs.

class uri { # URI scheme protected $scheme; # URI host protected $host; # URI user protected $user; # URI password protected $password; # URI port protected $port; # URI path protected $path; # URI query protected $query; # URI fragment protected $fragment; # Default constructor for the URI public function __construct() { # No setup required. } # Set the URI current URI. public function setCurrent() { # I would also use isset rather than array_key_exists in this case. # $_SERVER['HTTP'] does not exist, use an if/else on the HTTPS key. # The property values can be set directly. # This is basically the if($uri == null) {} code. $this->scheme = isset($_SERVER['HTTPS']) ? 'https://' : 'http://'; if (isset($_SERVER['SERVER_NAME'])) { $this->host = $_SERVER['SERVER_NAME']; } if (isset($_SERVER['REQUEST_URI'])) { # Process the URI into the object properties. } } # Set the URI from a string according to RFC 2396 (appendix B) public function setFromString($uri) { if (!is_string($uri)) { throw new \InvalidArgumentException( __METHOD__ . ' uri must be a string.'); } # if($uri != null and !is_array($uri)) {} code goes here. } # An URI is provided in array form public function setFromArray(Array $uri) { # if($uri != null and is_array($uri)) {} code goes here. } # Returns the URI as a string. public function __toString() { # Ensure that all required properties are set. if (!isset($this->host, $this->scheme)) # etc. { throw new \BadMethodCallException( __METHOD__ . ' url properties have not been set.'); } $uri = $this->scheme . $this->host; if (isset($this->query)) { # Add optional uri part here. } } } 

Answer 3

Reviewing your code only leads to one clear statement: You've done too much. Code for that already exists. As you use PHP build in functions, you should use a class that does URL parsing itself since longer and is tested. I suggest you to take Net_URL2 from the pear repository.

Even if you don't want to use it for obscure reasons (you want to eat your own dogfood probably), you can review the code that exists.

parse_url - which you use - is not following the standards btw. .

Answer 4

I think you need to look first at the problem you're trying to solve rather than the code you've written.

Is a class necessary

A url is just a string (or an array if it's split into component parts) - is a class really necessary to represent a url? Or are a couple of functions required in the (e.g.) router class to parse and build urls? Reviewing existing code that does a similar things (php frameworks, pear, github, the blogsphere) would help evaluate whether the uri class is appropriate - bear in mind a web page typically has 10s or 100s of urls on it, and it's not free to instanciate many objects.

This is likely to be faster to execute, and effectively easier to use:

$url = Router::url($args); 

Than

echo new Uri($args); 

If you find you're only using each instance of a class once - that's a relatively good indicator that a class is the wrong approach.

Is so much code necessary

The question states that the objective is:

a simple class to deal with parsing and handling URIs

Yet the code in the question isn't simple. It's 180 lines of code that mostly duplicates built-in php functions. To wrap the functionality in a class, simple would be (example and almost pseudo-code):

class url { protected $_url; function __construct($url) { $this->_url = parse_url($url); } function build() { return implode($this->_url); } function __get($key) { return $this->_url[$key]; } function __set($key, $value) { $this->_url[$key] = (string)$value; } } 

Is the API intuitive

It would appear you'd use this class like so:

$url = new uri($args); $absolute = (string)$url; // implied $absolute = $url->build(); $relative = $url->build(array('path', 'query', 'fragment')); 

But the way the class works also permits:

$emptyString = $url->build(array('made', 'up', 'or', 'typos')); $nonsense = $url->build(array('password', 'path', 'fragment')); 

Neither of these permutations should be possible and indicate that you need to know the internals of the class to use it. If you don't know how the class is storing the parts of a url, you can't use the class and/or will get no errors with mistakes, just nothing useful.

This would be easier to use, and a lot less error prone:

$absolute = $url->absolute(); $relative = $url->relative(); 

Since all the properties are public - it's not necessary to have a method which just returns a public property.

Assuming the code in the question will be used in one form or another, here are some tips for refactoring the code to be easier to read, use and maintain:

Don't repeat yourself (DRY) #1

if($uri == null) { ... if($uri != null and !is_array($uri)) { ... if($uri != null and is_array($uri)) { 

That means in all cases the variable is tested 3 times if it's null, and twice if it's an array.

A couple of side points: The first use of is_array is really testing if the variable is a string by inferrance - it would therefore be more appropriate to use is_string. Also by using type-insensitive checks an empty array or empty string are caught by the first if block (perhaps that's deliberate but it isn't obvious).

Instead the code can be written as:

if(!$uri) { //get currenturl ... } if(!is_array($uri)) { ... } else { 

Or, less nesting by using the principle of "return early":

if(!$uri) { //get currenturl ... } if(!is_array($uri)) { ... return; } // it's not empty and it's an array. 

Also note that using negative checks can be harder to read - so it would infact be preferable to write as:

if(!$uri) { //get currenturl ... } if(is_string($uri)) { ... return; } // it's not empty and it's not a string. 

OR

if(!$uri) { ... return; } if(is_array($uri)) { ... return; } // it's not empty and its NOT an array. 

Don't repeat yourself (DRY) #2

if(array_key_exists('HTTPS', $_SERVER)) array_push($uri_parts, 'https://'); ... if(!array_key_exists('HTTPS', $_SERVER)) array_push($uri_parts, 'http://'); 

In this block, the array key HTTPS is tested twice. But, unless it's a cli request it's not possible for SERVER_NAME or REQUEST_URI to be missing. This code is more approproate to acheive identical functionality:

if (empty($_SERVER['HTTPS'])) { $uri = 'http://'; } else { $uri = 'https://'; } $uri = $uri . $_SERVER['SERVER_NAME'] . $_SERVER['REQUEST_URI']; 

That's one logical test instead of 4

Don't rewrite php functions

Built-in php functions are much faster to execute than equivalent user-land php code, and are more robust.

preg_match('"^(([^:/?#]+):)?(//([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?"', $uri, $matches); ... preg_match('"(([^:@]*)(:([^:@]*))?@)?([^:]*)(:(.*))?"', $authority, $matches); 

This is exactly what parse_url is for, this whole block appears to be identical to:

$parsed = parse_url($uri); foreach($parsed as $key => $value) { $this->$key = $value; } $this->path = explode('/', trim($this->path, '/')); parse_str($this->query, $this->query); 

Be consistent

This is the only reference to anchor:

if(array_key_exists('anchor', $uri)) $this->fragment = $uri['anchor']; 

This is the only reference to pass:

if(array_key_exists('pass', $uri)) $this->scheme = $uri['pass']; 

But it should be "pass" and not "password" (internally) so that the code used is consistent with the output of the parse_url function - on which the class should rely.

Don't repeat yourself (DRY) #3

The build method could be written as:

public function build($parts) { $this->_prepare($parts); foreach($parts as $field) { $return .= $this->_prepared[$field]; } return $return; } /** * Ensure all bits of the url are in a position to just be concatenated */ protected function _prepare($parts) { $this->_prepared = array(); foreach($parts as $field) { $this->_prepared[$field] = $this->$field; } if (!empty($this->_prepared['scheme'])) { $this->_prepared['scheme'] .= '://'; } if (!empty($this->_prepared['user'])) { $this->_prepared['password'] .= '@'; } if (!empty($this->_prepared['path'])) { if (is_array($this->_prepared['path'])) { $this->_prepared['path'] = implode('/', $this->_prepared['path']); } if ($this->_prepared['path'][0] !== '/') { $this->_prepared['path'] = '/' . $this->_prepared['path']; } } if (!empty($this->_prepared['query'])) { $this->_prepared['query'] = '?' . http_build_query($this->_prepared['query']); } if (!empty($this->_prepared['fragment'])) { $this->_prepared['fragment'] = '#' . $this->_prepared['fragment']; } } 

In this way the embedded logic in the build method is separated and more obvious - and the repetative in_array!empty logic is avoided. It also becomes possible to run the _prepare function whenever the data changes rather than each time a call is made to return output.

Concusion

Some of the other answers have focussed on the use of public properties, on that: what does it matter. There are more fundamental concerns to address with the code presented in the question.

Whenever writing code, try to apply the DRY principle not only to the code you write but also to the code as executed. In this way you get code that's easier and faster to read and run.

The way the class works is only 'easy' to use if you want an absolute url. The build method should be refactored to permit only real possibilities. If you're already using the class with more permutations than just absolute and relative urls - you'll need to decide whether to change the calls or the code so that it still works.

Assuming your code already works you're in a perfect position to write a few unit tests for your class (if you don't already have some) - and then start tweaking it. Writing unit tests will give you the confidence to rip the guts out of any code and know that if/when the tests pass - you've safe to commit your changes and benefit from your shiny(er) code and/or new-found knowledge.