
Welcome to all phpBB2 Refugees! This site is intended to continue support for the legacy 2.x line of the phpBB2 bulletin board package. If you are a fan of phpBB2, please, by all means register, post, and help us out by offering your suggestions. We are primarily a community and support network. Our secondary goal is to provide a phpBB2 MOD Author and Styles area. |
|
Author |
Message |
dogs and things Board Member

Joined: 18 Nov 2008
         Posts: 621 Location: Spain

|
Posted: Mon Oct 25, 2010 7:04 pm Post subject: Code review request? |
|
|
I use the following code to post input from a html form.
Who wants to have a look at this code to see if it is complete enough or if it needs improvements.
One of the things that needs to be added is a check for empty values.
I made a sort of attempt but this doesn´t seem very good. It is the first uncommented block, starting on line 103.
The function I need is one that checks which $POST values are empty and avoid those <li> from being inserted into the posts_text.
Don´t know if this is at all possible.
I'm using a jquery form checker that makes sure all required fields are filled out. I'm aware of the fact that I need a php check for those cases where javascript is turned of.
There is a very basic empty $POST check present, the second uncommented block starting on line 145 but it's not functional as it is.
I don't know if an if statement like Code: | if (!$_POST['el_1'] == '' && !$_POST['el_2'] == '' && !$_POST['el_3'] == '' && !$_POST['el_4'] == '' && !$_POST['el_5'] == '' && !$_POST['el_6'] == '' && !$_POST['el_7'] == '' && !$_POST['el_8'] == '' && !$_POST['el_9'] == '' && !$_POST['el_10'] == '') | is a very sane start, considering the form has some 70 $POST values that need to be checked.
Well, without any further ado, here's the code for form_submit.php
Code: | <?php
define('IN_PHPBB', true);
$phpbb_root_path = './';
include($phpbb_root_path . 'extension.inc');
include($phpbb_root_path . 'common.'.$phpEx);
include($phpbb_root_path . 'includes/bbcode.'.$phpEx);
$userdata = session_pagestart($user_ip, PAGE_INDEX);
init_userprefs($userdata);
// Details
$titulo = htmlspecialchars($_POST['titulo'], ENT_QUOTES);
$ul1 = '<ul class="lista-formulario">';
$opttit = '<li class="list-title"><b>' . htmlspecialchars($_POST['titulo'], ENT_QUOTES) . '</b></li>';
$op1 = '<li><b>Nombre del perro:</b> ' . htmlspecialchars($_POST['el_1'], ENT_QUOTES) . '</b> </li>';
$op2 = '<li><b>Edad del perro:</b> ' . htmlspecialchars($_POST['el_2'], ENT_QUOTES) . '</li>';
$op3 = '<li><b>Raza:</b> ' . htmlspecialchars($_POST['el_3'], ENT_QUOTES) . '</li>';
$op4 = '<li><b>Sexo del perro:</b> ' . $_POST['el_4'] . '</li>';
$op5 = '<li><b>Esterilizado/Castrado:</b> ' . $_POST['el_5'] . '</li>';
$op6 = '<li><b>¿Qué edad tenía el perro cuando entró en casa?</b> ' . $_POST['el_6'] . '</li>';
$op7 = '<li><b>Procedencia del perro:</b> ' . $_POST['el_7'] . '</li>';
$op8 = '<li><b>Otros casos:</b> ' . htmlspecialchars($_POST['el_8'], ENT_QUOTES) . '</li>';
$br1 = '<li class="section_break"><p>DESCRIPCIÓN DEL PROBLEMA</p></li>';
$op9 = '<li><b>Comentarios y observaciones:</b><br />' . htmlspecialchars($_POST['el_9'], ENT_QUOTES) . '<br /></li>';
$op10 = '<li><b>Problemas más graves:</b><br />' . htmlspecialchars($_POST['el_10'], ENT_QUOTES) . '<br /></li>';
$br2 = '<li class="section_break"><p>DATOS DE CONDUCTA</p></li>';
$op11 = '<li><b>Conducta comenzó:</b> ' . htmlspecialchars($_POST['el_11'], ENT_QUOTES) . '</li>';
$op12 = '<li><b>Cambios sucedidos:</b> ' . htmlspecialchars($_POST['el_12'], ENT_QUOTES) . '</li>';
$op13 = '<li><b>¿Qué haces tú ante esa conducta normalmente?:</b> ' . htmlspecialchars($_POST['el_13'], ENT_QUOTES) . '</li>';
$op14 = '<li><b>¿En algún momento o situación no se comporta así?:</b> ' . htmlspecialchars($_POST['el_14'], ENT_QUOTES) . '</li>';
$br3 = '<li class="section_break"><p>COMPORTAMIENTO (Contestar sí o no)</p></li>';
$op15 = '<li><b>Tira de la cadena:</b> ' . $_POST['el_15'] . '</li>';
$op16 = '<li><b>Se escapa cuando está suelto:</b> ' . $_POST['el__16'] . '</li>';
$op17 = '<li><b>Caza o persigue:</b> ' . $_POST['el_17'] . '</li>';
$op18 = '<li><b>Qué persigue:</b> ' . htmlspecialchars($_POST['el_18'], ENT_QUOTES) . '</li>';
$op19 = '<li><b>Ladra o arremete cuando pasea atado:</b> ' . $_POST['el_19'] . '</li>';
$op20 = '<li><b>¿A qué ladra o arremete cuando está atado?:</b> ' . htmlspecialchars($_POST['el_20'], ENT_QUOTES) . '</li>';
$op21 = '<li><b>Miedo a machos:</b> ' . $_POST['el_21'] . '</li>';
$op22 = '<li><b>Agresivo con machos:</b> ' . $_POST['el_22'] . '</li>';
$op23 = '<li><b>Se pelea con machos:</b> ' . $_POST['el_23'] . '</li>';
$op24 = '<li><b>Miedo a hembras:</b> ' . $_POST['el_24'] . '</li>';
$op25 = '<li><b>Agresivo con hembras:</b> ' . $_POST['el_25'] . '</li>';
$op26 = '<li><b>Se pelea con hembras:</b> ' . $_POST['el_26'] . '</li>';
$op27 = '<li><b>Miedo a adultos de la familia:</b> ' . $_POST['el_27'] . '</li>';
$op28 = '<li><b>Agresivo con adultos de la familia:</b> ' . $_POST['el_28'] . '</li>';
$op29 = '<li><b>Miedo a niños de la familia:</b> ' . $_POST['el_29'] . '</li>';
$op30 = '<li><b>Agresivo con niños de la familia:</b> ' . $_POST['el_30'] . '</li>';
$op31 = '<li><b>Miedo a adultos no de la familia:</b> ' . $_POST['el_31'] . '</li>';
$op32 = '<li><b>Agresivo con adultos no de la familia:</b> ' . $_POST['el_32'] . '</li>';
$op33 = '<li><b>Miedo a niños no de la familia:</b> ' . $_POST['el_33'] . '</li>';
$op34 = '<li><b>Agresivo con niños no de la familia:</b> ' . $_POST['el_34'] . '</li>';
$op35 = '<li><b>Agresivo con gente cuando pasea atado:</b> ' . $_POST['el_35'] . '</li>';
$op36 = '<li><b>Agresivo con perros cuando pasea atado:</b> ' . $_POST['el_36'] . '</li>';
$br4 = '<li class="section_break"><p>PREGUNTAS GENERALES</p></li>';
$op37 = '<li><b>¿En qué tipo de casa vives?</b> ' . htmlspecialchars($_POST['el_37'], ENT_QUOTES) . '</li>';
$op38 = '<li><b>La composición de vuestra familia:</b> ' . htmlspecialchars($_POST['el_38'], ENT_QUOTES) . '</li>';
$op39 = '<li><b>¿El perro vive dentro de la casa?</b> ' . $_POST['el_39'] . '</li>';
$op40 = '<li><b>¿Dónde duerme el perro?:</b> ' . htmlspecialchars($_POST['el_40'], ENT_QUOTES) . '</li>';
$op41 = '<li><b>¿Alguien se hace responsable de los cuidados del perro?</b> ' . $_POST['el_41'] . '</li>';
$op42 = '<li><b>¿A quién obedece el perro mejor?</b> ' . $_POST['el_42'] . '</li>';
$op43 = '<li><b>Más problemas con...</b> ' . $_POST['el_43'] . '</li>';
$op44 = '<li><b>¿Demasiado amable con las visitas?</b> ' . $_POST['el_44'] . '</li>';
$op45 = '<li><b>¿Agresivo con las visitas?</b> ' . $_POST['el_45'] . '</li>';
$op46 = '<li><b>¿Ladra mucho al sonar el timbre?</b> ' . $_POST['el_46'] . '</li>';
$op47 = '<li><b>¿Gruñe a miembros de la familia?</b> ' . $_POST['el_47'] . '</li>';
$op48 = '<li><b>¿Gruñe cuando está comiendo?</b> ' . $_POST['el_48'] . '</li>';
$op49 = '<li><b>¿Gruñe al quitarle sus juguetes?</b> ' . $_POST['el_49'] . '</li>';
$op50 = '<li><b>¿Gruñe al cepillarle?</b> ' . $_POST['el_50'] . '</li>';
$op51 = '<li><b>¿Cuantas veces le cepilláis?</b> ' . htmlspecialchars($_POST['el_51'], ENT_QUOTES) . '</li>';
$op52 = '<li><b>¿Gruñe al examinarle las orejas/patas?</b> ' . $_POST['el_52'] . '</li>';
$op53 = '<li><b>¿Gruñe al acariciarle en su cama?</b> ' . $_POST['el_53'] . '</li>';
$op54 = '<li><b>¿Gruñe al ponerle la correa?</b> ' . $_POST['el_54'] . '</li>';
$op55 = '<li><b>Otras ocasiones en que gruñe:</b> ' . htmlspecialchars($_POST['el_55'], ENT_QUOTES) . '</li>';
$op56 = '<li><b>¿Puede estar solo en casa sin problemas?</b> ' . $_POST['el_56'] . '</li>';
$op57 = '<li><b>¿Ensucia en la casa cuando esta solo?</b> ' . $_POST['el_57'] . '</li>';
$op58 = '<li><b>¿Rompe cosas cuando esta solo?</b> ' . $_POST['el_58'] . '</li>';
$op59 = '<li><b>¿Ladra y / o aúlla cuando esta solo?</b> ' . $_POST['el_59'] . '</li>';
$op74 = '<li><b>¿¿Viven más animales en la casa?</b> ' . $_POST['el_74'] . '</li>';
$br5 = '<li class="section_break"><p>PASEOS (Indicar lo que proceda)</p></li>';
$op60 = '<li><b>¿Cuantas veces al día sale a la calle?:</b> ' . htmlspecialchars($_POST['el_60'], ENT_QUOTES) . '</li>';
$op61 = '<li><b>¿Pasea atado o suelto?:</b> ' . htmlspecialchars($_POST['el_61'], ENT_QUOTES) . '</li>';
$op62 = '<li><b>¿Qué ejercicio desarrolla en los paseos?:</b> ' . htmlspecialchars($_POST['el_62'], ENT_QUOTES) . '</li>';
$br6 = '<li class="section_break"><p>COMIDAS (Indicar lo que proceda)</p></li>';
$op63 = '<li><b>¿Cuántas veces al día come el perro y a qué horas?:</b> ' . htmlspecialchars($_POST['el_63'], ENT_QUOTES) . '</li>';
$op64 = '<li><b>¿Come antes o después que los dueños?:</b> ' . htmlspecialchars($_POST['el_64'], ENT_QUOTES) . '</li>';
$op65 = '<li><b>¿Qué come?:</b> ' . htmlspecialchars($_POST['el_65'], ENT_QUOTES) . '</li>';
$op66 = '<li><b>¿Come algo entre horas?:</b> ' . htmlspecialchars($_POST['el_66'], ENT_QUOTES) . '</li>';
$br_7 = '<li class="section_break"><p>Problemas físicos del perro</p></li>';
$op67 = '<li><b>' . $_POST['el_67_1'] . ' ' . $_POST['el_67_2'] . ' ' . $_POST['el_67_3'] . ' ' . $_POST['el_67_4'] . ' ' . $_POST['el_67_5'] . ' ' . $_POST['el_67_6'] . ' ' . $_POST['el_67_7'] . ' ' . $_POST['el_67_8'] . '</b></li>';
$op68 = '<li><b>Otros (oído, vista, etc.):</b> ' . htmlspecialchars($_POST['el_68'], ENT_QUOTES) . '</li>';
$br8 = '<li class="section_break"><p>VARIOS</p></li>';
$op69 = '<li><b>¿Alguien le ha hecho algo desagradable al perro?</b> ' . $_POST['el_69'] . '</li>';
$op70 = '<li><b>¿Habéis entrenado antes con el perro?</b> ' . $_POST['el_70'] . '</li>';
$op71 = '<li><b>¿Quién?</b> ' . $_POST['el_71_1'] . $_POST['el_71_2'] . $_POST['el_71_3'] . '</li>';
$op72 = '<li><b>¿Qué?</b> ' . $_POST['el_72_1'] . $_POST['el_72_2'] . $_POST['el_72_3'] . '</li>';
$op73 = '<li><b>Mi consulta es la siguiente:</b> ' . htmlspecialchars($_POST['el_73'], ENT_QUOTES) . '</li>';
$ul2 = '</ul>';
/*$empty = array($POST['el_18'],$POST['el_55'],$POST['el_67'],$POST['el_68'],$POST['el_71'],$POST['el_72']);
if (!isset($empty)){
$op18 = '<li>Qué persigue: ' . htmlspecialchars($_POST['el_18']) . '</li>';
$op55 = '<li>Otras ocasiones en que gruñe: ' . htmlspecialchars($_POST['el_55']) . '</li>';
$op67 = '<li>PROBLEMAS FÍSICOS DEL PERRO ' . htmlspecialchars($_POST['el_67']) . '</li>';
$op68 = '<li>Otros (oído, vista, etc.): ' . htmlspecialchars($_POST['el_68']) . '</li>';
$op71 = '<li>¿Quién? ' . htmlspecialchars($_POST['el_71']) . '</li>';
$op72 = '<li>¿Qué? ' . htmlspecialchars($_POST['el_72']) . '</li>';
} else {
$op18 = '';
$op55 = '';
$op67 = '';
$op68 = '';
$op71 = '';
$op72 = '';
}*/
//Enter the title for the topic
$topic_title = $titulo;
//Message for the post here, can contain spaces and <Enter> as long as it is between the " "
$post_message = "
$ul1 $opttit $op1 $op2 $op3 $op4 $op5 $op6 $op7 $op8 $br1 $op9 $op10 $br2 $op11 $op12 $op13 $op14 $br3 $op15 $op16 $op17 $op18 $op19 $op20
$op21 $op22 $op23 $op24 $op25 $op26 $op27 $op28 $op29 $op30 $op31 $op32 $op33 $op34 $op35 $op36 $br4 $op37 $op38 $op39 $op40
$op41 $op42 $op43 $op44 $op45 $op46 $op47 $op48 $op49 $op50 $op51 $op52 $op53 $op54 $op55 $op56 $op57 $op58 $op59 $op74 $br5 $op60
$op61 $op62 $br6 $op63 $op64 $op65 $op66 $br_7 $op67 $op68 $br8 $op69 $op70 $op71 $op72 $op73 $ul2";
$current_time = time();
//Post a new topic?
$post_topic = 'yes';
//Please enter the forum id number and the id number of the user that will be used for the post:
$forum = $_POST['f'];
//If you wish to use "Guest" as the poster, please enter exactly as follows: '-1'
//otherwise enter the id number of the user you want to post the form
$user = $userdata['user_id'];
//start the process of emailing and posting
//Here is where you set the options that are required to be filled out for submission
//EX: if(!$variable == '' && !$variable2 =='')
//be sure to put at least one variable as required, and && between multiple variables
//if (!$_POST['el_1'] == '' && !$_POST['el_2'] == '' && !$_POST['el_3'] == '' && !$_POST['el_4'] == '' && !$_POST['el_5'] == '' && !$_POST['el_6'] == '' && !$_POST['el_7'] == '' && !$_POST['el_8'] == '' && !$_POST['el_9'] == '' && !$_POST['el_10'] == '')
//{
if ($post_topic == 'yes')
{
$sql = "INSERT INTO " . TOPICS_TABLE . " (topic_title, topic_poster, topic_time, forum_id, topic_status, topic_type, topic_vote) VALUES ('$topic_title', '$user', $current_time, $forum, " . TOPIC_UNLOCKED . ", 0, 0)";
if (!$db->sql_query($sql))
{
message_die(GENERAL_ERROR, 'Error in posting 1', '', __LINE__, __FILE__, $sql);
}
$topic_id = $db->sql_nextid();
$sql = "INSERT INTO " . POSTS_TABLE . " (topic_id, forum_id, poster_id, post_username, post_time, poster_ip, enable_bbcode, enable_html, enable_smilies, enable_sig) VALUES ($topic_id, $forum, $user, '', $current_time, '$user_ip', 1, 1, 1, 1)";
if (!$db->sql_query($sql, BEGIN_TRANSACTION))
{
message_die(GENERAL_ERROR, 'Error in posting 2', '', __LINE__, __FILE__, $sql);
}
$post_id = $db->sql_nextid();
$sql = "INSERT INTO " . POSTS_TEXT_TABLE . " (post_id, post_subject, bbcode_uid, post_text) VALUES ($post_id, '$post_subject', '$bbcode_uid', '$post_message')";
if (!$db->sql_query($sql))
{
message_die(GENERAL_ERROR, 'Error in posting 3', '', __LINE__, __FILE__, $sql);
}
//update forum stats
$forum_update_sql = "forum_posts = forum_posts + 1, forum_last_post_id = $post_id, forum_topics = forum_topics + 1";
$topic_update_sql = "topic_last_post_id = $post_id, topic_first_post_id = $post_id";
$sql = "UPDATE " . FORUMS_TABLE . " SET $forum_update_sql WHERE forum_id = $forum";
if (!$db->sql_query($sql))
{
message_die(GENERAL_ERROR, 'Error in posting 4', '', __LINE__, __FILE__, $sql);
}
$sql = "UPDATE " . TOPICS_TABLE . " SET $topic_update_sql WHERE topic_id = $topic_id";
if (!$db->sql_query($sql))
{
message_die(GENERAL_ERROR, 'Error in posting 5', '', __LINE__, __FILE__, $sql);
}
$sql = "UPDATE " . USERS_TABLE . " SET user_posts = user_posts + 1 " . $row['posts'] . " WHERE user_id = " . $user;
if (!$db->sql_query($sql, END_TRANSACTION))
{
message_die(GENERAL_ERROR, 'Error in posting 6', '', __LINE__, __FILE__, $sql);
}
/*
$sql = "UPDATE " . USERS_TABLE . " SET user_topics = user_topics + 1 " . $row['topics'] . " WHERE user_id = " . $user;
if (!$db->sql_query($sql, END_TRANSACTION))
{
message_die(GENERAL_ERROR, 'Error in posting 6', '', __LINE__, __FILE__, $sql);
}*/
}
//}
//else {
// Here goes the empty required fields warnig function
//}
//
// Lets build a page ...
//
redirect(append_sid("viewtopic.$phpEx?" . POST_POST_URL . "=$post_id&ipr=true", true) . '#' . $post_id);
$post_append = ( strpos($HTTP_SERVER_VARS['HTTP_USER_AGENT'], 'MSIE') !== false ) ? '&ie' : "#$post_id";
redirect( append_sid( $redirect, true ) . $post_append );
?> |
And this is form.php, the one that calls form_body.tpl which contains a big html form.
Code: | <?php
//Enter the filename containing the html to be displayed
$tpl_filename = 'form_body.tpl';
define('IN_PHPBB', true);
$phpbb_root_path = './';
include($phpbb_root_path . 'extension.inc');
include($phpbb_root_path . 'common.'.$phpEx);
include($phpbb_root_path . 'includes/bbcode.'.$phpEx);
//
// Start session management
//
$userdata = session_pagestart($user_ip, PAGE_INDEX);
init_userprefs($userdata);
if ($userdata['user_id'] == ANONYMOUS)
{
$template->assign_vars(array(
'META' => '<meta http-equiv="refresh" content="3;url=' . append_sid("login.$phpEx?redirect=formulario.$phpEx") . '">'));
message_die(GENERAL_MESSAGE, $lang['Not_form']);
}
$page_title = "Formulario de consulta";
include($phpbb_root_path . 'includes/form_header.'.$phpEx);
$template->set_filenames(array(
'body' => $tpl_filename)
);
$template->pparse('body');
include($phpbb_root_path . 'includes/form_tail.'.$phpEx);
?> |
Mind you, this is working pretty nicely as it is, but before considering this a finished project I'd like to receive some expert opinions on the code base.
Thanks in advance,
Greetings. _________________ phpBB2 will never die, I hope! |
|
Back to top |
|
 |
