https://drive.google.com/open?id=14aiWzFLAxjqZ8oW7ypXTbbq3PUXUSSse
I apologize for the image showing just now. Tried to upload it yesterday, but the internet connection was poor.
sample image not loading
fix code formatting
don’t use string concatenation to build queries. use prepared statements or a user friendly query builder library instead
if (isset($_POST['newrole'])) {
$newdata = $_POST['newrole'];
$column = 'role';
}
if (isset($_POST['newfirstname'])) {
$newdata = $_POST['newfirstname'];
$column = 'firstname';
}
if (isset($_POST['newlastname'])) {
$newdata = $_POST['newlastname'];
$column = 'lastname';
}
if (isset($_POST['newemail'])) {
$newdata = $_POST['newemail'];
$column = 'email';
}
if (isset($_POST['newusername'])) {
$newdata = $_POST['newusername'];
$column = 'username';
}
if (isset($_POST['newbio'])) {
$newdata = $_POST['newbio'];
$column = 'bio';
}
if (isset($_POST['newarea'])) {
$newdata = $_POST['newarea'];
$column = 'area';
}
if (isset($_POST['newtags'])) {
$newdata = $_POST['newtags'];
$column = 'tags';
}
if (isset($_POST['newstatus'])) {
$newdata = $_POST['newstatus'];
$column = 'status';
}
if (isset($_POST['newjoined'])) {
$newdata = $_POST['newjoined'];
$column = 'joined';
}
There’s multiple shorter and clearer ways to write this part. I would even prefer variable-variables over this.
Simplest would be to just name your form fields to exactly match the database fields and then do a foreach over the $_POST array using the key as the db column name. But that gets complicated when working with prepared statements so I’d probably just use a library where I can give it the POST array and be done with it without having to write a lot of code to talk to the database.
I’ll second that this could be handled a varaity of ways, none of which is clear in the code you are using.
It may not be what you want to hear, I understand that, but have you considered a micro-framework such as Slim? It is specifically for writing API’s.
I’m unclear what you are trying to achieve with the code as well. If you are trying to update an account, you would use PATCH, not a GET or POST. Prepared statements are also a must. Don’t think that just because you are dealing with users that they are above messing with the system.
Thanks for your response thinsoldier. I will endeavour to refactor the code the way you suggested.
astonecipher, thank you for your suggestion. Believe me, I would very much love to use a framework for that, but I can’t. I am simply contributing to a project which has had it’s codebase built on vanilla php. The REST API in question is built with pure php, and i don’t see or know how i can bring the slim framework into the project. There’s also a template for the webservices which i have to follow.
I will try doing a PATCH request as you suggested. Thanks again
In that case I don’t think it realy matters if you only use $_GET and $_POST. No web browser actually supports PATCH and PUT and other through web forms anyway. Some (but not all?) support them through ajax requests though. So I suggest sticking with get and post since that’s what the project already uses.
As for frameworks you might not be able to use a routing+middleware+dependency-injection framework since such a thing has to take over the top layer of the app and be used everywhere. But if anyone else on this project has any sense they would allow you to use a query builder library to make your sql queries safe against injection attacks.
mysqli_query($BAS_sqli_link,
"UPDATE ".TABLE_USERS." SET $column = $newdata
WHERE user_id='$CLIENT_USER_ID'")
This is bad. I don’t even see anywhere in your code where you were using even outdated methods of securing the variables used in the query. Nothing. No mysqli_real_escape_string, no casting $CLIENT_USER_ID to an integer. Nothing. This is not good.
Prepared statements are a must. They can sometimes make the code a bit more verbose and hard to follow though. But your code was already very very hard to follow so I don’t think it should bother you.
Personally I prefer to suggest that beginners use a user friendly library like Idiorm or Paris http://j4mie.github.io/idiormandparis/ https://idiorm.readthedocs.io/en/latest/
If everyone else on that project is also concatenating variables into sql strings but otherwise are willing to listen to good advice, they should be willing to let you use some sort of safer query builder function.
Here’s my best guess of what your original unformatted code was doing but edited to have less lines of actual code, lots more comments, and no unsafe sql queries via Idiorm.
<?php
require_once "_api_init.php";
require '../vendor/autoload.php';
// Idiorm configuration
ORM::configure('mysql:host='.getenv('DB_HOST').';dbname='.getenv('DB_NAME') );
ORM::configure('username', getenv('DB_USER'));
ORM::configure('password', getenv('DB_PASS'));
ORM::configure('driver_options', array(PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8mb4'));
ORM::raw_execute('SET time_zone="-5:00"');
//-----------------------------
// $CLIENT_TOKEN and $CLIENT_USER_ID and $API_CALL_COUNT
// all already exist at this point somehow.
// Probably they come from the required file above.
function primaryAuthCheck($token,$userid,$apicalls)
{
if ( $results['auth'] == 1
&& isset($token)
&& isset($userid)
&& isset($apicalls))
{ return true; }
else { return false; }
}
if( false === primaryAuthCheck( $CLIENT_TOKEN,$CLIENT_USER_ID,$API_CALL_COUNT ) )
{ exit('something is wrong - cannot continue #1'); }
//-----------------------------
$get_type_options = ['acc.info', 'acc.caty'];
// Exchange all array keys with their values
// so you can use the easier and faster isset function.
$get_type_options = array_flip( $get_type_options );
if( !isset($_GET['type']) || !isset($get_type_options[$_GET['type']]) )
{
echo json_encode([
'http_code' => 401,
'status' => 0,
'error' => "Data is missing",
'result' => "",
'data' => array()
]);
exit;
}
//-----------------------------
$CHOSEN_TYPE = $get_type_options[$_GET['type'];
// $_POST is expected to contain only 1 field whose key corresponds
// exactly to the name of a database column and whose value
// will be placed in that column.
// The possible fields are:
// role, firstname, lastname, email,
// username, bio, area,
// tags, status, and joined.
// Also identity can be a field but its value must be
// verified before it is put into the database.
$allowed_field_names = ["role","firstname","lastname",
"email","username","bio","area",
"tags","status","joined","identity"];
if( 'acc.info' === $CHOSEN_TYPE )
{
// Nothing special needs to be done here
}
if( 'acc.caty' === $CHOSEN_TYPE )
{
// make sure the identity field contains one
// of the allowed values:
if ( false === isset($_POST['identity'])
||
false === in_array($_POST['identity'],
array('buyer', 'vendor', 'buyer+vendor'))
){ exit('something is wrong - cannot continue #2'); }
}
// Copy only the expected/allowed fields out of $_POST array.
$input_data = [];
foreach( $allowed_field_names as $key => $fieldname ){
if( isset($_POST[$fieldname]) && !empty($_POST[$fieldname] ))
{
$input_data[$fieldname] = $_POST[$fieldname];
}
}
// var_dump($input_data); exit(__line__);
// Update the database with the posted data and return success json message
// - First find the record via the primary key:
$db4update = ORM::for_table(TABLE_USERS)
->find_one( ['user_id'=>$CLIENT_USER_ID] );
// ---- Note that the for_table method does not escape its parameter
// ---- so the table name should not be passed directly from user input!
// - Then update whatever safe fields came throught POST and save the changes:
$db4update->set($input_data);
try {
$db4update->save();
} catch (Exception $e) {
// The update failed
echo "Error description:". $e->getMessage();
echo json_encode([
'http_code' => 402,
'status' => 0,
'error' => "An unexpected error occured",
'result' => "",
'data' => array()
]);
exit;
}
// The update has been successful
$db4read = ORM::for_table(TABLE_USERS)
->find_one( ['user_id'=>$CLIENT_USER_ID] );
$results = [];
$results['http_code'] = 200;
$results['status'] = 1;
$results['error'] = "";
$results['result'] = "The data has been updated on your profile";
$results['data'] = $db4read->as_array();
// send the result set back to the front-end
echo json_encode($results);
//update the API call count in table
$db = ORM::for_table(TABLE_API)
->where( 'user_id', $CLIENT_USER_ID)
->where( 'token', $CLIENT_TOKEN)
->where( 'expired_on', '0')
->find_one();
$db->last_call = time();
$db->call_count = $db->call_count + 1;
$db->active = 1;
$db->save();
Versus my attempt at formatting your original code:
<?php
require_once "_api_init.php";
$newdata = "";
$column = "";
$get_array_params = ['acc.info', 'acc.caty'];
if ( $results['auth'] == 1
&& isset($CLIENT_TOKEN)
&& isset($CLIENT_USER_ID)
&& isset($API_CALL_COUNT))
{
// code...
if (isset($_GET['type']) && in_array($_GET['type'], $get_array_params))
{
// code...
switch ($_GET['type'])
{
case 'acc.info':
// code...
if (isset($_POST['newrole'])) {
$newdata = $_POST['newrole'];
$column = 'role';
}
if (isset($_POST['newfirstname'])) {
$newdata = $_POST['newfirstname'];
$column = 'firstname';
}
if (isset($_POST['newlastname'])) {
$newdata = $_POST['newlastname'];
$column = 'lastname';
}
if (isset($_POST['newemail'])) {
$newdata = $_POST['newemail'];
$column = 'email';
}
if (isset($_POST['newusername'])) {
$newdata = $_POST['newusername'];
$column = 'username';
}
if (isset($_POST['newbio'])) {
$newdata = $_POST['newbio'];
$column = 'bio';
}
if (isset($_POST['newarea'])) {
$newdata = $_POST['newarea'];
$column = 'area';
}
if (isset($_POST['newtags'])) {
$newdata = $_POST['newtags'];
$column = 'tags';
}
if (isset($_POST['newstatus'])) {
$newdata = $_POST['newstatus'];
$column = 'status';
}
if (isset($_POST['newjoined'])) {
$newdata = $_POST['newjoined'];
$column = 'joined';
}
break;
case 'acc.caty': // code...
if (isset($_POST['newidentity']) && in_array($_POST['newidentity'], array('buyer', 'vendor', 'buyer+vendor')))
{
$newdata = $_POST['newidentity'];
$column = 'identity';
}
break;
} // end switch
echo "$newdata";
echo "$column";
if(strlen($newdata)>0 && strlen($column)>0)
{
if (mysqli_query($BAS_sqli_link, "UPDATE ".TABLE_USERS." SET $column = $newdata WHERE user_id='$CLIENT_USER_ID'"))
{
// The update has been successful
$q = mysqli_query($BAS_sqli_link,
"SELECT * FROM ".TABLE_USERS." WHERE user_id='$CLIENT_USER_ID' LIMIT 1");
$r = mysqli_fetch_assoc($q);
$results['http_code'] = 200;
$results['status'] = 1;
$results['error'] = "";
$results['result'] = "The column $column has been updated on your profile";
$results['data'] = $r;
//return the updated record
} else {
// The update failed
echo "Error description: ". mysqli_error($BAS_sqli_link);
$results['http_code'] = 402;
$results['status'] = 0;
$results['error'] = "An unexpected error occured";
$results['result'] = "";
$results['data'] = array();
}
}
}
else
{
$results['http_code'] = 401;
$results['status'] = 0;
$results['error'] = "Data is missing";
$results['result'] = "";
$results['data'] = array();
}
//update the API call count in table
mysqli_query($BAS_sqli_link,
"UPDATE ".TABLE_API." SET last_call = '".time()
."', call_count = call_count + 1, active = '1'
WHERE user_id = '$CLIENT_USER_ID' AND token = '$CLIENT_TOKEN'
AND expired_on = '0'");
}
// send the result set back to the front-end
echo json_encode($results);
?>
thinsoldier, thank you so much for your suggestions. Thank you immensely for your contribution, I will look immediately into the library you suggested. I appreciate the response given to me in this forum. And i will personally endeavor to participate in contributing to new topics which are posted in this forum. In so doing, my coding will get better, and improve. Thanks again thinsoldier.
I forgot to update the code to return the name of the changed field in the json message like in the original. I’ve been working with only one hand lately (holding a baby), so typing is a slow chore and couldn’t find time to type it
1 Like