-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix #67495: pdo_dblib incorrectly escapes binary and unicode values #2017
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fix quoting non-ascii and binary strings. Add more precise handling of BOOL and INT.
Does this one obsolete #2001 (if yes, the older one should be closed). @adambaratz, could you please check together with Alex? Thanks. |
These are independent PRs. #2001 adds ability to stringify field type and doesn't relate to quoting. |
}; | ||
|
||
if (is_ascii && (unquoted[i] < 32 || unquoted[i] > 126)) { | ||
is_ascii = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple practical issues with doing this automatically:
- While pdo_dblib is commonly used with MSSQL, that's not guaranteed. Some DBs might not like this syntax.
- It could force undesirable casts. You care about the destination column type, not the data you're passing.
There are probably other scenarios where being clever isn't advantageous. I'd thought about creating syntax like this:
$db->quote('Съешь же ещё этих мягких французских булок, да выпей чаю.', PDO::PARAM_STR | PDO::PARAM_STR_UNICODE);
That way, it's totally explicit. Which is generally a desirable trait when talking to databases. You could also have some driver attributes that let you default into unicode mode if that's desired. You'd then want a PDO::PARAM_STR_ASCII
type for overriding it.
I left some comments on specific points. I want to solve these problems with pdo_dblib, but I think there are some cases here you're not accounting for. I think this is one of those situations where the param types offered by PDO are limiting. |
@turboezh fix conflicts please :) In addition, can you respond to the comments left by other contributors, as I am not sure of the current status of this PR. If you consider this work abandoned, please close this PR. |
Hi. |
Since there is no clear consensus on this fix, and since it still has unresolved conflicts, and having waited a month for those conflicts to be fixed, I'm closing this PR. |
Fix quoting non-ascii and binary strings.
Also add more precise handling of BOOL and INT.
https://bugs.php.net/bug.php?id=67495
May ralates to:
https://bugs.php.net/bug.php?id=72414
https://bugs.php.net/bug.php?id=60818