Problem with inserting values from html form into mysql table

[chrisdee]

Im having a hard time trying to insert data into an mysql table.
The script works fine untill the query part at the bottom of the script.
mysqli_affected_rows returns -1 most of the time and no data values are entered into the mysql table.

print_r($values); correctly returns the values I input into the form, but those values are not inserted into the table.

Any ideas ?

<?php
$connect = mysqli_connect('localhost','database','user','pass');

echo "<a href=./>Home</a><br>";

$query = mysqli_query($connect, "SELECT filename,dato,size,spoken,speaker,dialect,project,gender FROM pdb");
$fetch_data = mysqli_fetch_array($query, MYSQLI_ASSOC);

foreach($fetch_data as $value) {
      $values = $_REQUEST[$value];
}
if (empty($_POST)) {
  echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
  echo "<table border=1>";

  foreach($fetch_data as $key => $value) {
    echo "<tr><td>" . $key . "</td><td><input type='text' name='" . $value . "[]'></td></tr>";
 }
  echo "</table>";
  echo "<input type='submit' value='Insert'>";
  echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
  echo "</form>";
}
else {
  echo "<pre>";
  print_r($values);
  echo "</pre><hr>";
  mysqli_query($connect,"INSERT INTO pdb ('filename','dato','size','spoken','speaker','dealect','project','gender') VALUES ('$values[0]','$values[1]','$values[2]','$values[3]','$values[4]','$values[5]','$values[6]','$values[7]')");
  echo mysqli_affected_rows($connect);
}
?>

[Weedpacket]

Have you looked at the query that is being sent to the database?

$query = "INSERT INTO pdb ('filename','dato','size','spoken','speaker','dealect','project','gender') VALUES ('$values[0]','$values[1]','$values[2]','$values[3]','$values[4]','$values[5]','$values[6]','$values[7]')";
echo $query;
  mysqli_query($connect,$query); 

Have you looked to see if MySQL returned an error? (Hint: according to the documentation for mysqli::$affected_rows, a value of -1 means exactly that)

echo mysqli_error($connect);

[chrisdee]

echo mysql_error($connect) gave me the following message :

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''filename,dato,size,spoken,speaker,dialect,project,gender' VALUES

This message went away after I removed the '' in the query names.

echo $query gives me the following message :

Catchable fatal error: Object of class mysqli_result could not be converted to string in /var/www/pdb/insert.php

Im not shure what this means.

[Derokorian]

Don't put single quotes around table/column name identifiers. Use back ticks ` instead

[chrisdee]

Derokorian;10999557 wrote:

Don't put single quotes around table/column name identifiers. Use back ticks ` instead

I have now changed all the single quotes with ticks instead.
Now I get the following error message :

Unknown column '' in 'field list'

What does this mean ?

[bradgrafelman]

chrisdee;10999565 wrote:

Now I get the following error message :

Unknown column '' in 'field list'

What does this mean ?

It means that the SQL server doesn't know of any column named '' (the empty string, apparently) that you referenced in the field list. (In other words, it means exactly what the error message states.)

Can you show us what your code looks like now (specifically the SQL query)?

[sneakyimp]

The query probably has two commas or two backticks in a row.

[chrisdee]

Here is the code now :

<?php
$connect = mysqli_connect("localhost","user","password","database") die (mysqli_connect_error());

echo "<a href=./>Home</a><br>";

$query = mysqli_query($connect, "SELECT `filename`,`dato`,`size`,`spoken`,`speaker`,`dialect`,`project`,`gender` FROM pdb") or die (mysqli_connect_error());
$fetch_data = mysqli_fetch_array($query, MYSQLI_ASSOC);

