Why This INSERT Doesn't Work...?

    $link = new mysqli($host, $user, $pass, $db);

    $i_mjesto = $_POST['mjesto'];
    $i_restoran = $_POST['restoran'];
    $i_radno_vrijeme = $_POST['radno_vrijeme'];
    $i_adresa = $_POST['adresa'];
    $i_mobitel = $_POST['broj_mobitela'];
    $i_telefon = $_POST['broj_telefona'];
    $i_email = $_POST['email'];
    $i_web_adresa = $_POST['web_adresa_restorana'];
    $i_broj_stolova = $_POST['broj_stolova'];
    $i_kratki_opis = $_POST['kratki_opis_restorana'];
    $i_dugi_opis_restorana = $_POST['opis_restorana'];
    $i_grill = $_POST['grill'];
    if($i_grill=='ima_grill'){ $i_grill=1; }else{ $i_grill=0;}
    $i_terasa = $_POST['terasa'];
if($i_terasa=='ima_terasu'){ $i_terasa=1;} else{ $i_terasa=0;}
$i_kategorija=$_POST['odabir'];
if($i_kategorija=='Jeftin')$i_kategorija=1;
if($i_kategorija=='Prosjecan')$i_kategorija=2;
if($i_kategorija=='Peka')$i_kategorija=3;
if($i_kategorija=='Ekskluzivni')$i_kategorija=4;

    $query = "INSERT INTO `RESTORAN` (B_MJESTO, B_RESTORAN, ADRESA, KATEGORIJA, BROJ_STOLOVA, MOBITEL, TELEFON, EMAIL, WEBADRESA) values ('$i_mjesto', '$i_restoran' , '$i_adresa', '$i_kategorija','$i_broj_stolova', '$i_mobitel', '$i_telefon', '$i_email', '$i_web_adresa');";
$result=$link->query($query);

Hi @robertbralic007 and welcome to the SP Forums.

I find with queries it is helpful if PHP HEREDDOC is used to display the query:

Try this:

// BEWARE: ONLY LINEFEED AFTER ____TMP
$query = <<< ____TMP
    INSERT INTO 
      `RESTORAN` 
      (
        `B_MJESTO`, 
        `B_RESTORAN`, 
        `ADRESA`, 
        `KATEGORIJA`, 
        `BROJ_STOLOVA`, 
        `MOBITEL`, 
        `TELEFON`, 
        `EMAIL`, 
        `WEBADRESA`
       )
     VALUES 
     (
      '$i_mjesto', 
      '$i_restoran' , 
      '$i_adresa', 
      '$i_kategorija',
      '$i_broj_stolova', 
      '$i_mobitel', 
      '$i_telefon', 
      '$i_email', 
      '$i_web_adresa'
     );
____TMP;
// BEWARE: MUST NOT HAVE PRECEDING SPACE and ONLY LINEFEED AFTER ____TMP; 
  echo $query;    

  $result = $link->query($query);
  if(mysqli_errno($result)):
      echo sprintf(
        "<br>Connect failed: %s\n", 
        mysqli_error($result)
      );
      exit();
  endif;

Also copying and pasting the query into PHP PhpMyAdmin is good for finding errors and warnings.

1 Like

OMG this is so open to SQL-Injections, everybody can delete your entire database. Read:

http://php.net/manual/en/security.database.sql-injection.php

then start using Prepared Statements, there are a lot of tutorials to read about, e.g.:

http://php.net/manual/en/mysqli.quickstart.prepared-statements.php

then set your database to report errors:

https://phpdelusions.net/mysqli/error_reporting
http://php.net/manual/en/mysqli-driver.report-mode.php

1 Like

Can you expand on “doesn’t work”? Does it give you an error message, or does it run but not insert the values that you expect? Is it possible that any of your fields contain a single-quote character ', which would ruin the query string? Using prepared statements as @chorn said above would eliminate this particular problem.

While I cannot see the HTML for your form, this code:

$i_kategorija=$_POST['odabir'];
if($i_kategorija=='Jeftin')$i_kategorija=1;
if($i_kategorija=='Prosjecan')$i_kategorija=2;
if($i_kategorija=='Peka')$i_kategorija=3;
if($i_kategorija=='Ekskluzivni')$i_kategorija=4;

suggests that your <select> tag could be improved using the value parameter on each option, rather than passing through the option display text and then translating it into a number when it is received. In your code, what happens if the selection does not match any of the strings you check for? It stays as whatever selection the user chose and potentially tries to insert a string into a field designed for a number.

NEVER EVER PUT VARIABLES IN YOUR QUERY!

You need to use Prepared Statements. DO not create variables for nothing.

2 Likes

Hi John. You can do the same exact layout without invoking HEREDOC. While we are here I would like to present something I don’t see any of you doing. It is the “Comma First” format. The benefit should be glaringly obvious so I won’t comment.

$query = 'INSERT INTO
      `RESTORAN`
      (
          `B_MJESTO`
        , `B_RESTORAN`
        , `ADRESA`
        , `KATEGORIJA`
        , `BROJ_STOLOVA`
        , `MOBITEL`
        , `TELEFON`
        , `EMAIL`
        , `WEBADRESA`
       )
     VALUES
     (
        ?
      , ?
      , ?
      , ?
      , ?
      , ?
      , ?
      , ?
      , ?
     )
