HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PHP function to access a database and return json

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
returnphpfunctionjsondatabaseandaccess

Problem

 "p_day", 
        "month"      => "p_month"
        ... etc. .....
    );

    $table = $tables[$user_table];

    if(!$table) {
        die(json_encode(array("error" => "bad table name")));
    }

    $con = getConnection(); // getConnection is in '../dbInfo.php'

    $query = "select * from " . $table;
    $res = mysql_query($query, $con);

    if(!$res) {
        die(json_encode(array("error" => "no results from table")));
    }

    $fields_num = mysql_num_fields($res);
    $fields = array();
    for($i=0; $i name;
    }

    $i = 0;
    while($row = mysql_fetch_array($res)) {
        $rows[$i] = $row;
        $i++;
    }
    $json = array("rows" => $rows, "headers" => $fields);

    $jsontext = json_encode($json);
    return $jsontext;
}

?>


What this code is doing:

  • access the database, selecting rows from a table, and returning them as a serialized json object



  • a table name is looked up in $tables -- the keys are acceptable user input, the values are actual table/view names in the database



  • data is selected from the table



  • the data is put into a big hash



  • the hash is serialized as a json string and returned



Specific issues I'm concerned about:

  • security -- is my DB connection info safe? This file is in the root directory of public content, so dbiInfo.php, with the database connection information, is not publicly accessible (I think)



  • security -- am I open to SQL injection attacks? I build a SQL query with string concatenation



  • security -- $user_table is untrusted input; is it safe? It's only used as a key to look up trusted input ...



  • error handling -- have I dealt with all error conditions



  • there are lots of versions of PHP functions -- am I using the right ones?



General issues:

  • following conventions



  • quality/readability/comments



Edit: the data is publicly available -- I'm worried about somebody getting more than read access to one of the listed tables, or any access to any other table in the DB.

Solution

This:

$tables = array(
    "day"        => "p_day", 
    "month"      => "p_month"
    ... etc. .....
);

$table = $tables[$user_table];

if(!$table) {
    die(json_encode(array("error" => "bad table name")));
}


will throw a notice if $user_table is not a valid array key, something you should have already tested. Rewrite as:

$tables = array(
    "day"        => "p_day", 
    "month"      => "p_month"
    ... etc. .....
);

if(!array_key_exists($user_table, $tables)) {
    die(json_encode(array("error" => "bad table name")));
}

$table = $tables[$user_table];


I really hate it when functions die(), and in your case there's no point in that. You could simply do:

if(!$table) {
    return json_encode(array("error" => "bad table name"));
}


since the function is expected to return a json formatted string. If you really want to die() you should do that where you call the function and the returned string is an error. Or you could just return false when an error occurs and the raw array when everything works, and when calling the function do something like:

$result = json_encode(getReport("some_table"));


That way getReport() will be useful even when you don't need json encoded output. But that's up to you and how you actually use it.

As @LokiAstari mentions mysql_ functions are deprecated and should be avoided. I would skip mysqli_ functions too and use PDO which will give you the extra bonus of switching to another database engine if you ever need to and it has a nice OO interface.

For everything else, I'm with @LokiAstari.

Code Snippets

$tables = array(
    "day"        => "p_day", 
    "month"      => "p_month"
    ... etc. .....
);

$table = $tables[$user_table];

if(!$table) {
    die(json_encode(array("error" => "bad table name")));
}
$tables = array(
    "day"        => "p_day", 
    "month"      => "p_month"
    ... etc. .....
);

if(!array_key_exists($user_table, $tables)) {
    die(json_encode(array("error" => "bad table name")));
}

$table = $tables[$user_table];
if(!$table) {
    return json_encode(array("error" => "bad table name"));
}
$result = json_encode(getReport("some_table"));

Context

StackExchange Code Review Q#6237, answer score: 6

Revisions (0)

No revisions yet.