0

HTML

            <form action="inc/q/prof.php?pID=<?php echo $the_pID; ?>" method="post">            
            <select id="courseInfoDD" name="courseInfoDD" tabindex="1"><?php while($row3 = $sth3->fetch(PDO::FETCH_ASSOC)) {
                  echo "<option>".$row3['prefix']." ".$row3['code']."</option>"; }echo "</select>"; ?>
            <input type="text" id="addComment" name="addComment" tabindex="3" value="Enter comment" />
        <input type="hidden" name="pID" value="<?php echo $the_pID; ?>">
        <input type="submit" name="submit" id="submit" />
        </form>

PHP 

$connect = mysql_connect("##", $username, $password) or die ("Error , check your server connection.");
mysql_select_db("###");

//Get data in local variable
if(!empty($_POST['courseInfoDD']))
    $course_info=mysql_real_escape_string($_POST['courseInfoDD']);
if(!empty($_POST['addComment']))
    $course_info=mysql_real_escape_string($_POST['addComment']);
if(!empty($_POST['pID']))
    $the_pID=mysql_real_escape_string($_POST['pID']);

print_r($_POST);
echo $the_pID;

// check for null values
if (isset($_POST['submit'])) {
$query="INSERT INTO Comment (info, pID, cID) values('$the_comment','$the_pID','$course_info')";
mysql_query($query)  or die(mysql_error());
echo "Your message has been received";
}
else if(!isset($_POST['submit'])){echo "No blank entries";}
else{echo "Error!";}

?> ?>

Table

commId    int(11)
info      text
date      timestamp
reported  char(1)
degree    char(1)
pID       int(11)
cID       int(11)

It gives me "Error!" now, I try the db credentials and they are fine... ?? And the r_post() is still giving an error of Array()

Why isn't Array() accepting values? Anyone???

Jshee
  • 2,620
  • 6
  • 44
  • 60
  • You should check if the $_POST variables are actually set before checking their value. – Borgenk May 01 '11 at 21:44
  • I don't think it's a problem with the SQL, at least not so far. The error being returned is his own default message for when the inputs aren't set? – Dormouse May 01 '11 at 21:44

6 Answers6

3

Like @user551841 said, you will want to limit your possibility of sql injection with his code. You are seeing that error because you're code told it to echo that error if nothing was entered, which is the case upon first page load. You shouldn't need that until submit is done.

Edit: Sorry, I was assuming you are directly entering the page which needs the $_POST data without going through the form submit.

You also should do something along the lines of if(!isset($variable)) before trying to assign it to something less your server will spit out error of undefined variables.

if(!empty($_POST['courseInfoDD']))
    $course_info=mysql_real_escape_string($_POST['courseInfoDD']);

do that to all of them.

Then you can check

