PHP IF statement within MSSQL call

Question

Ok, I've spent entirely too long trying to add an if statement within a TSQL query string. Any help would be appreciated. Here's the string with a syntax error.

$sql = "SELECT tblCasesLawyers.CaseID, tblCasesLawyers.PLAINTIFFLASTNAME + ', ' + tblCasesLawyers.PLAINTIFFFIRSTNAME AS PatientName, tblProcedures.ApplicationSubmitted, tblProcedures.CPTCode, tblProcedures.ProcedureDescription, tblCenters.CenterID, tblProcedures.ProcedureDate, tblProcedures.ProcedureStatus, tblProcedures.LeinAmount, tblProcedures.DatePaid
FROM (tblCasesLawyers INNER JOIN tblCenters ON tblCasesLawyers.Center_ID__C = tblCenters.CenterID) INNER JOIN tblProcedures ON tblCasesLawyers.CaseID = tblProcedures.CaseID
WHERE (((tblCenters.CenterID)={$_SESSION['center']}) AND (tblProcedures.ApplicationSubmitted >= 2012-05-01)".if !empty($_GET['search']) echo ('AND tblCasesLawyers.PLAINTIFFLASTNAME='.{$_GET['search']}).")
";

Thank you all for chiming in. I'm relatively new at this but attempted to prevent injection using the following:

    function ms_escape_string($data) {
    if ( !isset($data) or empty($data) ) return '';
    if ( is_numeric($data) ) return $data;

    $non_displayables = array(
        '/%0[0-8bcef]/',            // url encoded 00-08, 11, 12, 14, 15
        '/%1[0-9a-f]/',             // url encoded 16-31
        '/[\x00-\x08]/',            // 00-08
        '/\x0b/',                   // 11
        '/\x0c/',                   // 12
        '/[\x0e-\x1f]/'             // 14-31


    );
    foreach ( $non_displayables as $regex )
    $data = preg_replace( $regex, '', $data );
    $data = str_replace("'", "''", $data );
    return $data;
    }

    function sanitize($data){
    $data=trim($data);
    $data=htmlspecialchars($data);
    $data=ms_real_escape_string($data);
    return $data;
    }

    $search = sanitize($_GET['search']);

Can you give us the sql statement after its been created (print it) and what exactly is your error? — Kyra, Jun 11 '12 at 19:49
`if()` is not a function call and does not have a return value. You cannot use it in a string concatenation operation. echo will perform output, and cannot be concatted either — Marc B, Jun 11 '12 at 20:03

score 2 · Answer 1 · answered Jun 11 '12 at 19:55

2

What about an inline conditional?

<?php

