-1

I'm having a bit of trouble with some Myqsli query.

$sqlselect = "SELECT visitatori_id FROM visitatori WHERE   visitatori_nome='".$nome."' AND visitatori_cognome='".$cognome."' AND 
visitatori_orastart='".$time."' AND visitatori_idsede='".$idsede."'";   


if ($conn->query($sqlselect) === TRUE) 
{
    $resultz     = $conn->query($sqlselect);        
    $rowz        = $resultz->fetch_assoc();        
    $id          = $rowz["visitatori_id"];
    $data        = date('dmY');        
    $arr         = $id."/".$data.$idref.$idrep.$idsede;
    $JSON->value = $arr;
    $json        = json_encode($JSON);    
    echo $json;                      
    break;
}
else
{
    $JSON->value = $conn->error;
    $json = json_encode($JSON);    
    echo $json;
    break;
}

The query launched on the DB works with no problems at all. But if i try to launch it in the PHP code, the if returns false, and the $conn->error is just "".

I've searched a lot but I couldn't find anything that could at least show to me the errors.

EDIT

I know this code can be injected, but before the mysqli statements I've always done some tests with normal query, anyway thanks for the reminder.

Edoardo
  • 1
  • 2
  • 4
    Your code is vulnerable to [SQL injection attacks](https://en.wikipedia.org/wiki/SQL_injection). You should use prepared statements with bound parameters, via either the [mysqli](https://secure.php.net/manual/en/mysqli.prepare.php) or [PDO](https://secure.php.net/manual/en/pdo.prepared-statements.php) driver. [This post](https://stackoverflow.com/q/60174/6634591) has some good examples. – Luca Kiebel May 03 '18 at 16:07
  • 2
    ^ Not just SQL injection attacks, but also quoting headaches. Switch to prepared statements, and those headaches go away. – aynber May 03 '18 at 16:08
  • === checks for EXACT conditions... aka does your query return true... not necessarily if it returned NOT FALSE – Phillip Weber May 03 '18 at 16:08
  • you need to either check for !== false OR == true (2 equal signs) – Phillip Weber May 03 '18 at 16:09
  • @luca prepared statements work for sanitizing but I personally would not advise using them for single queries (in most cases)... they are used for a different, mostly unrelated purpose and are almost twice as slow on a single query. – Phillip Weber May 03 '18 at 16:31
  • 1
    Why would you not use a prepared statement? So long as there are variables in the query, prepared statement is better in every way to escaping them. – Qirel May 03 '18 at 16:34
  • @Qirel not always.. especially when speed is very important and you are doing a single query. and I think it's a bad/lazy habit to ALWAYS use them... but that's just my opinion. – Phillip Weber May 03 '18 at 16:39
  • @Qirel I've spent some time trying to emulate prepared statements: https://github.com/zpweber/MySQLiX – Phillip Weber May 03 '18 at 16:42
  • The double query was for desperation, because I've brought down several pantheons with my imprecations – Edoardo May 03 '18 at 17:01
  • 1
    @PhillipWeber The difference is really negligible. If you think you can write some code that's better than the folks over at PHP, they'll be thrilled - but really, prepared statements is the way to go, and its superior in every way to escaping the inputs. If you're so depending on speed, there are many other aspects you should consider before opimising that. – Qirel May 03 '18 at 18:13
  • @Qirel Respectfully, let's agree to disagree (whatever that means hehe).. It all depends on the application. The code I wrote isn't meant to replace prepared statements... it's meant to emulate the escaping part and avoid a redundant call to the db, which is not the main purpose of a prepared statement. – Phillip Weber May 03 '18 at 18:48

1 Answers1

-1

You're condition check needs to either check for "not false" or for a loose true (==)(mysqli_query returns mysqli_result object on successful select and false on failure: http://php.net/manual/en/mysqli.query.php).. and you are executing the same query twice when I'm sure you do not mean to. Also, as others have said, this code is prone to injection (see: http://php.net/manual/en/mysqli.real-escape-string.php, https://en.wikipedia.org/wiki/SQL_injection).

if( ($res = $link->query($queryString)) ){
    if( $res->num_rows > 0 ){
        $row = $res->fetch_assoc();
        /* Do something with result */
    }else{
        /* No results, do something else */
    }
}

and before this point... you should escape all necessary data to avoid injection attack:

$link->escape_string($aString);
Phillip Weber
  • 554
  • 3
  • 9