[TDD] Who wants to help me build an http request class?

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

alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

Well, here are my suggestions...

1) CLI (Command line interface) input isn't HTTP data, it's environment data...so the classname with HTTP in it wouldn't make sense...besides I think that's going over board as PHP as a web application engine, wouldn't typeically be called that way...

Plus it would be tricky to build into an overall class hierarchy

2) Something of a factory method which cranked out custom *cleaned* objects with direct access to property names (albeit - going against OOP tradition) would make a lot of sense here - IMHO anyways.

So you call a method and it returns objects with no methods, just public properties...to emulate an OO approach to HTTP input.

Code: Select all

static HTTP_Request::getObject($_POST);

$ret = HTTP_Request::getObject($_POST];
$ret->first_name; // $_POST['first_name'];
The factory should also be implemented as a singleton, as there is no need for other instances, that I can this far anyways...

Perhaps including a HTTP_Filter object as a plugin would be interesting as well. Admittedly this needs more thought, I have a few ideas, but would like the rest of the community to through in some ideas on the subject - Out of context or plugin approach???

The factory method need only be a factory, unless PHP supports expando properties like javascript?

My stipulations for continuing support... :P

1) phpDocumentor comments
2) Zend framework coding conventions & license are followed

Other than that, I'm not sure I would ever use a class like this, as I don't find using $GLOBALS dangerous, but if it did more than act as a registry, I could be pursuaded into using it and contributing my thoughts, sure... :)

Cheers :)
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Hockey wrote:The factory should also be implemented as a singleton, as there is no need for other instances, that I can this far anyways...
I'd favor static over singleton. Singletons are overused and painful to test correctly.
Hockey wrote:Perhaps including a HTTP_Filter object as a plugin would be interesting as well. Admittedly this needs more thought, I have a few ideas, but would like the rest of the community to through in some ideas on the subject - Out of context or plugin approach???
Plugin is my preference.
Hockey wrote:The factory method need only be a factory, unless PHP supports expando properties like javascript?
Yes, it does support dynamic properties. They are automatically public and are not included in get_class_vars() but are included in get_object_vars()
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

feyd wrote:
Hockey wrote:The factory should also be implemented as a singleton, as there is no need for other instances, that I can this far anyways...
I'd favor static over singleton. Singletons are overused and painful to test correctly.
Hockey wrote:Perhaps including a HTTP_Filter object as a plugin would be interesting as well. Admittedly this needs more thought, I have a few ideas, but would like the rest of the community to through in some ideas on the subject - Out of context or plugin approach???
Plugin is my preference.
Hockey wrote:The factory method need only be a factory, unless PHP supports expando properties like javascript?
Yes, it does support dynamic properties. They are automatically public and are not included in get_class_vars() but are included in get_object_vars()
a) A static function or just using a static variable? I'm not sure it would be tremendously difficult to implement psuedo-singleton behaviour, atleast at first though it isn't...

But at the same time, it doesn't make sense to implement it as a singleton, but restricting returned instances to one does make sense...

I mean, depending on how HTTP data was entered...if it was by passing $_GET, $_REQUEST, etc in the ctor() then you'd only really need to pass it in once...so you'd in theory only ever get one object out of that...

Unless the class parsed POST name's and added additional funcitonality...like assigning a "type" for the form data which could trigger filter plugins...

Code: Select all

first_name[string:25]
Not sure if that would validate as a XHTML attribute value but the idea just dawned on me and it has my interest piqued/peaked... :P

Actually, now that I think of it, if the above convention did pass as valida XHTML in a INPUT type name attribute...you could use that data in a javascript framework as well, like the one we were disscussing earlier...using that syntax we could ehance the client side experience as well...

Hmmmmm...growing slightly out of context for a trivial HTTP_Request class, but...the idea is interesting to me...

b) Well...yes, plugin is the way to go...but what I meant was, is it a good idea? Or is it out of context for such a class...

For instance, in my ORM class, it's the ORM objects responsibility to clean data...as that INPUT data is really only harmful when passed to a SQL database or system() maybe...

