0

I'm encountering a very interesting issue with a MySQL stored procedure. The procedure is as follows:

DROP PROCEDURE IF EXISTS `removeSubscription`;
DELIMITER ;;
CREATE DEFINER=`root`@`%` PROCEDURE `removeSubscription`(IN `userId` int,IN `channelId` int,IN `channelTypeTitle` varchar(255))

BEGIN

SET @userId = userId;
SET @channelId = channelId;
SET @channelTypeTitle = channelTypeTitle;

DELETE FROM subscriptions 
WHERE 
    userid = @userId AND 
    channelid = @channelId AND 
    channeltypeid = (SELECT id FROM channeltypes WHERE `name` = @channelTypeTitle) 
LIMIT 1;

END
;;
DELIMITER ;

When this is called as a stored procedure from PHP, it is ignoring ALL of the 'WHERE' clause and is simply deleting the first row it encounters. This means that when the 'LIMIT 1' is left out, it deletes EVERYTHING from the table :s

This is the PHP:

$stmt = $db->prepare("CALL removeSubscription(:userId, :channelId, :channelTypeTitle)");

$stmt->bindValue('userId', $userId);
$stmt->bindValue('channelId', $channelId);
$stmt->bindValue('channelTypeTitle', $channelTypeTitle);

$stmt->execute();

Bizarrely enough, if I rename the passed parameters in both the PHP's prepare and in the Stored Procedure (for example, to have an 'x' before them), then it works correctly. Am I missing something obvious here?

Mark Hinch
  • 33
  • 5

1 Answers1

2

don't reuse the name of the variables names are supposed to be unique inside their scope... and don't use global variables inside the stored procedure (they have a global scoped, hence you should use local ones)...

DROP PROCEDURE IF EXISTS `removeSubscription`;
DELIMITER ;;
CREATE DEFINER=`root`@`%` PROCEDURE `removeSubscription`(IN `userId` int,IN `channelId` int,IN `channelTypeTitle` varchar(255))

BEGIN

declare userId_, channelId_, channelTypeTitle_ integer;

SET userId_ = userId;
SET channelId_ = channelId;
SET channelTypeTitle_ = channelTypeTitle;

DELETE FROM subscriptions 
WHERE 
    userid = userId_ AND 
    channelid = channelId_ AND 
    channeltypeid = (SELECT id FROM channeltypes WHERE `name` = channelTypeTitle_) 
LIMIT 1;

END
;;
DELIMITER ;
SQL.injection
  • 2,607
  • 5
  • 20
  • 37
  • I don't understand why suffixing a variable name with an _ is any different to prefixing it with an @. Does the @ prefix not make a variable sufficiently unique? – Mark Hinch Jul 05 '13 at 13:58
  • 1
    Aha, I just found the answer to that - they're session variables - see [link](http://stackoverflow.com/questions/1009954/mysql-variable-vs-variable-whats-the-difference) – Mark Hinch Jul 05 '13 at 14:12
  • Before I can really consider this resolved (...and understood by me), are the following assumptions correct? 1 - I was using global variables (simply bad practice), 2 - These global variables are case insensitive and 3 - The MySQL query was actually matching rows on themselves because the parameters therefore matched? (so, 'channelid = @channelId' really means 'channelid = channelid') Does that make sense? – Mark Hinch Jul 05 '13 at 14:51
  • 1
    @MarkHinch 1 - Yes, you should use local variables. The global variables might be changed by another process running on the server, perhaps another process executing the exact same time. If this happens you will have inconsistent data. Note the global variables have the prefix @ – SQL.injection Jul 05 '13 at 15:38
  • 1
    @MarkHinch 2- you should not try do the experiment: set @A='A'; set @a='a'; select @A,@a; and see the the result for yourself. – SQL.injection Jul 05 '13 at 15:39
  • 1
    @MarkHinch 3- if you run this query you will get your answer: set @userid= 1234; select * from subscriptions where userId = @userId; – SQL.injection Jul 05 '13 at 15:47
  • @MarkHinch and always use mySQL workbench or phpMyAdmin (or other tool) to test your queries before include them on php code / java / c# ... . To test stored procedures do something like this: set @a='a'; set @b='b'; set @c = 'c'; call removeSubscription(@a,@b,@c); – SQL.injection Jul 05 '13 at 15:48