Andy_Kemp Posted December 14, 2015 Share Posted December 14, 2015 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 Quote Link to comment Share on other sites More sharing options...
Barand Posted December 14, 2015 Share Posted December 14, 2015 Only checked checkbox values are posted so you need to do something like this $captcha_reset_pass = isset($_POST['captcha_reset_pass'] ? 'yes' : 'no'; Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted December 14, 2015 Share Posted December 14, 2015 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”. Quote Link to comment Share on other sites More sharing options...
Andy_Kemp Posted December 15, 2015 Author Share Posted December 15, 2015 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; Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted December 15, 2015 Share Posted December 15, 2015 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. Quote Link to comment Share on other sites More sharing options...
Psycho Posted December 15, 2015 Share Posted December 15, 2015 (edited) @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 Quote Link to comment Share on other sites More sharing options...
Andy_Kemp Posted December 15, 2015 Author Share Posted December 15, 2015 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. Quote Link to comment Share on other sites More sharing options...
Psycho Posted December 15, 2015 Share Posted December 15, 2015 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 Quote Link to comment Share on other sites More sharing options...
Andy_Kemp Posted December 16, 2015 Author Share Posted December 16, 2015 (edited) 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 Quote Link to comment Share on other sites More sharing options...
Andy_Kemp Posted December 16, 2015 Author Share Posted December 16, 2015 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(); } Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted December 16, 2015 Share Posted December 16, 2015 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(); } Quote Link to comment Share on other sites More sharing options...
Andy_Kemp Posted December 16, 2015 Author Share Posted December 16, 2015 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(); ?> Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted December 16, 2015 Share Posted December 16, 2015 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. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.