Inserting into a file or testing data or other actions, sanitized or security checked data isn't really an issue is it?

c) Cool, never tried it...so how does expando work in PHP? Do you just assign a variable to an object?

Code: Select all

$obj = new MyObject();
$obj->new_property = "Somename";
The member variable new_property would exist for that instance only?

Cool...thats awesome, I thought for sure we'd have to use eval() or something :)

Cheers :)
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Hockey wrote:a) A static function or just using a static variable?
Static method. Static variables are almost as bad as Singletons.
Hockey wrote:b) Well...yes, plugin is the way to go...but what I meant was, is it a good idea? Or is it out of context for such a class...
The class should only handle stripslashes() or whatever else is required to make a common string to begin work from once passed to filters, transforms, or the page controller. Validation and such should go through the filters, which in turn would likely be controlled by the page controller. Owned by the page controller may be a different story. I guess that depends on where the trusted data starts.
Hockey wrote:Inserting into a file or testing data or other actions, sanitized or security checked data isn't really an issue is it?
If the data is expected to be in a certain format, yes it should be run through a masher. That functionality would probably be attached to the storage container though.
Hockey wrote:c) Cool, never tried it...so how does expando work in PHP? Do you just assign a variable to an object?
Yup.
Hockey wrote:The member variable new_property would exist for that instance only?
Yup.
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

Hmmmm...from what I can tell from glancing over the W3C grammar for XML...attribute values will not accept [] as valid characters, but colon works :P

Code: Select all

name="first_name:string.24"
I think that would validate though :?

Kind of cryptic...and any decimal values would first need be converted to integers...but still it's neat :P
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

let's stay on topic Hockey, that stuff will come later :)
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Come on Ninja, it's your 'turn' :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:Come on Ninja, it's your 'turn' :P
LOL I've been trying to get to this since I got home... keep getting distracted. I'm working on it! :D
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Keep in mind that I have never done any unit testing. EVER. So here goes... Image

Code: Select all

class HttpRequest
{
    protected $reqVar;
   
    public function __construct($method)
    {
        if ($method == 'post') $this->reqVar = $_POST;
    }
   
    public function getRequestGlobal()
    {
        return $this->reqVar;
    }

    public function get ($name)
    {
        return $this->reqVar[$name];
    }

    public function has ($name)
    {
        return isset($this->reqVar[$name]);
    }
}
class testOfHttpRequest extends UnitTestCase
{
    public function testCreationOfNewRequestMethod()
    {
        $_POST['dumy'] = 1;
        $request = new HttpRequest('post');
        $this->assertIdentical($_POST, $request->getRequestGlobal());
    }

    public function testRetrievalOfSingleRequestVar()
    {
        $_POST['dumy'] = 1;
        $request = new HttpRequest('post');
        $this->assertIdentical($_POST['dumy'], $request->get('dumy'));
    }

    public function testCheckVarExists()
    {
	$_POST['dumy'] = 1;
	$request = new HttpRequest('post');
	$this->assertTrue($request->has('dumy'));
    }
}
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

No that looks right Ninja.... I''ll move along once I get to work... to throw a spanner in the works I want to use GET...
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Really sorry to interupt, but when I saw this:

Code: Select all

<?php
protected function removeSlashes(&$var) {
        if (is_array($var)) {
            foreach ($var as $name => $value) {
                if (is_array($value)) {
                    $this->removeSlashes($value);
                } else {
                    $var[$name] = stripslashes($value);
                }
           }
        } else {
            $var = stripslashes($var);
        }
    } 
?>
I had to think about something that Ilia mentions in his book (guide to php sec.) about using a recursive function to stripslashes on an input array. He says that a recursive function uses the system stack which is limited. So with enough iterations, it's possible to crash php that way, if a user input contains a very deep, multidimensional array. Therefore he first flattens the input array into a single array.

Code: Select all

