3

My open-source project was working just fine, until I started to work on it after 6 month of break. Updated to latest XAMPP, and start getting tons of weird errors, one of which is as:

I have Input class, with a caller method as:

<?php
class Input
{
    public function __call ( $name , $arguments )
    {
        if ( !in_array( $name, array( "post", "get", "cookie", "request", "server", "env" ) ) )
        {
            throw new Exception( "Input::" . $name . "() not declared!" );
        }

        $_name_of_superglobal = "_" . strtoupper( $name );
        $_max_iteration_level_for_cleanup = in_array( $name, array( "server", "env" ) ) ? 1 : 10;

        # $arguments[0] is the index of the value, to be fetched from within the array.
        if ( !empty( $arguments[0] ) and array_key_exists( $arguments[0], $this->$name ) )
        {
            return $this->$name[ $arguments[0] ];
        }
        elseif ( !empty( $arguments[0] ) and array_key_exists( $arguments[0], $GLOBALS[ $_name_of_superglobal ] ) )
        {
            return $this->$name[ $this->clean__makesafe_key( $arguments[0] ) ] = $this->clean__makesafe_value( $GLOBALS[ $_name_of_superglobal ][ $arguments[0] ], array(), true );
        }
        elseif ( !empty( $arguments[0] ) and !array_key_exists( $arguments[0], $GLOBALS[ $_name_of_superglobal ] ) )
        {
            return null;
        }
        else
        {
            if ( $this->_is_cleanup_done_for[ $name ] === true )
            {
                return $this->$name;
            }
            $this->_is_cleanup_done_for[ $name ] = true;
            return $this->$name = $this->clean__makesafe_recursively( $GLOBALS[ $_name_of_superglobal ], $_max_iteration_level_for_cleanup );
        }
    }
?>

This piece of code, works like this: you ask certain superglobal value from it, and it returns clean version of it, on-demand:

<?php
$input = new Input();
$server_name = $input->server("SERVER_NAME");
?>

Easy right? Well, after I updated PHP with XAMPP, it just doesn't work [edit: it works, with the Warning message] - error is:

PHP Warning:  Illegal string offset 'SERVER_NAME' in S:\...\kernel\input.php on line 159

line, which corresponds to line of code:

return $this->$name[ $this->clean__makesafe_key( $arguments[0] ) ] = $this->clean__makesafe_value( $GLOBALS[ $_name_of_superglobal ][ $arguments[0] ], array(), true );

which is stupid: $_name_of_superglobal = "_SERVER" there, and $arguments[0] = "SERVER_NAME" and overall assignment is string which gets cleaned.

WHAT MIGHT BE THE PROBLEM THERE? I am totally lost here!

16
  • Project in question is at: githib.com/audith/Persephone Commented Dec 17, 2012 at 0:17
  • You seem to be setting "SERVER_NAME" as the first parameter, corresponding to $tims->name rather then $arguments[0]. Commented Dec 17, 2012 at 0:18
  • @bobthyasian - answered you below Commented Dec 17, 2012 at 0:30
  • @Neograph734: I'm not sure what you mean. There is no $this->name, it is $this->$name Commented Dec 17, 2012 at 0:30
  • 1
    Typo again, its github, not githib: github.com/audith/Persephone Commented Dec 17, 2012 at 0:33

2 Answers 2

27
+50

Introduction

I know this has been answered but Illegal string offset ERROR is not the only issue i see here. I belive they are better ways to introduce the flexibility you want and also still make elements save without all that complexity and using of $GLOBALS.

You can start by looking at :

Quick Review

$input = new Input();                          <-- You add to initiate a class 
$server_name = $input->server("SERVER_NAME");
      ^                  ^           ^
      |                  |           |
    variable             |           |
                     Variable        |
                                  Variable 

Am not sure what stops you from just using

    $_SERVER['SERVER_NAME'] = makeSave($_SERVER['SERVER_NAME']);
                                  ^
                                  |- I guess this is what you want to introduce 

Assumption - You want fexibility

Lest assume you want flexibility and also recursion then your class call can be as flexible as :

print_r($input->SERVER_NAME);            |
print_r($input['SERVER_NAME']);          |----- Would Produce same result 
print_r($input->SERVER_NAME());          |

If this is the kind of flexibility you want the i would consider you combine __get , __call and ArrayAccess altogether ...

Lets Imagine

$var = array();
$var["name"] = "<b>" . $_SERVER['SERVER_NAME'] . "</b>";
$var["example"]['xss'] = '<IMG SRC=javascript:alert("XSS")>';
$var["example"]['sql'] = "x' AND email IS NULL; --";
$var["example"]['filter'] = "Let's meet  4:30am Ât the \tcafé\n";

$_SERVER['SERVER_NAME'] = $var ; // Just for example 

Now back to your format

$makeSave = new MakeSafe(MakeSafe::SAVE_XSS | MakeSafe::SAVE_FILTER);
$input = new Input($_SERVER, $makeSafe);

//You can 
print_r($input->SERVER_NAME);

//Or
print_r($input['SERVER_NAME']);

//Or
print_r($input->SERVER_NAME());

They would all output

Array
(
    [0] => &lt;b&gt;localhost&lt;/b&gt;
    [1] => Array
        (
            [0] => &lt;IMG SRC=javascript:alert(&quot;XSS&quot;)&gt;
            [1] => x&#039; AND email IS NULL; --
            [2] => Let&#039;s meet  4:30am &#195;&#130;t the &#9;caf&#195;&#169;&#10;
        )

)

See Live DEMO

Your INPUT class modified

class INPUT implements \ArrayAccess {
    private $request = array();
    private $makeSafe;

