phpBB2Refugees.com Logo
Not affiliated with or endorsed by the phpBB Group

Register •  Login 

Continue the legacy...

Welcome to all phpBB2 Refugees!Wave Smilie

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.

Code review request?


 
Search this topic... | Search MOD Writing... | Search Box
Register or Login to Post    Index » MOD Writing  Previous TopicPrint TopicNext Topic
Author Message
dogs and things
Board Member



Joined: 18 Nov 2008

Posts: 621
Location: Spain


flag
PostPosted: 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


flag
PostPosted: 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 ILMac 512K BlogMac GUI
Back to top
dogs and things
Board Member



Joined: 18 Nov 2008

Posts: 621
Location: Spain


flag
PostPosted: 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
Display posts from previous:   
Register or Login to Post    Index » MOD Writing  Previous TopicPrint TopicNext Topic
Page 1 of 1 All times are GMT
 
Jump to:  

Index • About • FAQ • Rules • Privacy • Search •  Register •  Login 
Not affiliated with or endorsed by the phpBB Group
Powered by phpBB2 © phpBB Group
Generated in 0.0151 seconds using 16 queries. (SQL 0.0023 Parse 0.0005 Other 0.0123)
phpBB Customizations by the phpBBDoctor.com
Template Design by DeLFlo and MomentsOfLight.com Moments of Light Logo