<?php
if (get_magic_quotes_gpc()) {
  $input = array(&$_GET, &$_POST, &$_COOKIE, &$_ENV, &$_SERVER);
    while (list($k,$v) = each($input)) {
      foreach ($v as $key => $val) {
        if (!is_array($val)) {
          $input[$k][$key] = stripslashes($val);
          continue;
        }
        $input[] =& $input[$k][$key];
      }
    }
  unset($input);
}
?>
Maybe this is not relevant in this specific case or in the way this class is going to be used, but I couldn't resist asking. So what do you think?
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

d11wtq wrote:No that looks right Ninja.... I''ll move along once I get to work... to throw a spanner in the works I want to use GET...
That I've had in my head since we began from scratch :)

Should we use multiple instances for each global (cookie, get and post,) or should we use one object for _all_ data, with an arg to specify where it comes from, or should we lumber $_REQUEST in and not care?
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

matthijs:

I just ran this:

Code: Select all

<?php

function removeSlashes(&$var)
{
    if (is_array($var)) {
        foreach ($var as $name => $value) {
            if (is_array($value)) {
                removeSlashes($value);
            } else {
                $var[$name] = stripslashes($value);
            }
        }
    } else {
        $var = stripslashes($var);
    }
}

$str = '';
for ($i = 0; $i < 1000; $i++) {
  $str .= '[0]';
}
eval ('$array' . $str . ' = "foo";');

removeSlashes($array);

print_r($array);

?>
and it quite surprisingly ran rather quick :) Generated 4000 lines of output, but ran quick :)
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:
d11wtq wrote:No that looks right Ninja.... I''ll move along once I get to work... to throw a spanner in the works I want to use GET...
That I've had in my head since we began from scratch :)

Should we use multiple instances for each global (cookie, get and post,) or should we use one object for _all_ data, with an arg to specify where it comes from, or should we lumber $_REQUEST in and not care?
Why don't we let the tests decide that? :) Although I can already see the brick wall at present if we didn't use multiple instances.

OK, the test:

Code: Select all

class testOfHttpRequest extends UnitTestCase
{
    public function testCreationOfNewRequestMethod()
    {
        $_POST['dumy'] = 1;
        $request = new HttpRequest('post');
        $this->assertIdentical($_POST, $request->getRequestGlobal());
        $_POST = array(); //Clean up
    }

    public function testRetrievalOfSingleRequestVar()
    {
        $_POST['dumy'] = 1;
        $request = new HttpRequest('post');
        $this->assertIdentical($_POST['dumy'], $request->get('dumy'));
        $_POST = array(); //Clean up
    }

    public function testCheckVarExists()
    {
        $request = new HttpRequest('post');
        $this->assertFalse($request->has('dumy'));
        unset($request);
        $_POST['dumy'] = 1;
        $request = new HttpRequest('post');
        $this->assertTrue($request->has('dumy'));
        $_POST = array(); //Clean up
    }

    public function testOfGETRequestMethod()
    {
        $_GET['dummy'] = 1;
        $request = new HttpRequest('get');
        $this->assertIdentical($_GET, $request->getRequestGlobal());
        $this->assertTrue($request->has('dummy'));
        $this->assertIdentical($_GET['dummy'], $request->get('dummy'));
        $_GET = array();
    }
}
(Sorry, I enhanced some things on previous tests so as not to jade results from left over superglobal contents)

And the new code:

Code: Select all

class HttpRequest
{
    protected $reqVar;

    public function __construct($method)
    {
        switch (strtolower($method))
        {
            case 'get': $this->reqVar = $_GET;
            break;
            case 'post': $this->reqVar = $_POST;
            break;
        }
    }

    public function getRequestGlobal()
    {
        return $this->reqVar;
    }

    public function get ($name)
    {
        return $this->reqVar[$name];
    }

    public function has ($name)
    {
        return isset($this->reqVar[$name]);
    }
}
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Jenk, that looks ok. Would suffice for most of us. But I think what Ilia was trying to point to is a situation in which a malicious person tries to consciously crash php. His example:

Code: Select all

<?php
$str = str_repeat(“[]”, 100000);
file_get_contents(http://site.com/script.php?foo={$str});
?>
Post Reply