$sql = "SELECT tblCasesLawyers.CaseID, tblCasesLawyers.PLAINTIFFLASTNAME + ', ' + tblCasesLawyers.PLAINTIFFFIRSTNAME AS PatientName, tblProcedures.ApplicationSubmitted, tblProcedures.CPTCode, tblProcedures.ProcedureDescription, tblCenters.CenterID, tblProcedures.ProcedureDate, tblProcedures.ProcedureStatus, tblProcedures.LeinAmount, tblProcedures.DatePaid
FROM (tblCasesLawyers INNER JOIN tblCenters ON tblCasesLawyers.Center_ID__C = tblCenters.CenterID) INNER JOIN tblProcedures ON tblCasesLawyers.CaseID = tblProcedures.CaseID
WHERE (((tblCenters.CenterID)={$_SESSION['center']}) AND (tblProcedures.ApplicationSubmitted >= 2012-05-01)"
.( (!empty($_GET['search'])) ? ' AND tblCasesLawyers.PLAINTIFFLASTNAME='.$_GET['search'] : '').")";

?>

answered Jun 11 '12 at 19:55

ERJ

166
5

3

@JeremyHolovacs - I was just mimicking the OP's code making it as simple as possible. Besides for all you know he might already have filtered the `$_GET` variable:) – ERJ Jun 11 '12 at 20:03
`$_GET['search']` is not filtered. If you protect against SQL injection I will upvote your answer. – Jeremy Holovacs Jun 11 '12 at 20:10
2

@JeremyHolovacs You know this? Last time I checked, `$_GET` was not readonly, and in fact I have often pre-filtered it. If the OP has already done something of that sort, "re-cleansing" it could result in double-escaping, which could be very bad. Writing for scenarios you don't know about is generally bad. This is a reasonable answer based on the limited information the OP revealed. – Unsigned Jun 11 '12 at 20:14
@Unsigned, it's a bad answer until it's safe from SQL Injection. – Jeremy Holovacs Jun 11 '12 at 20:21
1

@JeremyHolovacs ~ Without knowing specifics about the OP's application, tacking on unrequested code could even result in a *wrong* answer. Both aleroot and ERJ's answers are excellent drop-in replacements for the OP's snippet. The OP did not ask about injection, the OP asked what amounts to a syntax question. Both answers correct the syntax without introducing any new possible sources of error. – Unsigned Jun 11 '12 at 20:33
@Unsigned: There's a fair amount of disagreement with my downvotes, but my viewpoint is that some n00b will come along with a similar problem, google the question, and basically post the accepted answer into his or her app... and it will work. In my opinion, these answers are misleading and dangerous in what they do *not* address. – Jeremy Holovacs Jun 11 '12 at 20:45

score 2 · Answer 2 · edited May 23 '17 at 11:49

2

I don't think it is syntactically legal to concatenate an if statement like that. It doesn't evaluate to anything that the operator can undestand. Use a ternary or chop your statement into three pieces (after writing a cleanse method based on this answer:

$cleanCenterID = cleanse($_SESSION['center']);
$clentSearch = cleanse($_GET['search']);


$sql = "SELECT tblCasesLawyers.CaseID, tblCasesLawyers.PLAINTIFFLASTNAME + ', ' +tblCasesLawyers.PLAINTIFFFIRSTNAME AS PatientName, tblProcedures.ApplicationSubmitted, tblProcedures.CPTCode, tblProcedures.ProcedureDescription, tblCenters.CenterID, tblProcedures.ProcedureDate, tblProcedures.ProcedureStatus, tblProcedures.LeinAmount, tblProcedures.DatePaid
FROM (tblCasesLawyers INNER JOIN tblCenters ON tblCasesLawyers.Center_ID__C = tblCenters.CenterID) INNER JOIN tblProcedures ON tblCasesLawyers.CaseID = tblProcedures.CaseID
WHERE (((tblCenters.CenterID)={$cleanCenterID}) AND (tblProcedures.ApplicationSubmitted >= 2012-05-01)";

if(!empty($cleanSearch)) {
    $sql .= 'AND tblCasesLawyers.PLAINTIFFLASTNAME='.{$cleanSearch});
}

$sql .= ")";

edited May 23 '17 at 11:49

Community

1
1

answered Jun 11 '12 at 19:56

bluevector

3,485
1
15
18

1

@JeremyHolovacs Not any more so that the original. I was merely making a point about syntax, not solving all the world's ills. – bluevector Jun 11 '12 at 19:59
writing an answer that might end up in production with a dangerous security flaw is a bad answer (you're not alone, though.) Sanitize your inputs and I will remove my downvote. – Jeremy Holovacs Jun 11 '12 at 20:02
Copying and pasting code from a answer into production is is worse. – bluevector Jun 11 '12 at 20:03
3

I will upvote every question you downvoted, because your downvote is simply wrong ... – aleroot Jun 11 '12 at 20:03
1

@jonnyGold, I understand your point, however SO is supposed to be a place where people can come and get reliable answers to their coding questions. It is not only for the one asking the question. In this sense, it should not contain any errors. If you are reading a book called `MySQL for dummies`, you wouldn't want it to be full of SQL injection vulnerabilities. In the same way, when someone is reading a resource on SO, they also don't want examples full of security flaws. So I agree with Jeremy on this one. – Mike Jun 11 '12 at 20:03
@jonnyGold, I will even upvote if you fix your answer to protect against sql injection. – Jeremy Holovacs Jun 11 '12 at 20:07
@aleroot, I would expect better of you with a rep like yours. How difficult is it to write an extra line of code that makes the answer something that both works *and* is safe for actual use? – Jeremy Holovacs Jun 11 '12 at 20:08
@JeremyHolovacs While I still don't think solving non-question-specific problems is warranted, I believe that I have cleansed the code in question properly while teaching Zach to fish w.r.t. cleansing inpute. – bluevector Jun 11 '12 at 20:09
What is `cleanse()`? Why not just use `mysql_real_escape_string()`? – Mike Jun 11 '12 at 20:11
2

@JeremyHolovacs downvote every my answer if you think my reputation is too high ;-), i don't change my mind, this is the answer that let the OP understand where him have made the mistake ... – aleroot Jun 11 '12 at 20:12
@aleroot, this isn't a pissing contest, I'm not trying to throw my insignificant weight around. Giving a SQL answer that is inherently vulnerable to SQL injection is a bad answer. – Jeremy Holovacs Jun 11 '12 at 20:15
Keep your opinion, i have only answered a question without going into deep and outside the context ... – aleroot Jun 11 '12 at 20:19

score 1 · Answer 3 · answered Jun 11 '12 at 19:54

1

You could use an inline "if" to insert your conditional statement into the string, i.e.

$sql = 'Your sql '.(!empty($_GET['search']) ? 'some sql' : 'some other sql').' the rest of your sql';

answered Jun 11 '12 at 19:54

ezekielDFM

1,947
13
26

score 1 · Accepted Answer · edited May 23 '17 at 12:21

1

I think that you need a single line if here :

 "AND (tblProcedures.ApplicationSubmitted >= 2012-05-01)".
((!empty($_GET['search']) ? 'AND tblCasesLawyers.PLAINTIFFLASTNAME=' . $_GET['search'] : '' ). 
")

As stated in some comments concatenating a query in this way could lead to SQL Injection, to avoid SQL injection you can use prepared statements and parameterized queries, see this question to know the best way to avoid SQL injection ;-) .

Thanks

Went with

$sql = "SELECT tblCasesLawyers.CaseID, tblCasesLawyers.PLAINTIFFLASTNAME + ', ' + tblCasesLawyers.PLAINTIFFFIRSTNAME AS PatientName, tblProcedures.ApplicationSubmitted, tblProcedures.CPTCode, tblProcedures.ProcedureDescription, tblCenters.CenterID, tblProcedures.ProcedureDate, tblProcedures.ProcedureStatus, tblProcedures.LeinAmount, tblProcedures.DatePaid
FROM (tblCasesLawyers INNER JOIN tblCenters ON tblCasesLawyers.Center_ID__C = tblCenters.CenterID) INNER JOIN tblProcedures ON tblCasesLawyers.CaseID = tblProcedures.CaseID
WHERE (((tblCenters.CenterID)={$_SESSION['center']}) AND (tblProcedures.ApplicationSubmitted >= 2012-05-01)".( (!empty($search)) ? " AND tblCasesLawyers.PLAINTIFFLASTNAME='". $search . "'" : '').")";

edited May 23 '17 at 12:21

Community

1
1

answered Jun 11 '12 at 19:55

aleroot

71,077
30
176
213

1

I have responded to the question, i know that could lead to SQL injection, but i gave a precise response to a precise question ! Sorry professor ... – aleroot Jun 11 '12 at 20:00
you've been around a while here, you know that your answer could end up in production code somewhere. Please fix your answer. – Jeremy Holovacs Jun 11 '12 at 20:04
No, the correct answer is that i cannot concatenate a string in the way he did, the single line if have a different synthax. I given to the OP the best answer to let him understand his error, SQL Injection is another story ... – aleroot Jun 11 '12 at 20:07
Added a link to another question where the OP can learn how to prevent SQL Injection... – aleroot Jun 11 '12 at 20:45
You are a little bit pedantic :-) but you are right, i have to admit :D – aleroot Jun 11 '12 at 20:51
I have been called worse, and just as accurately :). – Jeremy Holovacs Jun 11 '12 at 20:57

score 0 · Answer 5 · answered Jun 11 '12 at 19:53

Would it work if you replace your code:

.if !empty($_GET['search']) echo ('AND tblCasesLawyers.PLAINTIFFLASTNAME='.{$_GET['search']}).")

with this:

'AND (  tblCasesLawyers.PLAINTIFFLASTNAME='.{$_GET['search']}.' OR len('.{$_GET['search']}.') = 0 OR '.{$_GET['search']}.' is null)'

PHP IF statement within MSSQL call

5 Answers5