0

I'm using Wordpress and deleting posts from a MYSQL database via PHP using AJAX.

This works great.

The users on my site have the ability to "favourite" posts, this renders a list showing them what they have favourited (basically a separate table that cross references their user ID with the post ID's).

Now the issue is that when a post is deleted, I also need to delete any "favourited" references from the other table.

I'm doing this by running the following query as part of the delete post routine :

 $wpdb->query("DELETE FROM favourite_posts WHERE post_id = ".$_REQUEST['id']);

This works perfectly but if I add error checking :

 $wpdb->query("DELETE FROM favourite_posts WHERE post_id = ".$_REQUEST['id']) or die(mysql_error());

And the post is NOT in the favourites table the entire routine fails, obviously.

So my question is this...

Will running the first query (without error checking), cause problems / server overload etc when this routine is run on posts that don't also exist in the favourite_posts table? Or does MYSQL just ignore failed requests and carry on as normal?

Or is this bad practice and should I check if the post exists in favourite_posts before attempting to remove it? Obviously this means more queries which I'm hoping to avoid.

Grant
  • 1,297
  • 2
  • 16
  • 39
  • 2
    You also realize you're wide open to injection attacks, right? – Daedalus Oct 18 '15 at 09:17
  • 1
    A `DELETE` statement that finds no matching row to delete will _not_ throw an error! That is perfectly fine, since it is logical. An error would only get thrown in case of a syntax error in the sql statement (see above: sql injection vulnerability) or if the deletion would violate some constraints like foreign keys. – arkascha Oct 18 '15 at 09:17
  • @arkascha thank you, that's what I was hoping ;) – Grant Oct 18 '15 at 09:19
  • @Daedalus can you please explain a bit further? – Grant Oct 18 '15 at 09:19
  • Ok you've got me worried now! Can somebody explain how this particular line of code is vulnerable and how I should fix it please? – Grant Oct 18 '15 at 09:23
  • 1
    @arkascha It was an example; I'm certain there's other methods out there, but the point remains; the OP is using an unescaped request variable directly in a query without even typecasting it. – Daedalus Oct 18 '15 at 09:23
  • One well known side effect is how they drive developers slightly mad – Professor Abronsius Oct 18 '15 at 09:24
  • Ok so I just need to escape the request variable? – Grant Oct 18 '15 at 09:24
  • @Daedalus " I'm certain there's other methods out there"... that is hearsay. Actually I am more and more convinced that typical php/mysql installations are simply _not_ vulnerabilities to such attacks any more. I never found a working example for such a primitive attack. The only thing that I can think of is if you construct some wild construction deleting from joined tables. I never succeeded myself. Yes, you are right, sql injection should be prevented. but I am not sure if those "sql injection!" cries under each and every question here really help anyone. – arkascha Oct 18 '15 at 09:25
  • @arkascha While I recognize that you have more rep than I do, I could say the same your statement. Neither of us have quoted a source. I will however link Grant to an SO question worth at least a read. – Daedalus Oct 18 '15 at 09:28
  • 1
    @Grant http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – Daedalus Oct 18 '15 at 09:28
  • 1
    @Grant Escaping parameters helps, but it is the wrong approach. You should instead read about the advantages of "prepared statements" and "parameter binding". – arkascha Oct 18 '15 at 09:28
  • @Daedalus you do not really ask me to prove a negative, do you? :-))) – arkascha Oct 18 '15 at 09:29
  • @arkascha As far as I know, SQL injection is still a possible thing, however neither of us have quoted a definitive source backing up our own respective opinions. – Daedalus Oct 18 '15 at 09:31
  • @Daedalus And what might a "definitive source" be? Some article written by some journalist? But I get what you mean. And you have a point. Thanks! – arkascha Oct 18 '15 at 09:32
  • Ok, so wordpress has a prepare function as standard - https://developer.wordpress.org/reference/classes/wpdb/prepare/ So I guess I should be using this right? – Grant Oct 18 '15 at 09:33
  • @arkascha Official php documentation stating that such queries will no longer work. Though that's just one side of the coin, not taking into account the other langs out there. Anyway, I'll back away from this now. – Daedalus Oct 18 '15 at 09:33

1 Answers1

0

It is not a failed request, it is a query that affects no rows because none match the WHERE condition. Not a problem at all.

David Soussan
  • 2,698
  • 1
  • 16
  • 19