Skip to content

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

Closed
wants to merge 3 commits into from

Conversation

turboezh
Copy link
Contributor

@turboezh turboezh commented Jul 21, 2016

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

Fix quoting non-ascii and binary strings. Add more precise handling of BOOL and INT.
@weltling
Copy link
Contributor

Does this one obsolete #2001 (if yes, the older one should be closed). @adambaratz, could you please check together with Alex?

Thanks.

@turboezh
Copy link
Contributor Author

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;
Copy link
Contributor

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.

@adambaratz
Copy link
Contributor

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.

@smalyshev smalyshev added the Bug label Sep 5, 2016
@krakjoe
Copy link
Member

krakjoe commented Jan 3, 2017

@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.

@turboezh
Copy link
Contributor Author

Hi.
It is not abandoned. But I'm not sure yet how to solve all this the best way. And I think now that BOOL and LOB cases should be done and discussed in separate PRs, i.e. this one should be reworked.
I doesn't mind if anyone will present another solution.

@krakjoe
Copy link
Member

krakjoe commented Apr 3, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants