0

I have 4 tables in database and an unique clientid is common in all tables but rest field are different. if we search for any client id , how can we get the information stored corresponding to the searched client id from the any table.

$clientid=$_POST['client'];
$query = "SELECT * FROM 'pfs'
          JOIN 'pfssurety' 
          JOIN 'iso'
          JOIN 'incometax'
          WHERE clientid='$clientid'";
$result = mysql_query($query)or die (mysql_error());

while($row=mysql_fetch_array($result)) {
    echo $row['clientid'];
    echo $row['name'];
}
J-Alex
  • 6,881
  • 10
  • 46
  • 64
  • You miss the `on` clause – Jens May 26 '17 at 12:22
  • 1
    **Stop** using deprecated `mysql_*` API. Use `mysqli_*` or `PDO` – Jens May 26 '17 at 12:22
  • 1
    This will not work cause when you are using JOIN you need to use the ON clause and the mysql_query , mysql_fetch_array() will not work you need to use the mysqli_.... – Omis Brown May 26 '17 at 12:23
  • Your code is vulnerable to **SQL Injection**, check my answer for a simple example on how to prevent this. Additionally read about what SQL Injection is and how to prevent it. – lloiacono May 26 '17 at 12:38

5 Answers5

1

Here is an implementation using mysqli and prevents injection for your $clientid where the id is a number grate then zero as AUTO_INCREMENT columns will never have a 0 value they start at 1

// as this is an int using inval will force it to be a valid whole number 
// basic SQL Injection Protection for a fixed id

$clientid = intval($_POST['client']);
if($clientid === 0){
    // it was not a valid number as an auto_increment field in mysql can never be 0
    die("invalid client");
}

$query="SELECT * FROM `pfs` 
    JOIN `pfssurety`  ON  pfssurety.clientid = pfs.clientid 
    JOIN `iso` ON  iso.clientid = pfs.clientid 
    JOIN `incometax` ON  incometax.clientid = pfs.clientid 
    WHERE pfs.clientid=$clientid";

$result= mysqi_query($query) or die("Query Failed");

while($row=mysql_fetch_array($result))
{
    echo $row['clientid'];
    echo $row['name'];
}
Barkermn01
  • 6,781
  • 33
  • 83
  • How is this preventing SQL Injection? You are still using the user input directly into the query, You should use prepared statements. – lloiacono May 26 '17 at 12:39
  • no im not `$clientid = intval($_POST['client']);` intval will turn it into an int no matter what so if it has any none number chars in it it will result in `0` go learn php... – Barkermn01 May 26 '17 at 12:41
  • I missed the intval() part, sorry. Still IMO you should use prepared statements, since this is one of the main reasons why you would like to use them, read more about them here: https://www.w3schools.com/php/php_mysql_prepared_statements.asp – lloiacono May 26 '17 at 12:44
  • yeah i know but i thought that might be a bit to complex since the OP is using `mysql_` instead of `mysqli_` let alone `mysqli::`(objective) or `PDO` before trying to work out why his `$_POST['clientid']` would bugger up coz of array value not passed by reference – Barkermn01 May 26 '17 at 12:52
0

You meant to use backtique instead of single quote. Otherwise your table names are considered as normal string literal. Also, you are missing ON condition for all your JOIN clauses So your query

SELECT * FROM 'pfs'

Should actually be

SELECT * FROM `pfs`

Change your query to be

SELECT * FROM `pfs`
JOIN `pfssurety` ON condition
JOIN `iso` ON condition
JOIN `incometax` ON condition
WHERE clientid='$clientid'
Rahul
  • 76,197
  • 13
  • 71
  • 125
0

You are on the right track using JOIN. You need to specify the common column that the JOIN should be made on.

https://www.w3schools.com/sql/sql_join_left.asp

SELECT a.fieldname, i.fieldname, t.fieldname, p.fieldname FROM 'pfs' as a
LEFT JOIN 'pfssurety' as p ON p.clientid  = a.clientid 
LEFT JOIN 'iso' as i ON i.clientid  = a.clientid 
LFT JOIN 'incometax' as t ON t.clientid  = a.clientid 
WHERE a.clientid='$clientid'";

Also, you should escape your variables to prevent SQL Injection. At a minimum:

a.clientid= "' . mysqli_real_escape_string($clientid) . '"';

http://php.net/manual/en/mysqli.real-escape-string.php

Blueline
  • 388
  • 1
  • 10
  • why did you pic left join he might want nulls https://stackoverflow.com/questions/38549/what-is-the-difference-between-inner-join-and-outer-join – Barkermn01 May 26 '17 at 12:27
  • Have added a one liner on injection, I swear every sql question has unescaped variables. – Blueline May 26 '17 at 12:32
  • Regarding the joins good point. LEFT JOIN is a habit for me for most joins. – Blueline May 26 '17 at 12:33
0

Maybe you need to set the on in your join as something like this:

SELECT * FROM 'pfs'
JOIN 'pfssurety'  ON pfs.clientid=pfssurety.clientid
JOIN 'iso' ON pfs.clientid=iso.clientid
JOIN 'incometax' ON pfs.clientid=incometax.clientid
WHERE clientid='$clientid'

Y suposed that all tables have the clientid attribute.

  • Instead of using the SELECT * you could use the enumeration of the different attributes you need
nacho
  • 5,280
  • 2
  • 25
  • 34
0

Your code is vulnerable to SQL Injection, you should never use user input directly into your SQL queries. In your code the problem is here:

$clientid = $_POST['client']; // anyone could manipulate this field to inject malicious code
# ...
WHERE clientid='$clientid'";

Check what happens if the value for $_POST['client'] is: ' or 1 = 1;

Next as mentioned in one of the comments stop using deprecated methods, instead for example you can use mysqli. Here is an example of how to use mysqli with prepared statements to avoid SQL Injection:

$conn = new mysqli($servername, $username, $password, $dbname);
// Check connection
if ($conn->connect_error) {
    die("Connection failed: " . $conn->connect_error);
} 

$stmt = $conn->prepare('SELECT * FROM `pfs` JOIN `pfssurety` ON condition JOIN `iso` ON condition JOIN `incometax` ON condition WHERE clientid = ?');

$stmt->bind_param('i', $clientid);
$stmt->execute();
$stmt->close();
$conn->close();

A prepared statement is a feature used to execute the same (or similar) SQL statements repeatedly with high efficiency.

Compared to executing SQL statements directly, prepared statements have three main advantages:

  • Prepared statements reduces parsing time as the preparation on the query is done only once (although the statement is executed multiple times)

  • Bound parameters minimize bandwidth to the server as you need send only the parameters each time, and not the whole query

  • Prepared statements are very useful against SQL injections, because parameter values, which are transmitted later using a different protocol, need not be correctly escaped. If the original statement template is not derived from external input, SQL injection cannot occur.

Finally one more thing worth mentioning, try not using * to fetch all columns, instead simply list the columns you need to get. Even if you need to get all columns there are good reasons why not to use *, but instead list all columns.

lloiacono
  • 4,714
  • 2
  • 30
  • 46