Checkbox data to db

[Andy_Kemp]

How to do it? If checkbox is checked insert "yes" to db else "no". And what else should i change in this code?

<?php

if(!defined('test_project')) {
	die('Hacking attempt ...');
}

if(!logged_in($ui)) {
	redirect_to('index.php?do=login');	
}

$errors = array();
$alert = NULL;

// Form is not submitted display db values.
$post = array(
	'captcha_type' => $config['captcha_type'],
	'captcha_login' => $config['captcha_login'],
	'captcha_pass_recovery' => $config['captcha_pass_recovery'],
	'captcha_reset_pass' => $config['captcha_reset_pass']
);

if(filter_has_var(INPUT_POST, 'submit_form')) {
	
	$defs = array(
		'captcha_type' => FILTER_SANITIZE_STRING,
		'captcha_login'	=> FILTER_DEFAULT,
		'captcha_pass_recovery' => FILTER_DEFAULT,
		'captcha_reset_pass' => FILTER_DEFAULT
	);
	
	$post = filter_input_array(INPUT_POST, $defs);
	
	if(demo_version($config)) {
		$errors[] = $lang['error']['h_0000'];
	}
	
	else {
		
		if(!NoCSRF::valid_token($lang, 'csrf_token', $_POST)) {
			$errors[] = NoCSRF::get_error();
		}
		
		else {
			
			if(!user_has_permission($db, $ui, 'edit_captcha_settings')) {
				$errors[] = $lang['error']['h_0001'];
			}
			
		}
		
	}
			
}

if(filter_has_var(INPUT_POST, 'submit_form') && empty($errors)) {
	
	foreach($post as $key => $value) {
		
		$query = 'UPDATE config SET value = :value WHERE config = :key';
  		$update = $db->prepare($query);
		$update->bindParam(':value', $value, PDO::PARAM_STR);
		$update->bindParam(':key', $key, PDO::PARAM_STR);
		$bool = $update->execute();
		
	}
	
	if($bool) {
		$alert = array('type' => 'success', 'message' => $lang['success']['h_0000']);
	}
	
	else {
		$alert = array('type' => 'error', 'message' => $lang['error']['h_0000']);
	}
	
}

$smarty->assign('post', $post);
$smarty->assign('errors', $errors);
$smarty->assign('alert', $alert);
$smarty->assign('csrf_token', NoCSRF::generate('csrf_token'));
$smarty->display('captcha_settings.tpl');

$db = NULL;	
$config = NULL;
$ui = NULL;
$lang = NULL;

?>

<input type="checkbox" name="captcha_login" value="1">Captcha login page enabled<br>
<input type="checkbox" name="captcha_pass_recovery" value="1">Captcha password recovery page enabled<br> 
<input type="checkbox" name="captcha_reset_pass" value="1">Captcha reset password page enabled

[Barand]

Only checked checkbox values are posted so you need to do something like this

