You have an error in your SQL syntax;

[trezremay]

Hi, I keep getting thrown this error, ‘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 ‘‘users’WHERE’emails’=‘[email protected]’AND’password’=‘test’’ at line 1’ and I cant seem to find or devise a solution, the entire code this applies to is:

if($_SERVER[“REQUEST_METHOD”] == “POST”); {
// username and password sent from form
if(isset($_POST[‘email’])){
$email = mysqli_real_escape_string($dbhandle,$_POST[‘email’]);
}
if(isset($_POST[‘password’])){
$mypassword = mysqli_real_escape_string($dbhandle,$_POST[‘password’]);
}

  //sql match if password and email are the same
  
  $br = "SELECT'accountID'FROM'users'WHERE'emails'='".$email."'AND'password'='".$mypassword."'";
  $result = mysqli_query($dbhandle,$br) or die(mysqli_error($dbhandle));

  $row = mysqli_fetch_array($result, MYSQLI_ASSOC)or die(mysqli_error($dbhandle));
  
  $totNumRow=mysqli_num_rows($result)or die(mysqli_error($dbhandle));
  if($totNumRow >0)
  {
      echo("success");
      header("location: welcome.php");
  }
  else{
      echo("login failed");
  } 
}

Any help is appreciated, also, I know this is currently vulnerable to sql injection but i need to sort this so went to basics - Thanks in advance ;D

[benanamen]

Assoonasyoufixthebrokenspacebaronyourcomputeryourproblemshouldbesolved.

[trezremay]

Assoonasyoufixthebrokenspacebaronyourcomputeryourproblemshouldbesolved.

Thanks for the reply, I tried removing the spaces as a solution and just tried it like this:

$br = “SELECT ‘accountID’ FROM ‘users’ WHERE ‘emails’ = '”.$email."’ AND ‘password’ =’ “.$mypassword.”’";

but I’m still getting the same error - thanks ;D

[phdr]

Only literal string data values are enclosed by single-quotes. Database table and column names are not string data values, they are identifiers. If you saw something posted on the web that had database table and column names enclosed by something, they were back-ticks ``, not single-quotes ‘’. Back-ticks are only required around identifiers in certain circumstances (you can research in the mysql database documentation to find out when) and should be avoided unless absolutely necessary (KISS - Keep It Simple.)

Next, you should NOT put data values directly into the sql query statement, even if you have used the mysqli_real_escape_string() function, because if you haven’t set the character set of the database connection (which is rare in code found on the web) to match your database tables, this code is still open to sql special characters in the data breaking the sql query syntax and causing an error and allowing sql injection. You should instead use a prepared query, with place-holders in the sql query for any data value(s), then supply the data when you execute the query. This actually simplifies the sql query syntax, since you don’t put any single-quotes around the place-holders (KISS.)

You can research on the web or in the php documentation to find out more about using prepared-queries. However, the php mysqli extension is overly complicated and inconstant to use and if you can switch to use the simpler and more constant php PDO extension (KISS.)

Also, you have a bunch of logic in your code that isn’t doing what you think and should be removed and you are missing some needed logic -

1. Don’t use isset() unless needed (KISS.) Only check-box and radio-button form fields won’t be set if none were checked. All other form fields types will be set if the form has been submitted. By using isset() for these form field types, you end up hiding html errors and typo errors in the names.

2. You should validate all input data before using it. If either the email or password values empty, there’s no point in using them and wasting time and resources in running the sql query. You should also trim() all input data before validating it to detect if all white-space characters were entered. If you use a php array variable, $errors, to hold the validation error messages, you can also use that same variable as an error flag (KISS.) If the array is empty, there are no errors. If the array is not empty, there were errors.

