-1

I am getting the following error:

Update 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 'ContactOfficeNo='20045878' WHERE ClientCode='MARI'' at line 2

Now I notice after ClientCode='Mari', there is another '?

But my code is as below. I can't seem to find the issue. I've got opening & closing "" with the update query inside as it should be:

include("../dbinfo.inc.php");
$comm=@mysql_connect(localhost,$username,$password);
$rs=@mysql_select_db($database) or die( "Unable to select database"); 

$id = $_POST["clientcode"];
$cli = $_POST["clientname"];
$em1 = $_POST["email1"];
$em2 = $_POST["email2"];
$em3 = $_POST["email3"];
$cry = $_POST["currency"];
$occ = $_POST["othercc"];
$mob = $_POST["mobile"];
$dty = $_POST["dtymobile"];
$off = $_POST["office"];

$sql = "UPDATE tbl_client SET ClientName='$cli', ContactEmail#1='$em1', ContactEmail#2='$em2', ContactEmail#3='$em3', Currency='$cry', OtherCCinfo='$occ', ContactMobileNo='$mob', DutyMobileNo='$dty', 
ContactOfficeNo='$off' WHERE ClientCode='$id'";
Mark Baker
  • 209,507
  • 32
  • 346
  • 385
sanitizer
  • 95
  • 3
  • 14
  • 3
    [Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)***. Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Feb 13 '17 at 22:31
  • 4
    If you use parameterized queries, you won't have this problem. – Gordon Linoff Feb 13 '17 at 22:31
  • I have edited the code, this is parameterized is it not? The issue im having is strange to me, because i have the exact same format for another table, which works perfectly fine, yet im having this error – sanitizer Feb 13 '17 at 22:37
  • 1
    This is not parameterized queries. For that, you need to use MySQLi or PDO (which you really should switch to, `mysql_*` is old and insecure), and prepare your queries with the `prepare()` method and use placeholders in your query instead of directly injecting the variables. – Qirel Feb 13 '17 at 22:38
  • No, it's not a parameterized query. You can't do those using the old deprecated and insecure `mysql_*`-functions – M. Eriksson Feb 13 '17 at 22:39
  • `ContactEmail#1`?!? You really have a `#` in your column names? `Permitted characters in unquoted identifiers: ASCII: [0-9,a-z,A-Z$_] (basic Latin letters, digits 0-9, dollar, underscore) Extended: U+0080 .. U+FFFF` If you have other characters, then you need to wrap the column name in backticks (`\``) – Mark Baker Feb 13 '17 at 22:39
  • @MarkBaker I thought the same thing on first impression but if the error message refers to a later column in the SQL I can only assume that `#` are infact acceptable in this particular breed of setup..... else the error would relate to earlier columns, any of them..... (?) – Martin Feb 13 '17 at 22:40
  • 1
    @Martin [MySQL Docs reference](https://dev.mysql.com/doc/refman/5.7/en/identifiers.html)... though you're right that the error message isn't highlighting this directly – Mark Baker Feb 13 '17 at 22:41
  • @MarkBaker I would have expected an error feedback for column `ContactEmail#1` rather than the later column. I do realise that you are correct, it's just odd that this issue is somehow not the one the OP is writing about... – Martin Feb 13 '17 at 22:44
  • I've placed the code on a single line, and the error being outputted now is **right syntax to use near '' at line 1** – sanitizer Feb 13 '17 at 22:44
  • @CS88 read my answer, do what is outlined in my answer, then edit your question with the additional details from point 1 and 2, or magically discover your question solved by following points 3-7. `:-)` – Martin Feb 13 '17 at 22:45
  • @MarkBaker I've referenced your comment and your doc reference to my answer. – Martin Feb 13 '17 at 22:46
  • There is no need to shout, @CS88. Would you replace the title with the sentence case equivalent please? – halfer Feb 13 '17 at 22:47
  • @MarkBaker you were right, Changed the cols in phpmyadmin to remove the '#' from each of the emails and its appeared to do the trick.. Not sure as to why the error was pointing to the " at the end of the query.. – sanitizer Feb 13 '17 at 22:49
  • Potentially the newline in your code; `#` starts comment in an SQL statement until the end of the current line... if that is treated as a comment, it will read as `UPDATE tbl_client SET ClientName='$cli', ContactEmail ContactOfficeNo='$off' WHERE ClientCode='$id'`, which is invalid SQL, and would highlight the point of error that you got – Mark Baker Feb 13 '17 at 23:10
  • 1
    However, editing your question has now made it meaningless and unhelpful to anybody else that might encounter a similar problem... I've rolled it back again so that it may help others – Mark Baker Feb 13 '17 at 23:12
  • Great, thanks Mark – sanitizer Feb 14 '17 at 17:50

1 Answers1

2

what is the value of $dty ? It is useful in these moments to remember two tips:

  • ) The error (almost) always occurs before the "near" point in the SQL error output notice, so the few characters BEFORE ContactOfficeNo='20045878 is whatever is causing your issue.

  • ) It is good practive to error log out your complete SQL before you run it to see exactly what's going on when the error occurs.

    error_log("My SQL: Line ".__LINE__." :". $sql );
    

Additionals:

1) As others comments have said you really, REALLY should be looking at using MySQLi or PDO Parameterised queries.

Little Bobby says your script is at risk for SQL Injection Attacks.. Even escaping the string is not safe!

-- Jay Blanchard

If you use parameterized queries, you won't have this problem.

-- Gordon Linoff

2) Stop using @ to suppress errors. You're causing yourself more problems than you're solving.

3) MySQL is DEPRECATED and should not be used. Use MySQLi (or PDO). NOW.

4) Please read and take heed of the comments by Gordon and Jay underneath your question, this is (probably) the cause of your issue, you're not escaping your $_POST input. This is extremely dangerous and will (is) causing you all sorts of issues and bumps.

5) Having non alphabetic characters (so not a-z) in your MySQL column name is risky at best, and may be causing you additional issues. Highly recommended to move away from having any non-a-z characters in your column names.

Please read the docs refrence by Mark Baker about suitable column names.

ContactEmail#1?!? You really have a # in your column names? Permitted characters in unquoted identifiers: ASCII: [0-9,a-z,A-Z$_] (basic Latin letters, digits 0-9, dollar, underscore) Extended: U+0080 .. U+FFFF If you have other characters, then you need to wrap the column name in backticks (`)

-- Mark Baker

Community
  • 1
  • 1
Martin
  • 22,212
  • 11
  • 70
  • 132
  • Answers are better if they incorporate what somewhat said, rather than asking the reader to read a comment elsewhere. The main reason for this is that comments may be deleted, which would make this answer harder to understand if that happens in this case. You can either quote someone verbatim (e.g. with a quote block) or you can just take the essence of what they said and merge it into your answer. – halfer Feb 13 '17 at 22:50
  • +10. And if there's some (unfathomable) reason that it's not possible to use a prepared statement with bind placeholders, then at a bare minimum, potentially unsafe values included in SQL text must be properly escaped. (Consider e.g. what happens when the value passed in for `clientcode` is "**`1' OR 1=1 --`** – spencer7593 Feb 13 '17 at 23:12