Fix db escape negative integers #5277
Conversation
system/Database/BaseConnection.php
Outdated
| @@ -1201,7 +1203,7 @@ public function escape($str) | |||
| } | |||
|
|
|||
| if (is_numeric($str) && $str < 0) { | |||
There was a problem hiding this comment.
Why is there a need to add this for negative numbers? I think we can remove this if block entirely as the last return will do the job.
There was a problem hiding this comment.
And it seems inconsistent, with negative numbers we get a string. For positive, we get numbers.
There was a problem hiding this comment.
Good point!
For now, I remove it.
There was a problem hiding this comment.
let's have @lonnieezell shed the light on this. 😂
There was a problem hiding this comment.
I don't remember exactly. It's been 4 years :) . Looking back through the issue, though, I'm guessing that wrapping it in quotes would get it past the escaping and adding of the slash, which was then converted by MySQL into an INT or whatever type when it was saved to the table.
I would say as long as we have tests in place that ensure his problem is still fixed, and it doesn't escape the negative sign, this is fine.
kenjis
force-pushed
the
fix-escape-negative-integers
branch
from
e3571ae to
b009bfd
Compare
kenjis
added
database
| @@ -40,7 +40,7 @@ protected function setUp(): void | |||
| */ | |||
| public function testEscapeProtectsNegativeNumbers() | |||
There was a problem hiding this comment.
This should be renamed to say that this does not escape negative numbers now.
It is inconsistent, with negative numbers we get a string. For positive, we get numbers.
kenjis
force-pushed
the
fix-escape-negative-integers
branch
from
b009bfd to
127b9cf
Compare
Description
Fixes #4973
Checklist: