php query causing responses to post as three rows instead of one per user

Question

I am making one of my first databases and am using one table to contain all resposnes in a php/mysql survey. The responses, however, are posting to the one table but in three different rows. I suspect it has to do with the query being executed 3x for the three section responses. Do I need to concactenate this and, if so, how? Is there another solution.

Here's the HTML Form:

<html>
<?php include 'C:\xampp\htdocs\paxdb\head.php'; 
include 'config/menu.php';?> 
<div id="dataentry">

<!--This section is the demographic text field area-->
<form method="post" action="dataentered.php">
First Name:&nbsp;<input type="text" name="First_Name"/></br>
</br>
Last Name:&nbsp;<input type="text" name="Last_Name"/></br>
</br>
E-mail:&nbsp;<input type="text" name="email"/></br>
</br>

<!--This section is the age range checkbox selection area-->
<p><u><b>Age Range</b></u></p>
<input type="checkbox" name="age[]" id="20-25" value="20-25"/>&nbsp;20-25</br>
<input type="checkbox" name="age[]" id="26-30" value="26-30"/>&nbsp;26-30</br>
<input type="checkbox" name="age[]" id="31-35" value="31-35"/>&nbsp;31-35</br>
</div>
<div id="checkboxes">
</div>

<!--This section is the trips take checkbox area-->
<div id="tripstodatetype">
<p><u><b>WHAT TYPE OF TRIPS TO DATE HAVE YOU TAKEN?</b></u></p>
<input type="checkbox" name="trip2date[]" id="Bus" value="Bus">&nbsp;Bus&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</br>
<input type="checkbox" name="trip2date[]" id="Car" value="Car">&nbsp;Car</br>
<input type="checkbox" name="trip2date[]" id="Weekend fly-in" value="Weekend fly-in">&nbsp;Weekend fly-in&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</br>
</div>
<div id="tripstodateborder">
</div>

<!--This section is the type of trip client likes best checkbox area-->
    <div id="triplikebest">
<p><u><b>WHAT TYPE OF TRIP DO YOU LIKE BEST?</b></u></p>
<input type="checkbox" name="triplikebest[]" value="Bus">&nbsp;Bus&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</br>
<input type="checkbox" name="triplikebest[]" value="Car">&nbsp;Car</br>
<input type="checkbox" name="triplikebest[]" value="Weekend fly-in">&nbsp;Weekend fly-in&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;</br>
</div>
<div id="triplikeborder">
</div>

and the corresponding PHP:

<html>
<?php
include 'head.php';
include 'config/menu.php'; 
$host="localhost";
$username="someusername";
$password="somepass";
$dbname="somedb";

$dbc = mysql_connect($host, $username, $password, $dbname);
if (!$dbc)
{
    die('Error connecting to MySQL server' . mysql_error());
    }
mysql_select_db($dbname, $dbc);

//send user data to the database table
$first_name=$_POST['First_Name'];   
$last_name=$_POST['Last_Name'];
$email=$_POST['email'];

mysql_query("INSERT INTO pax (First_Name, Last_Name, email)
VALUES('$first_name','$last_name','$email')"); 

//send age data to the database table
$age = $_POST['age'];
$my_range = "";
foreach($age as $range) 
$my_range = $my_range . $range . " ";
mysql_query("INSERT INTO pax(age) VALUES ('$my_range')") or die (mysql_error()); 

//send trip to date data to the database table
$trip2date = $_POST['trip2date'];
$my_triprange = "";
foreach($trip2date as $triprange) 
$my_triprange = $my_triprange . $triprange . ", ";
mysql_query("INSERT INTO pax(trip2date) VALUES ('$my_triprange')") or die (mysql_error()); 


mysql_close($dbc);
?>

Your help is greatly appreciated.

Where to start with this one !!! SQL Injection or Deprecated mysql_ or the multiple inserts ..... — Manse, Sep 10 '12 at 16:06
Nice [SQL injection holes](http://bobby-tables.com). Enjoy having your server pwn3d. — Marc B, Sep 10 '12 at 16:06

David Precious · Answer 1 · 2012-09-10T16:12:12.737

... you're running 3 INSERT queries. I think you seriously misunderstand what you're trying to achieve. You probably want to either concatenate the values for the 'trip2date' param, or preferably insert multiple records in another table (look in to basic relational database usage).

Also, why can a user select multiple age ranges? How can someone be both 20-25 and 26-30? You probably want a radio group there, so they can select only one option. (Again, you're inserting potentially multiple rows there, too; if a user ticks all three age ranges (which you allow them to, for no discernable reason), then you'll insert one row containing their name and email, then three rows containing only their age.)

Also, holy SQL injection, batman. See http://bobby-tables.com/

I assume you're still learning, but learn not to write insecure code first before worrying about anything else.

Yes, you should probably just get all your information ready and then make a single database insert with the data for all fields. — Mike Brant, Sep 10 '12 at 16:09

score 0 · Answer 2 · answered Sep 10 '12 at 16:10

0

You are executing three INSERTs. An INSERT query creates a new record in the database.

If you want to store the information in the same row, there should be one INSERT query encompassing the age and trip2date values.

Or, if you want to keep the structure have one INSERT and two UPDATE queries which update the record by whatever primary key you have.

answered Sep 10 '12 at 16:10

johntayl

140
2
8

Thank you - this was my assumption, that the php inserts were the cause of the problem; but I'm trying to find a method of concactenating the queries (if that is even possible). If not, how do I design tables (or should I) to resolve the matter? – shane Sep 10 '12 at 20:37

score 0 · Accepted Answer · edited May 23 '17 at 10:24

0

Wow okay, I agree with ManseUK

1) Your queries are extremely vulnerable to SQL Injection. See - How does the SQL injection from the "Bobby Tables" XKCD comic work?

2) mysql library is deprecated, the library is no longer being developed. You should use MySQLi or PDO - see http://www.php.net/manual/en/mysqlinfo.api.choosing.php

3) Your database is not normalised. That is you have multiple values going into one field of a row (all the trip dates. See http://en.wikipedia.org/wiki/Database_normalization

4) You're running 3 insert queries. You should do it in one query by preparing the data first.

UPDATE

Okay here's my solution. It comes with some more advice. Don't attack or insult the people helping you. We have no idea where this code will end up and it's a bad habit to get into designing insecure software. Bad code in the wild produces more bad code which ends up being responsible for making the IT industry look like morons. LinkedIn's breach was SQL Injection, most likely. Some on this site will learn the hard way.

Second, the aim of the site is to educate people as well as solve problems. So a primer on databases.

When you run an INSERT query it creates one or more rows in a table in the database. The fields you specify are filled in with the data you provide, the rest are filled with blanks. 3 INSERT queries? 3 rows.

Here's a few versions to improve it

Version 1.0: 'mysql_' Single Row (With SQL Injection fixed):

<?php
//Database Connection as Before ...
$first_name = mysql_real_escape($_POST['First_Name']); 
$last_name = mysql_real_escape($_POST['Last_Name']);
$email = mysql_real_escape($_POST['email'];
foreach($age as $range) {
   $my_range = $my_range . mysql_real_escape($range) . ", ";
}
foreach($trip2date as $triprange) {
   $my_triprange = $my_triprange . mysql_real_escape($triprange) . ", ";
}
mysql_query("INSERT INTO `pax` (`First_Name`, `Last_Name`, `email`, `age`,`trip2date`)   
     VALUES('$first_name','$last_name','$email', '$my_range','$my_triprange')") 
     or die(mysql_error()); 
mysql_close($dbc);
?>

Version 2.0: 'mysql_' Single Row (With SQL Injection fixed):

<?php

$dbc = @new mysqli($host, $username, $password, $dbname);
if ($dbc->connect_error) {
  die('Error connecting to MySQL server' . $mysqli->connect_error);
}
$first_name = $dbc->real_escape($_POST['First_Name']); 
$last_name = $dbc->real_escape($_POST['Last_Name']);
$email = $dbc->real_escape($_POST['email'];
foreach($_POST['age'] as $range) {
   $my_range = $my_range . $dbc->real_escape($range) . ", ";
}
foreach($trip2date as $triprange) {
   $my_triprange = $my_triprange . $dbc->real_escape($triprange) . ", ";
}
$dbc->query("INSERT INTO `pax` (`First_Name`, `Last_Name`, `email`, `age`,`trip2date`)   
     VALUES('$first_name','$last_name','$email', '$my_range','$my_triprange')") 
     or die($dbc->error()); 
$dbc->close();
?>

Regarding the database, multi-table would be the ideal way to store the ranges. However, given this is a small project, it's probably sufficient to just use the list as a non-normalised field. I normally don't bother with FOREIGN KEYS even in a large project (which don't work in the default DB type (MyISAM)) and just enforce it application side. Database developers on here will differ.

Sample DB structure and content

pax  //Sample data, only a few fields for example
-----------
id , firstname , lastname
1 , 'John' , 'Doe',
2,  'Joe', 'Bloggs',
3, 'Anne', 'Smith',

pax_ages
--------
pax_id, ageGroup
1, 20-55
1, 55+
2, 20-55
3, 20-55

Queries & some pseudo code to insert

//Do insert similar to V2.0 (removing age) and then
$id = $mysqli->insert_id;  //mysql version $id = mysql_insert_id();
foreach($_POST['age'] as $range) {
   $iQ = "INSERT INTO `pax_ages` VALUES('$id', '$mysqli->real_escape($range)')";
}
while($row = $result->fetch_assoc) {   // mysql version: $row = mysql_fetch_assoc($result)
   $query2 = "SELECT * FROM `pax_ages` WHERE `pax_id` = $row['id']";
   while($age_row = $result2->fetch_assoc) {
       $ageRanges .=  $age_row['ageGroup'];
   }
}

Queries & some pseudo code to retrieve

"SELECT * FROM `pax`";
while($row = $result->fetch_assoc) {   // mysql version: $row = mysql_fetch_assoc($result)
   $query2 = "SELECT * FROM `pax_ages` WHERE `pax_id` = '$row['id']'";
   while($age_row = $result2->fetch_assoc) {
       $ageRanges .=  $age_row['ageGroup'];
   }
}

edited May 23 '17 at 10:24

Community

1
1

answered Sep 10 '12 at 16:12

Philip Whitehouse

4,293
3
23
36

1) irrelevant. 2) irrelevant. 3) Thank you, I know this. Question is how do I resolve this (steps?). 4) Save as #3, my original question stated and asked exactly this ... how do I concactenate these three queries to one? Or does it even make sense to take such an approach? – shane Sep 10 '12 at 20:40
Updated my answer with solutions – Philip Whitehouse Sep 10 '12 at 21:54
Thank you. First, I apologize for blasting all in my responses and yourself. In light of a different day, I can see you were being genuine in your answer. One note of kind in return to all others - when writing with a reply that is loaded with passive-aggressive input as to how someone is approaching a project ... be prepared to get a little in return. Instead, try to approach your advice in a more constructive tone ... e.g. "I think you seriously misunderstand what you're trying to achieve." is a passive-aggressive attack on the person asking for help. – shane Sep 11 '12 at 23:25
Back to the subject, my dbase is in INNODB vs. MyISAM (-not certain if that has a deliterious affect on your answer/solution). Thanks! I see how you concactenated the insert - makes sense. There are some later questions on the survey in which multiple-checkboxes may be used in the response by the customer. Would this require separate tables (yes, I've been reading up some - though it's not quite clear to me as far as normalization in the 2nd normal form process and use of keys, etc. I understand the goal - just not the 'how to' without simply making up a key.) Question: – shane Sep 11 '12 at 23:36
does "real_escape..." also remove 'odd' characters a user might enter without intending harm? Users of the database (as well as the customers) are from another country and might use the accent marks or letters of the language (alt-243, etc) - would this clean it up? – shane Sep 11 '12 at 23:38
InnoDB vs MyISAM is essentially irrelevant for this situation - it's related to handling processes involving multiple queries which must be all completed or all ignored. If you are never going to change the survey then there's not much point messing around with multiple tables. However I'll append a quick write up of how to connect the tables. – Philip Whitehouse Sep 12 '12 at 01:06
Real escape doesn't remove 'odd' characters. It ensures that attempts to break the database don't work. For example, if I wrote "SELECT * FROM users WHERE username = '$username' and $password = '$password'"; Some guy could send: http://mydomain.com/submit.php?username=admin&password=' OR 1=1 and the fact that I got the password wrong would be irrelevant. It would see: SELECT * FROM users WHERE username = 'admin' and password = '' OR 1=1; and I would be logged in as admin. To 'clean up' the data is difficult. The solution is to limit what can be entered and check each one manually. – Philip Whitehouse Sep 12 '12 at 01:08
Hi, I ran V2.0 first (there was a syntax error on line 16 (missing backet) but there's a problem with the real escape string that I'm not certain how to correct. 'Fatal error: Call to undefined method mysqli::real_escape() in C:\xampp\htdocs\paxDB\dataentered.php on line 14' – shane Sep 13 '12 at 00:19
and here is line 14: $first_name = $dbc->real_escape($_POST['First_Name']); – shane Sep 13 '12 at 00:21
I then tried the mysqli version you wrote and obtained the following error: 'Parse error: syntax error, unexpected T_ENCAPSED_AND_WHITESPACE, expecting T_STRING or T_VARIABLE or T_NUM_STRING in C:\xampp\htdocs\paxDB\dataentered.php on line 22' and here is line 22: ' $query2 = "SELECT * FROM `pax_ages` WHERE `pax_id` = $row['id']"; ' for which I see nothing wrong. (NOTE: the ' is added to the code to separate from my text replies - it's not part of the code). – shane Sep 13 '12 at 00:24
Think I figured out the line 22 error ... $row['id'] should be '$row[id]' instead, correct? (this time the ' in the code is not added for comments). Then I re-ran the code and got the following error: Fatal error: Call to undefined method mysqli::real_escape() in C:\xampp\htdocs\paxDB\dataentered.php on line 14 – shane Sep 13 '12 at 00:31
line 14 is the line defining $first_name. – shane Sep 13 '12 at 00:33
ok, figured it out ... 1) updated XAMPP, 2) change 'real_escape' to 'real_escape_string, 3) defined '$my_triprange' ... all works! Thanks for your help!!!!!!!!! (VERY happy). – shane Sep 13 '12 at 02:13
New question with same database: If I were to put the triprange data into a separate table, how would I convert the following insert query to perform the insert into the new table? (let's say the new table is called 'trip'). mysql_query("INSERT INTO `pax` (`First_Name`, `Last_Name`, `email`, `age`,`trip2date`) VALUES('$first_name','$last_name','$email', '$my_range','$my_triprange')") or die(mysql_error()); – shane Sep 13 '12 at 02:54
Post it as a new question on the site please. – Philip Whitehouse Sep 13 '12 at 17:22

php query causing responses to post as three rows instead of one per user

3 Answers3

Here's a few versions to improve it