';
3 Likes

On the DataBase forum quite a few knowledgeable users use the Comma First option.

The benefit should be glaringly obvious so I won’t comment.

I will… I am a big fan and have been using HereDoc for quite some time and really like the lack of extraneous echos, quotes, periods, trailing semi-colons, etc. It is far easier to implement, more readable, far less typing, reduces file size,less CPU processing,… have I missed anything? :slight_smile:

Edit:
I forgot to mention that it is also very easy to use variables, arrays and pointers within HereDoc although the latter two must be enclosed in Curly Braces.

Only values not able to use are Contants/Defines which can be given temporary dollar prefixed variables and then used without any problems. Similar to functions such as date('H:i:s');

2 Likes

My “won’t comment” was referring specifically to the comma first example.

My HEREDOC comment was specifically for the SQL example you posted not HEREDOC in any other case so your arguments don’t apply to my response. In the current example there are 17 HEREDOC characters for absolutely no benefit compared to two characters (two single quotes). I am ignoring that variables were used in the query since that is wrong anyways.

1 Like

I once was at a conference where I heard two people talking regarding single quote vs double quoted strings and one of them said they would love to debate that problem, because if that is what you’re concerned about it means all other (more serious) performance problems in the application you are working on must be solved!

4 Likes

Not sure if you are referring to my post, but just in case, it had absolutely nothing to do with single or double quote’s. It was regarding using the posted HEREDOC SQL example vs just using quotes. I thought the thread reads pretty clear but perhaps not.

I was not

1 Like

Maybe it’s a good idea first to help OP solving the problem and then pointing OP how things can be done better?

3 Likes

@robertbralic007

I agrree with John. It always helped me a lot running queries in PhpMyAdmin. Sometimes I didn’t understand why certain queries didn’t return any results. When running them in PhpMyAdmin it appeared it was an error in the query. Try that first

1 Like

@John_Betong. Agree. And not only on the database forum. I prefer the comma first option, ever since I’m working in this business.

1 Like

OPs main problem on long term will be the SQL injection - even if he did not ask for it, maybe because he did not know until now. So i do not see why i should delay my response to a time where he may not even recognise any answers anymore, because what he thought to be his main problem was fixed and he’s leaving the thread. And i think people can handle more than one problem at a time, or start looking at the more critical ones.

2 Likes

A clear sign of having the database library’s error reporting disabled (e.g. that’s the default for PDO) or missing error handling.

@Dormilich
Hahaha, that was indeed the case, long time ago when just starting (not using PDO then but mysql) and completely new to everything, not knowing anything about error handling. So running a query in phpAdmin helped me a lot in that period. I think that is what @John_Betong ment when giving that suggestion

1 Like

@chorn. At long term you get a resounding yes and yes one would expect that someone could handle more then one problem at a time. But OP, when asking this question just signed up to sitepoint and just wanted to know why an insert didn’t work (probably just started, first website?). Do you realy expect that OP thought, when seeing you answer, ok first I have a look at what SQL injection means, later on I try to solve the insert? He just wanted to solve his problem, which i think is very normal. I speak now for myself but if I get nice help on a site I am more inclined to go back to that forum than when I am overwhelmed with terms that I do not understand

2 Likes

Yes i do, because that’s what i would do. When presented with a serious security issue that i’ve not known before, that harms a major piece of my software - i would stick to get everything to know about the vulnerability and try to fix it immediatly. But still i don’t see why i would have mentioned that later (when exactly?). I don’t start guessing what he could have meant in any way, i’m just relying on what he posted. Not telling people about their security vulnerability is a lack of personality, as it is to just ignore them - just my view on people. OP’s free to do whatever he want with this information, even ignoring it, but getting tips you didn’t ask for is the natural behavior on a public board (i always held this as an advantage) that one can knock out by just paying someone for a blind anwser - there are enough people out there just getting things done without looking left and right. And i’ve seen this so often on Stackoverflow that i skiped participating there, no need for PLZ EMAIL ME TEH CODEZ.

4 Likes

In my opinion, I often see this happen a lot especially with beginners. Whether you are a beginner, intermediate, or professional, security is a big thing in the IT field. If you blatantly ignore security then you are pretty much ignoring IT altogether. The situation may not be what you were looking for to begin with, but if your code does pose a security risk, I would say that it’s sufficient enough to point it out.

People seem to behave about security as if it’s some kind of chore. We shouldn’t be thinking or acting this way when it comes to IT. We aren’t just the only people out there that know our codes. Hackers know our codes. Do you think a hacker cares if you’re a beginner who just signed up to a random website on the internet to ask a question? Do you think they’re going to go easy on you just because you’re a beginner? We have to start thinking like a hacker in order to combat hacking attempts.

Whether this is some kind of homework assignment or some kind of job test or even a hobby project, I think everyone should start moving forward with technology and put security at the top of your list. Not moving backwards and putting security at the bottom of your list. That’s just my 2 cents. Whether you want to ignore it or take action is up to you. Just note that it takes a lot of effort into making an application secure, but it takes 1 mistake to break it.

6 Likes