if (!isset($user_submitted) && !isset($the_comment) && !isset($course_info) && !isset($the_pID) ){
echo "All fields must be entered, hit back button and re-enter information";
}
else{
$query="INSERT INTO Comment (info, pID, cID) values('$the_comment','$the_pID','$course_info')";
mysql_query($query)  or die(mysql_error());
echo "Your message has been received";
}
robx
  • 3,093
  • 2
  • 26
  • 29
  • How do I implement an on-submit type function into my script? Can you please show me? – Jshee May 01 '11 at 22:12
  • give your submit button a name and check for that to be submitted is one simple way ;). If you go directly to that page, there will be nothing and no error to show, if you click the submit, then the submit button value will follow. – robx May 01 '11 at 22:15
  • Oh you already have one, you shouldn't really use the "or" but the double pipe (||). But I am betting that you should use && (and) rather than the "or" for your logic – robx May 01 '11 at 22:18
  • Please see updated code... still giving that error.. despite removing that if empty condition – Jshee May 01 '11 at 22:22
  • Please see updated, this still gives that un-desired error as stated above – Jshee May 01 '11 at 22:30
  • Try this to see if it will help – robx May 01 '11 at 22:33
  • It gives me "Error!" now, I try the db credentials and they are fine... ?? – Jshee May 01 '11 at 22:46
  • If I just ignore the error and try to submit it says 'No pID specified' now its getting that from the url, why does this go away upon submit? Is it possible to keep it at the end of the url like it is at the first page load? – Jshee May 01 '11 at 22:52
  • The error is your telling the code to say error, it has nothing to do with the db connection it seems. Why do you have assigning post values twice? I meant to replace your old ones so you don't have any undefined variables errors. And you have actually flip flopped the logic again. Also, what does your print_r($_POST) show? – robx May 01 '11 at 22:53
  • It goes away upon submit because when you submit,the data is being sent to the server, when you just enter the page without a submit, there is no data there. – robx May 01 '11 at 23:03
  • And since you changed the top portion, please also change the bottom portion of the logic to match and let me know. I think a var_dump($_POST) would be more sufficient in data dump than print_r in this case. The case that it shows array means that the POST data is 2d array. – robx May 01 '11 at 23:05
  • How do I get it to show, if and only if someone trys to submit a blank form (basically spam) ? – Jshee May 01 '11 at 23:07
  • This also does not fix the problem, when I try to submit - it says no pID specified. The url before submission might be "prof.php?pID=273" after submission it goes to prof.php . How do I retain this pID=[somenumber] ? – Jshee May 01 '11 at 23:10
  • if you want to get something from an url you have to use $_GET['variable'] instead of $_POST['var']. so in your case you need to check for $_GET['pID'] if that is you are after. But you have a form with a post method, so I don't understand where the url would contain pID=123. If your php variable "$the_pID" contained something, it should go along with the form post – robx May 01 '11 at 23:14
  • lets do this. check to make sure that your php variable $the_pID actually has something to send along with the form post. Even if it is not in the url, you can surely still use it since it is being sent via form post. – robx May 01 '11 at 23:18
  • This pID is in the inital page onload, but when submitted it clears it. I updated my code above to include the get pid, doesnt seem to do anything. – Jshee May 01 '11 at 23:19
  • that's your error. you need to do this before the form: if(!empty($_GET['pID'])) $the_pID = mysql_real_escape_string($_GET['pID']); – robx May 01 '11 at 23:28
  • Robx - I have added this above in the html, doesn't seem to do anything different as far as output – Jshee May 01 '11 at 23:38
  • if you have an url as such prof.php?pID=273, and that line doesn't give you anything when trying to print out $the_pID, something else is wrong. Maybe the problem is coming from your php.ini file where that feature is disabled. – robx May 01 '11 at 23:41
  • But the line above isnt outputting the pID, its just checking if its empty, correct? – Jshee May 01 '11 at 23:43
  • before the form, do this var_dump($_GET) and see if there's any data there. – robx May 01 '11 at 23:43
  • I thought you had a line somewhere to verify that something is in fact in that variable. You need to fetch it from the url, then output it to verify that it is getting what you need. – robx May 01 '11 at 23:44
  • Because if i try this in the html : " – Jshee May 01 '11 at 23:44
  • THe problem is, this pID seems to vaporize from the URL on submission – Jshee May 01 '11 at 23:45
  • if you want it there, then you'd have to change the form method to get rather than post, BUT, if you do that, everything else will be included in the url as well. – robx May 01 '11 at 23:46
  • How do I 'retain' this 273 value for $the_pID on submission ? – Jshee May 01 '11 at 23:47
  • Is it possible to put this $the_pID in a hidden field that can get POST'd too upon submission? – Jshee May 01 '11 at 23:48
  • you already have that in a hidden field in your form post which is why i said even if it was not in the url, you can surely still get and use the pID of 273 – robx May 01 '11 at 23:49
  • I have other queries in the same file that use this $pID3 = filter_input(INPUT_GET, 'pID', FILTER_SANITIZE_NUMBER_INT); for example, I feel like its trying to pull that as well for this query. Anyway to tell it to INPUT_POST instead? – Jshee May 01 '11 at 23:51
  • okay, form method="get" will send form data via the visible URL, form method="post" will send data via the back end and none visible. – robx May 01 '11 at 23:53
  • I understand that, but I dont want to expose all of fields to users lol. Just the pID . How do I do this??????????? – Jshee May 01 '11 at 23:54
  • Is it possible to use GET in a POST method to get one field (thus being the pID from the URL) ? – Jshee May 01 '11 at 23:55
  • must they see the ID? If you can get it via post, then just go that route. What other purpose for it to be in the url? – robx May 01 '11 at 23:57
  • That is correct, I can get the pID from POST, but it is still giving me the error that 'no pID specified'. I feel like its still trying to use the filter_input as specified a few comments above. Why is this and is there a problem to fix that? – Jshee May 02 '11 at 00:00
  • I guess if you insist try this:
    – robx May 02 '11 at 00:00
  • the 'no pID' sounds like the sql error. $query="INSERT INTO Comment (`info`, `pID`, `cID`) values('".$the_comment."',".$the_pID.",'".$course_info."')"; – robx May 02 '11 at 00:04
  • the 'no pID' comes ONLY when the inital form is submitted and the 'GET'ed value of pID=273 or some other INT is stripped from the URL since the pID is put there by GET in the first place – Jshee May 02 '11 at 00:05
  • Then your problem is not with the form, but checking errors at this point for more undefined variables. You should always check your variables if they are defined before trying to use them. You're pID variable must be undefined thus you should not need to run the line which is giving you that error. If you need it, you have to make sure pID is defined with some value. Whether you get it from the url or from post methods. Either way will work. – robx May 02 '11 at 00:12
  • Please see updated code Robx, it now submits, but it gives the output as an array??? And says its been submitted, but then nothing is added to the database :) lol. – Jshee May 02 '11 at 00:27
  • Heres a sample output: Array ( [courseInfoDD] => Info 110 [addComment] => Testing one comment [pID] => 273 [submit] => Submit ) 273Your message has been received – Jshee May 02 '11 at 00:28
  • It seems to be submitted now, but the info column is blank in the db. – Jshee May 02 '11 at 00:32
  • check your variables. They do not match up for the comment ;) – robx May 02 '11 at 00:34