$captcha_reset_pass = isset($_POST['captcha_reset_pass'] ? 'yes' : 'no';

[Jacques1]

This also means you can get rid of the weird filter gymnastics, because what matters is whether the checkbox field has been sent or not. The actual value is entirely irrelevant and should simply be discarded.

 

The PHP filters are generally very questionable and tend to damage your data rather than providing security. For example, the input “I <3 PHP” will be truncated to “I ”, because the “<” is misinterpreted as the beginning of an HTML tag (which is of course complete nonsense). Don't use the filter_* functions unless you know exactly what they're doing and why you need them.

 

I also don't think that “yes” and “no” are appropriate configuration values. If you insist on stuffing everything into a text column, you should at least choose some kind of canonical representation so that you don't end up with dozens of variations (“yes”/“no” vs. “on”/“off” vs. “yea”/“nay” ...). Booleans are commonly represented by “true” and “false”.

[Andy_Kemp]

Changed this block

$defs = array(
    'captcha_type' => FILTER_SANITIZE_STRING,
    'captcha_login' => FILTER_DEFAULT,
    'captcha_pass_recovery' => FILTER_DEFAULT,
    'captcha_reset_pass' => FILTER_DEFAULT
);
	
$post = filter_input_array(INPUT_POST, $defs);

to

$post = array(
    'captcha_type' => $_POST['captcha_type'],
    'captcha_login' => isset($_POST['captcha_login']) ? 'true' : 'false',
    'captcha_pass_recovery' => isset($_POST['captcha_pass_recovery']) ? 'true' : 'false',
    'captcha_reset_pass' => isset($_POST['captcha_reset_pass']) ? 'true' : 'false'
);

Now i have another question. When is right time to close db connection?

Do i really need all of this at the end of page?

$db = NULL;	
$config = NULL;
$ui = NULL;
$lang = NULL;

[Jacques1]

You don't need any of this. When the script is terminated, PHP automatically closes the connection and frees all memory.

 

By the way, you should move the prepare() call before the foreach loop. Prepared statements can be reused (that's pretty much the whole point), so you should create it once and then use it for every configuration item.

[Psycho]

@Andy_Kemp

 

Regarding Jacques1's previous statement:

 

I also don't think that “yes” and “no” are appropriate configuration values. If you insist on stuffing everything into a text column, you should at least choose some kind of canonical representation so that you don't end up with dozens of variations (“yes”/“no” vs. “on”/“off” vs. “yea”/“nay” ...). Booleans are commonly represented by “true” and “false”.

 

A Boolean is not the string value of "true" or "false". It is a distinct data type just like strings, integers, floats, etc. Depending on the language/context it can be represented in different ways. In PHP, for example, the literal TRUE Boolean value is represented as:

$foo = TRUE; //Literal Boolean

However, you can also use other values that will be interpreted as true/false within PHP. Basically anything with a non 0, non-empty value would be interpreted as True (http://php.net/manual/en/language.types.boolean.php). In the MySQL there is no literal Boolean field type. There is a Bool field type which is just an alias for a tiny int. The intent is that Booleans are stored as 0/1. In fact, if you use a literal Boolean in the INSERT query it will be stored as 0 or 1.

 

By storing the strings "true"/"false" you would have to add additional/unnecessary logic when using those values, e.g.

if($captcha_login_VALUE_FROM_DB=='true') { 
    //do something
}

Instead, if you just store as a Boolean/Int

'captcha_login' => isset($_POST['captcha_login']) //Will be TRUE or FALSE

Then you can use the returned value directly without having to compare to an arbitrary string

if($captcha_login_VALUE_FROM_DB) {
    //do something
}

Edited December 15, 2015 by Psycho

[Andy_Kemp]

My db value column type is VARCHAR, because it holds any kind of data.

Now i tried. It stores TRUE as 1 and FALSE has no value.

[Psycho]

My db value column type is VARCHAR, because it holds any kind of data.

 

Then you are doing it wrong. You stated previously that the value would be "yes" or "no" based on the checked status. That implies a Boolean value. If you are mixing a Boolean and other types of values in the same field it sounds as if the DB schema isn't correct for the intended need

[Andy_Kemp]

TABLE NAME config 
id = INT(11), config = varchar(255), value = varchar(255)
+----------+--------------------------------+----------------------------------+
|    id    |	config	                    |	value	                       |
+----------+--------------------------------+----------------------------------+
| 1        | captcha_type  	            | disabled                         | # disabled, recaptcha_version_2, solvemedia 
+----------+--------------------------------+----------------------------------+
| 2        | captcha_login  	            | true	                       | # false, true
+----------+--------------------------------+----------------------------------+
| 3        | captcha_pass_recovery  	    | true	                       | # false, true
+----------+--------------------------------+----------------------------------+
| 4        | captcha_reset_pass  	    | false	                       | # false, true
+----------+--------------------------------+----------------------------------+
| 5        | recaptcha_version_2_site_key   | K7ERoXZln9UvnD.NaOksQEDlfkRWMlYz |
+----------+--------------------------------+----------------------------------+
| 6        | recaptcha_version_2_secret_key | K7ERoXZln9UvnD.NaOksQEDlfkRWMlYz |
+----------+--------------------------------+----------------------------------+
| 7        | recaptcha_version_2_type  	    | image	                       | # audio, image
+----------+--------------------------------+----------------------------------+
| 8        | recaptcha_version_2_theme      | light	                       | # dark, light
+----------+--------------------------------+----------------------------------+
| 9        | recaptcha_version_2_size  	    | normal	                       | # compact, normal
+----------+--------------------------------+----------------------------------+
| 10       | solvemedia_public_key          | K7ERoXZln9UvnD.NaOksQEDlfkRWMlYz |
+----------+--------------------------------+----------------------------------+
| 11       | solvemedia_private_key  	    | K7ERoXZln9UvnD.NaOksQEDlfkRWMlYz |
+----------+--------------------------------+----------------------------------+
| 12       | solvemedia_hash_key            | K7ERoXZln9UvnD.NaOksQEDlfkRWMlYz |
+----------+--------------------------------+----------------------------------+
| 13       | site_name                      | Testing site name                |
+----------+--------------------------------+----------------------------------+
| 14       | site_title                     | Testing site title               |
+----------+--------------------------------+----------------------------------+
| 15       | site_description               |                                  |
+----------+--------------------------------+----------------------------------+
| 16       | site_keywords                  |                                  |
+----------+--------------------------------+----------------------------------+
| 17       | site_url                       | www.testingsite.com/test         |
+----------+--------------------------------+----------------------------------+

.....................................................................................................................................................................

Edited December 16, 2015 by Andy_Kemp

[Andy_Kemp]

You don't need any of this. When the script is terminated, PHP automatically closes the connection and frees all memory.

 

By the way, you should move the prepare() call before the foreach loop. Prepared statements can be reused (that's pretty much the whole point), so you should create it once and then use it for every configuration item.

You mean something like this

$query = 'UPDATE config SET value = :value WHERE config = :key';
$update = $db->prepare($query);
$update->bindParam(':value', $value, PDO::PARAM_STR);
$update->bindParam(':key', $key, PDO::PARAM_STR);
	
foreach($post as $key => $value) {
		
    $value = $value;
    $key = $key;
    $bool = $update->execute();
		
}	

[Jacques1]

No. I mean that you should create the prepared statement outside of the loop and then execute it with different values inside the loop:

$configurationUpdateStmt = $db->prepare('
    UPDATE config
    SET value = :value
    WHERE config = :key
');

foreach ($post as $configKey => $configValue)
{
    $configurationUpdateStmt->bindParam('key', $configKey, PDO::PARAM_STR);
    $configurationUpdateStmt->bindParam('value', $configValue, PDO::PARAM_STR);

    $configurationUpdateStmt->execute();
}

[Andy_Kemp]

http://php.net/manual/en/pdo.prepared-statements.php

<?php
$stmt = $dbh->prepare("INSERT INTO REGISTRY (name, value) VALUES (:name, :value)");
$stmt->bindParam(':name', $name);
$stmt->bindParam(':value', $value);

// insert one row
$name = 'one';
$value = 1;
$stmt->execute();

// insert another row with different values
$name = 'two';
$value = 2;
$stmt->execute();
?>

[Jacques1]

Yes, you can theoretically just assign new values to the variables, since they're passed by reference.

 

However, it's much, much clearer and less error-prone to explicitly bind each value to the prepared statement.