    public function __construct(array $array, MakeSafe $makeSafe) {
        $this->request = $array;
        $this->makeSave = $makeSafe;
    }

    function __get($offset) {
        return $this->offsetGet($offset);
    }

    function __call($offset, $value) {
        return $this->offsetGet($offset);
    }

    public function setRequest(array $array) {
        $this->request = $array;
    }

    public function offsetSet($offset, $value) {
        trigger_error("Error: SUPER GLOBAL data cannot be modified");
    }

    public function offsetExists($offset) {
        return isset($this->request[$offset]);
    }

    public function offsetUnset($offset) {
        unset($this->request[$offset]);
    }

    public function offsetGet($offset) {
        return isset($this->request[$offset]) ? $this->makeSave->parse($this->request[$offset]) : null;
    }
}

Make your Save method a class

class MakeSafe {
    const SAVE_XSS = 1;
    const SAVE_SQL = 2;
    const SAVE_FILTER_HIGH = 4;
    const SAVE_FILTER_LOW = 8;
    const SAVE_FILTER = 16;

    private $options;

    function __construct($options) {
        $this->options = $options;
    }

    function escape($value) {
        if ($value = @mysql_real_escape_string($value))
            return $value;
        $return = '';
        for($i = 0; $i < strlen($value); ++ $i) {
            $char = $value[$i];
            $ord = ord($char);
            if ($char !== "'" && $char !== "\"" && $char !== '\\' && $ord >= 32 && $ord <= 126)
                $return .= $char;
            else
                $return .= '\\x' . dechex($ord);
        }
        return $return;
    }

    function parse($mixed) {
        if (is_string($mixed)) {
            $this->options & self::SAVE_XSS and $mixed = htmlspecialchars($mixed, ENT_QUOTES, 'UTF-8');
            $this->options & self::SAVE_SQL and $mixed = $this->escape($mixed);
            $this->options & self::SAVE_FILTER_HIGH and $mixed = filter_var($mixed, FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_HIGH);
            $this->options & self::SAVE_FILTER_LOW and $mixed = filter_var($mixed, FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_LOW);
            $this->options & self::SAVE_FILTER and $mixed = filter_var($mixed, FILTER_SANITIZE_STRING, FILTER_FLAG_ENCODE_HIGH | FILTER_FLAG_ENCODE_LOW);
            return $mixed;
        }

        if (is_array($mixed)) {
            $all = array();
            foreach ( $mixed as $data ) {
                $all[] = $this->parse($data);
            }
            return $all;
        }
        return $mixed;

        return $this->final;
    }
}

Conclusion

Has i said i know this has been answered but i hope this helps someone else not to write code like yours ...

PS : This has also fixed your PHP Warning: Illegal string offset ERROR

Sign up to request clarification or add additional context in comments.

8 Comments

Indeed. I think I will implement the idea to my code - never used Dependency Injection before :) If you PM me your real name, I'd like to put your name in code as an author.
@Shehi how is the project going ?
@Bob: Good, I am upgrading it to PHP 5.3 and making some optimizations to it. I expanded your DI idea to three separate classes actually, separating Validation, Sanitation and UNicode for the moment. Still work in progress tho :)
@Bob: Sure, but you still have to gimme some name to put in code as co-author to some classes :) If not interested, ignore this request.
Sure, you welcome to fork it on Github. If it proves to be fruitful, in future I can have you in our JIRA environment, where I manage and run the project. By the way, I am on upgrade-XXX branch on Github - once whole system is upgraded and tested, I will merge it into master branch.
|
5

I found my answer here. Apparently, the line

return $this->$name[ $this->clean__makesafe_key( $arguments[0] ) ] = $this->clean__makesafe_value( $GLOBALS[ $_name_of_superglobal ][ $arguments[0] ], array(), true );

should be like:

return $this->{$name}[ $this->clean__makesafe_key( $arguments[0] ) ] = $this->clean__makesafe_value( $GLOBALS[ $_name_of_superglobal ][ $arguments[0] ], array(), true );

due to the precedence.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.