0

Check that the hidden field "pID" has a value set from value=<?php echo $the_pID; ?>

Dormouse
  • 5,130
  • 1
  • 26
  • 42
  • I actually ran his code and it failed but now checking back it was a mistype on my part. Removing it now – Dormouse May 01 '11 at 21:51
  • This error about no pID specified comes from onsubmission the pID is lost in the url, see the post above. – Jshee May 01 '11 at 23:12
0

Make sure that your data is valid before checking it.

For instance do 

print_r($_POST);

and check if the keys and their data match up.

Also, as a side note, NEVER do what you're doing with :

$query="INSERT INTO Comment (info, pID, cID) values('$the_comment','$the_pID','$course_info')";

This is how mysql injection happens, either use prepared statements or

$course_info= mysql_real_escape_string($_POST['courseInfoDD']);
AlanFoster
  • 8,156
  • 5
  • 35
  • 52
0

To answer to your question what is wrong here
you've got a huge gaping SQL-injection hole!!

Change this code

//Get data in local variable
$course_info=$_POST['courseInfoDD'];
$the_comment=$_POST['addComment'];
$the_pID=$_POST['pID'];

To this

//Get data in local variable
$course_info = mysql_real_escape_string($_POST['courseInfoDD']);
$the_comment = mysql_real_escape_string($_POST['addComment']);
$the_pID = mysql_real_escape_string($_POST['pID']);

See: How does the SQL injection from the "Bobby Tables" XKCD comic work?

For more info on SQL-injection.

Community
  • 1
  • 1
Johan
  • 74,508
  • 24
  • 191
  • 319
  • I'm happy to see that. Did you know about this issue before? – Johan May 01 '11 at 22:29
  • I did know about it, but ive had so much trouble with this before just trying to get a lame comment into a db, that i would add that after this. Can you help with the above updated code? Its still giving me that error onload. – Jshee May 01 '11 at 22:31
  • @user, sorry don't see it now, but it's late here. BTW if you post SQL-injectable code on SO, you will only get comments on that. If you want sensible answers do not post SQL-injectable code, people are allergic to that and will find it hard to look past it. – Johan May 01 '11 at 22:36
  • Johan - everyone else seems to be trying to help, but i will try to remember that. – Jshee May 01 '11 at 22:43
0

i would change this line

if (isset($_POST['submit'])) {

to 

if ($_POST) {

the sumbit button field will not always be posted, for example if you press return on keyboard instead of clicking on the submit button with the mouse.

bumperbox
  • 10,166
  • 6
  • 43
  • 66
0

Cleaner:

$submit = isset($_POST['submit']) ? true : false;
$comment = isset($_POST['comment']) ? trim($_POST['comment']) : '';
if ($submit && $comment) {
  $query = 'INSERT INTO comments (comment) values("' . mysql_real_escape_string($comment) . '")';
  //...
}

As you can see I place the escaping inside the query. And this is a good idea because sometimes you loose track of the complete code and this won't happen inside a query.

mgutt
  • 5,867
  • 2
  • 50
  • 77