3. Do not use - or die(mysqli_error($dbhandle)) for database statement error handling. All this does is give hackers information about your server when they deliberately trigger errors. You should instead use exceptions for handling database statement errors. All you need to do is enable exceptions for the php database extension you are using and let php catch the exception. Php will use its error_reporting/display_errors/log_errors setting to determine what will happen with the actual error information. You would then remove all the or die(…) statements (KISS.) BTW - the only place the or die(…) statement in your current code ‘works’ is for the query() statement. The or die(…) on the fetch and num_rows statement will cause the code to halt with no output when they are triggered, because the fetch and num_rows don’t throw msyqli_error() information.

4. You cannot successfully echo anything before a header() redirect and you need an exit/die statement after the header() redirect to stop the rest of the code from running. A header() statement doesn’t stop code execution. If the echo statement is there for debugging purposes, you should also temporarily comment out the header() redirect (if php’s output_buffering is ON, the header() redirect will ‘work’ and you won’t see the output from the echo statement.)

5. Your code needs to start a session and upon successful login, set a $_SESSION variable, with the user’s id. You would test if this $_SESSION variable isset() on any ‘protected’ page to detect if the user is logged in. Also, if the current user is already logged in, you should not display the login form or run the login form processing code.

6. Echo is not a function and doesn’t need () around the value being echoed. The () just clutter up your code (KISS.)

Lastly, you need to hash the passwords. See php’s password_hash() and password_verify() functions. You would use password_hash() when inserting/updating the stored password. In the login code, you would only use the email in the WHERE clause in the sql query, fetch any matching row, then use password_verify() to test if the entered password matches the hashed password value.

Most of these things will simplify the code (KISS.) Adding validation logic and a prepared query will replace existing unneeded code and add a few statements. Using password_verify() will require that you fetch the data from the query, but this will replace the num_rows() logic. If I/someone has time, they will post example code.

[phdr]

Also, just noticed you have a stray ; in the following line of code -
[php]if($_SERVER[“REQUEST_METHOD”] == “POST”); {[/php]

This is causing the if() statement to be ineffective and the form processing code is running every time the page gets requested, not just when a post method form has been submitted.

[phdr]

See the following minimal example code (doesn’t show db connection code, a complete html document, and doesn’t apply htmlentities() to content being output) -

[php]<?php
session_start();

if(isset($_SESSION[‘user_id’]))
{
// if the user is already logged in, don’t run the login form processing and form code
header(‘location:welcome.php’);
die;
}

/*
make database connection here…
this example code uses the php PDO extension, with the connection in - $pdo
the pdo connection code needs to set the error mode to exceptions, turn off emulated prepared queries, and set the default fetch mode to ASSOC
*/

$errors = []; // define an array to hold errors
$data = []; // define an array to hold a ‘working’ copy of the data being operated on

// form processing code
if($_SERVER[“REQUEST_METHOD”] == “POST”)
{
$data = array_map(‘trim’,$_POST); // get a trimmed copy of all 1st level (non-array) data

// validate the inputs
if($data['email'] == '')
{
	$errors['email'] = "Email is required.";
}
if($data['password'] == '')
{
	$errors['password'] = "Password is required.";
}

// if no errors, use the form data
if(empty($errors))
{
	$sql = "SELECT accountID, password FROM users WHERE emails=?";
	$stmt = $pdo->prepare($sql);
	$stmt->execute([$data['email']]);

	if(!$row = $stmt->fetch())
	{
		// email didn't match
		$errors['login'] = "Login failed.";
	}
	else
	{
		// email matched, test password
		if(!password_verify($data['password'],$row['password']))
		{
			// password didn't match
			$errors['login'] = "Login failed.";
		}
		else
		{
			// password matched
			$_SESSION['user_id'] = $row['accountID'];
			header('location: welcome.php');
			die;
		}
	}
}

}

/*
as part of the html document on this page, display any validation errors, in $errors,
and populate the from field values with any existing values in $data.
any content you output to the browser needs to be passed through htmlentities(), not shown in this example code.
*/

// display any errors
if(!empty($errors))
{
echo implode(’
’,$errors);
}

// output the form, with any field values populated from existing values
?>

Email: '> Password: '> [/php]