Dog Cow Board Member

Joined: 18 Nov 2008
         Posts: 378

|
Posted: Wed Oct 27, 2010 6:55 pm Post subject: Re: Code review request? |
|
|
dogs and things wrote: |
I don't know if an if statement like Code: | if (!$_POST['el_1'] == '' && !$_POST['el_2'] == '' && !$_POST['el_3'] == '' && !$_POST['el_4'] == '' && !$_POST['el_5'] == '' && !$_POST['el_6'] == '' && !$_POST['el_7'] == '' && !$_POST['el_8'] == '' && !$_POST['el_9'] == '' && !$_POST['el_10'] == '') | is a very sane start, considering the form has some 70 $POST values that need to be checked.
|
No, it's not a very sane start. Use a loop instead:
Code: |
$c = 0;
for($i = 0; $i < 70; $i++)
{
if($_POST['el_' . $i] != '')
{
$c++;
}
}
|
Then if $c equals some value, you know that the forms have been filled. If it's not, then some were left blank. _________________ Moof!
Lincoln's Tomb, Oak Ridge Cemetery, Springfield IL • Mac 512K Blog • Mac GUI |
|
Back to top |
|
 |
dogs and things Board Member

Joined: 18 Nov 2008
         Posts: 621 Location: Spain

|
Posted: Thu Oct 28, 2010 7:47 pm Post subject: Re: Code review request? |
|
|
Thanks a lot, I'll have a look into that.
As a matter of fact I have been searching ways of using php for form validation and to be honest I have found many roads that lead to Rome but so far I have not found a main road.
Nevertheless, I guess I'll find a way.
The purpose of my posting was in the first place to read opinions about the code that is working, the part that is used to do the phpBB2-side of the story.
Is it secure enough, is anything being overlooked, should any other additional data be inserted into the db...? _________________ phpBB2 will never die, I hope! |
|
Back to top |
|
 |
|
Not affiliated with or endorsed by the phpBB Group
Powered by phpBB2 © phpBB Group
Generated in 0.0176 seconds using 15 queries. (SQL 0.0020 Parse 0.0005 Other 0.0151) |
phpBB Customizations by the phpBBDoctor.com
Template Design by DeLFlo and MomentsOfLight.com
|
|