foreach($fetch_data as $value) {
  $values = $_REQUEST[$value];
}
if (empty($_POST)) {
  echo "<form method=`post` action=`".$_SERVER['PHP_SELF']."`>";
  echo "<table border=`1`>";

  foreach($fetch_data as $key => $value) {
    echo "<tr><td>" . $key . "</td><td><input type=`text` name=`" . $value . "[]`></td></tr>";
 }
  echo "</table>";
  echo "<input type=`submit` value=`Insert`>";
  echo "<input type=`button` onclick=`history.go(-1)` value=`Cancel`>";
  echo "</form>";
}
else {
  #echo "<pre>";
  #print_r($values);
  #echo "</pre><hr>";
  $query="INSERT INTO pdb (`filename`,`dato`,`size`,`spoken`,`speaker`,`dialect`,`project`,`gender`) VALUES (`$values[0]`,`$values[1]`,`$values[2]`,`$values[3]`,`$values[4]`,`$values[5]`,`$values[6]`,`$values[7]`)";
  echo $query;
  mysqli_query($connect,$query) || die (mysqli_error($connect));
  #echo "<br>" . mysqli_affected_rows($connect);
  echo "<br>" . mysqli_error($connect);
}
?>

[bradgrafelman]

Backticks (`) are used to denote identifiers, e.g. the name of a database/table/column/view/etc. Single quotes are used to denote ordinary strings.

Thus, by surrounding all of the items in your VALUES() section with backticks, you're telling MySQL that each of them is the name of a database/table/column/view/etc. rather than just string data.

EDIT: Also, note that using backticks (or double quotes as applicable) to denote identifiers is optional, unless of course you picked a reserved word (or used something like hypens or spaces).

This begs the question.. why are you using delimiters around the identifiers in your query to begin with?

[sneakyimp]

I see a numerous problems.
1) This query fetches ALL the records in pdb and then just grabs one record from the result set:

$query = mysqli_query($connect, "SELECT `filename`,`dato`,`size`,`spoken`,`speaker`,`dialect`,`project`,`gender` FROM pdb") or die (mysqli_connect_error());
$fetch_data = mysqli_fetch_array($query, MYSQLI_ASSOC); 

You should limit the query using a WHERE or LIMIT clause. E.g.,

$query = mysqli_query($connect, "SELECT `filename`,`dato`,`size`,`spoken`,`speaker`,`dialect`,`project`,`gender` FROM pdb LIMIT 1") or die (mysqli_connect_error());

2) You are using backticks for both database column names and for the values you are trying to insert. Use backticks for column names, quotes (single or double) for values.

3) You don't escape any of the members of $values before inserting them right into your database, thus running the risk of SQL injection.

Your code looks pretty confused to me. You might want to refactor it a bit -- maybe define $values closer to where you actually use it -- and try filtering and validating some of the incoming data. Its' not really clear what you're trying to accomplish here so perhaps you could spell it out.

[sneakyimp]

Derkorian told him to use backticks (see above).

[bradgrafelman]

sneakyimp;10999610 wrote:

Derkorian told him to use backticks (see above).

Right, but that was after the OP had already surrounded the identifiers with single quotes. He was right in noting that backticks are one of the correct delimiters to use for identifiers, but my point was that they shouldn't even need to be delimited at all.

[Weedpacket]

sneakyimp wrote:

1)...
2)...
3)...

4) Parameterised queries, anyone?

[bradgrafelman]

Also..

sneakyimp;10999609 wrote:

I see a numerous problems.
1) This query fetches ALL the records in pdb and then just grabs one record from the result set:

$query = mysqli_query($connect, "SELECT `filename`,`dato`,`size`,`spoken`,`speaker`,`dialect`,`project`,`gender` FROM pdb") or die (mysqli_connect_error());
$fetch_data = mysqli_fetch_array($query, MYSQLI_ASSOC); 

You should limit the query using a WHERE or LIMIT clause. E.g.,

$query = mysqli_query($connect, "SELECT `filename`,`dato`,`size`,`spoken`,`speaker`,`dialect`,`project`,`gender` FROM pdb LIMIT 1") or die (mysqli_connect_error());

You should also add an 'ORDER BY' clause (or a 'WHERE' clause that matches only one row), otherwise you don't even know which row in the table you might get.

sneakyimp;10999609 wrote:

Use backticks for column names, quotes (single or double) for values.

Make that only single quotes; double quotes can be used to delimit identifiers.

[chrisdee]

Thanks for all the information.

I am attempting to build a phonetic database for one of our professors at work. Through PHP he and a couple of other people should be able to upload wav/mp3 files to the server and in addition tag the files with phonetic related information. In addition to upload one file at a time the script should also let the users upload multiple files to the server in one operation and give these files the same tag info. This is for cases where wav/mp3 content is different but the metadata should be the same.

But the script below is still very much in the beginning stage of the end target. Im currently trying to understand how to properly insert data from user input into a mysql table from an html form.

Anyway, I have updated the script and tried to accommodate your suggestions about security and such. Im shure there are still much room for improvement so please feel free to comment so I can make it better.

<?php
$connect = mysqli_connect("localhost","user","pass","database");

if (mysqli_connect_errno()) {
  mysqli_connect_error();
  exit();
}

echo "<a href=./>Home</a><br>";

$query = mysqli_query($connect, "SELECT filename,dato,size,spoken,speaker,dialect,project,gender FROM pdb ORDER BY id LIMIT 1") or die (mysqli_connect_error());
$fetch_data = mysqli_fetch_array($query, MYSQLI_ASSOC);

foreach($fetch_data as $value) {
  $values = $_REQUEST[$value];
}
if (empty($_POST)) {
  echo "<form method='post' action='".$_SERVER['PHP_SELF']."'>";
  echo "<table border='1'>";

  foreach($fetch_data as $key => $value) {
    echo "<tr><td>" . htmlspecialchars($key) . "</td><td><input type='text' name='" . htmlspecialchars($value) . "[]'></td></tr>";
 }
  echo "</table>";
  echo "<input type='submit' value='Insert'>";
  echo "<input type='button' onclick='history.go(-1)' value='Cancel'>";
  echo "</form>";
}
else {
  $query="INSERT INTO pdb (filename,dato,size,spoken,speaker,dialect,project,gender) VALUES ('$values[0]','$values[1]','$values[2]','$values[3]','$values[4]','$values[5]','$values[6]','$values[7]')";
  mysqli_query($connect,$query) or die (mysqli_error($connect)); 

  $affected_rows = mysqli_affected_rows($connect);
  echo "<strong>".$affected_rows."</strong> row inserted.";
}
mysqli_close($connect);
?>

[bradgrafelman]

I still don't understand what this:

$query = mysqli_query($connect, "SELECT filename,dato,size,spoken,speaker,dialect,project,gender FROM pdb ORDER BY id LIMIT 1") or die (mysqli_connect_error()); 
$fetch_data = mysqli_fetch_array($query, MYSQLI_ASSOC); 

foreach($fetch_data as $value) { 
  $values = $_REQUEST[$value]; 
} 

is supposed to be doing. Can you explain?

Also, you have done nothing (as far as I can see) to prevent SQL injection and/or SQL errors in regards to sanitizing user-supplied data.

[chrisdee]

I'll try to explain.

It should get the column names from my mysql table and echo this as description for the input values. I use $_REQUEST to make the input values globally accessable to the rest of the script. Meaning accessable to the last part of the script (else part).

The point is to take the user input values and put these into the corresponing mysql table fields. But I don't know if Im doing this correctly.
It seems to work.

[bradgrafelman]

That's definitely not what that code snippet is doing at all.

What it's actually doing is retrieving the first row in the result set of that SELECT query and attempting to use the values of each of the columns as indexes to the $_REQUEST array. So, for example, if you had a row in the DB that looked like:

('my_file.jpg', 'dato_value_here', 12345, 'spoken_value_here', 'speaker_value_here', ...)

then you're trying to access the array elements $REQUEST['my_file.jpg'], $REQUEST['dato_value_here'], $_REQUEST[12345], etc.

Not only does that not make any sense, but what you're doing with those nonsensical values doesn't make sense either. 🙂 This loop:

foreach($fetch_data as $value) { 
  $values = $_REQUEST[$value]; 
} 

is constantly overwriting the previous value of the variable $values before you ever do anything with it. Thus, $values will only contain data corresponding to the last value in the $fetch_data array (which means you might as well get rid of the pointless loop and just skip straight to using only that last value).