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!

Gelmir
  • 1,829
  • 1
  • 18
  • 28
  • Project in question is at: http://www.githib.com/audith/Persephone – Gelmir Dec 17 '12 at 00:17
  • You seem to be setting "SERVER_NAME" as the first parameter, corresponding to `$tims->name` rather then $arguments[0]. – Neograph734 Dec 17 '12 at 00:18
  • @bobthyasian - answered you below – Gelmir Dec 17 '12 at 00:30
  • @Neograph734: I'm not sure what you mean. There is no `$this->name`, it is `$this->$name` – Gelmir Dec 17 '12 at 00:30
  • @Shehi GitHub link returns 404. Check your link again please. – bobthyasian Dec 17 '12 at 00:33
  • 1
    Typo again, its github, not githib: http://www.github.com/audith/Persephone – Gelmir Dec 17 '12 at 00:33
  • @Shehi Slow down, take a breath lol – bobthyasian Dec 17 '12 at 00:34
  • Man, I have so many errors here, its just stupid. I will ask them later on, even ini_set() is messed up :D – Gelmir Dec 17 '12 at 00:35
  • Not to nit-pick, but the README demo link is dead. :/ – bobthyasian Dec 17 '12 at 00:37
  • @bobthyasian - yea, I am working on that. Just moved servers, putting files back in. Sorry mate. Will be up in 5 mins. – Gelmir Dec 17 '12 at 00:37
  • When I launch the script in my server (from the github source), everything seems to work fine ! – Gilles Hemmerlé Dec 17 '12 at 00:38
  • Moving servers may be the issue here, not the code. Check server setup. – bobthyasian Dec 17 '12 at 00:40
  • And if you show a `var_dump($_SERVER);` what is the result ? – Gilles Hemmerlé Dec 17 '12 at 00:42
  • It all works fine, even data gets assigned properly. Problem is: Why the hell I have that WARNING there. I hate errors in code, especially those which don't make sense. README url is up now - http://persephone.audith.org/acp/components/modules – Gelmir Dec 17 '12 at 00:48
  • @GillesHemmerlé - there is /log/php-errors.log file in the package. Check that for PHP errors. Config file is under /httpdocs, if u wanna install locally for testing. Also import sql.sql file into local MySQL-server and set Db config info as well. Lastly, fix hostname located in config file - app is hostname sensitive. App also uses Memcache - if you dont have it installed locally, change Cache setting to "diskcache" in Config file. – Gelmir Dec 17 '12 at 00:54
  • bookmarked to share with colleagues – Spechal Feb 26 '13 at 04:22

2 Answers2

27

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

Community
  • 1
  • 1
Baba
  • 94,024
  • 28
  • 166
  • 217
  • 1
    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. – Gelmir Mar 26 '13 at 07:49
  • @Shehi how is the project going ? – Baba Apr 25 '13 at 09:59
  • @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 :) – Gelmir Apr 25 '13 at 22:06
  • @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. – Gelmir Apr 26 '13 at 06:22
  • 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. – Gelmir Apr 26 '13 at 12:26
  • [Pushed first version](https://github.com/olekukonko/SuperVariable) More filters coming with time – Baba Apr 26 '13 at 15:42
  • Can you email me at shehi@imanov.name please? I think we shouldn't communicate via comments here :) – Gelmir Apr 29 '13 at 08:31
  • Supperb..... I am trouble to much with this error.. but you are genius.. – Kaushik Makwana Sep 08 '16 at 06:23
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.

Community
  • 1
  • 1
Gelmir
  • 1,829
  • 1
  • 18
  • 28