Fixed User Alert decrementing below zero errors (_postDelete/_postSave)

[Xon]

Affected version

2.0.4

MySQL query error [1690]: BIGINT UNSIGNED value is out of range in '(`xenforo`.`xf_user`.`alerts_unread` - 1)'

This is caused by this;

$this->db()->query("
   UPDATE xf_user
   SET alerts_unread = GREATEST(0, alerts_unread - 1)
   WHERE user_id = ?
", $this->alerted_user_id);

The required solution is;

$this->db()->query("
   UPDATE xf_user
   SET alerts_unread = GREATEST(0, cast(alerts_unread AS SIGNED) - 1) 
   WHERE user_id = ?
", $this->alerted_user_id);

 

[DragonByte Tech]

You'd think MySQL / MariaDB would be smart enough to parse the GREATEST function before actually trying to check the range of the column, but I tested this on MariaDB 10.2.14 and I can confirm the issue by simply manually running the query.

Can also confirm your fix, the only thing I wonder about is whether it would mess up if the value was above the range of a signed int? It's not relevant for alerts, but for other things it may be.

EDIT: Tested it by manually setting the unread alert count to 65535 (the max for a smallint in MySQL) and ran the query, works fine and correctly sets the value to 65534


Fillip

 

[Xon]

@DragonByte Tech I actually ran into this with Alert Improvements for XF1, when I saw this error I knew immediately what XF2 was doing and double check and yup, mixing unsigned integer with a subtraction operation which can go below zero.

 

[TickTackk]

One would also run into reported bug when trying add a add a alert directly using XF:UserAlert entity and a value for column view_date for a user who has 0 unread alerts.

 

[Chris D]

There's a bunch of similar queries so they're all sorted now for the next release.

 

[Xon]

Can also confirm your fix, the only thing I wonder about is whether it would mess up if the value was above the range of a signed int? It's not relevant for alerts, but for other things it may be.

AFAIK, MySQL will expand the size of an integer during an integer operation, and as long as it fits in the destination it is all good. But doesn't change the signed-ness since this can significantly change the results of the operation.