[PHP-DEV] Use MySQL syntax only for parsing MySQL statements in PDO

[Sergei Morozov]

Hi Internals,

There are bugs #79276  and #80340
 where PDO parses valid PostgreSQL
statements incorrectly because it uses MySQL grammar for all statements
regardless of the underlying platform.

I submitted pull request #6410  which
enables extensions to opt-in to MySQL-specific syntax (since it's not
standard). Currently only `pdo_mysql` uses it.

As Christoph Becker suggested, I'm bringing it here for discussion.

Technically, it may be a BC break since there may exist PDO-based
extensions outside the PHP repository that rely on compatibility with the
MySQL syntax.

Are there known extensions like this? Would this change be acceptable in
one of the next minor releases?

Regards,
Sergei Morozov

Re: [PHP-DEV] Changes to Git commit workflow

[Jakub Zelenka]

On Thu, Apr 1, 2021 at 3:21 PM Bishop Bettini  wrote:

> On Thu, Apr 1, 2021 at 9:22 AM Rowan Tommins 
> wrote:
>
> > On 01/04/2021 05:54, Bishop Bettini wrote:
> > > I've documented why we need signing, and how to set it up:
> > >
> > > https://wiki.php.net/vcs/commit-signing
> > >
> > > Feedback welcomed!
> >
> >
> > This looks great, and very easy to follow.
> >
> > One edit I would strongly suggest though:
> >
> > Remove the "Passphrase:" line from the --generate-key command, so that
> > gpg will prompt interactively for the passphrase using the same entry as
> > it will use later when signing. You should never include a password or
> > passphrase in a command if you can avoid it, as it will be visible on
> > your screen, and stored in plain text in your shell history.
> >
> >
> > Some additional tips that might be worth adding:
> >
> > As an advanced setup suggestion, "gpg --full-generate-key" launches a
> > wizard with a couple of extra prompts.
> >
> > If you're on Ubuntu and don't have a new enough git (e.g. 18.04LTS ships
> > with 2.17.1), there is an official PPA to upgrade it; just run: "sudo
> > add-apt-repository ppa:git-core/ppa && sudo apt update && sudo apt
> > install git"
> >
> > Before pushing to github, you can verify the signature on a commit
> > locally with "git show --show-signature HEAD", or similarly for a tag by
> > passing the tag name.
> >
>
> Excellent suggestions. I've updated the guide with these.
>
> I also added a FAQ.
>
> https://wiki.php.net/vcs/commit-signing
>
>
Nice! It would be great if we start enforcing that. I finally set it up
too. I should have done it long time ago considering that I have been
maintaining gnupg extension for some time. :)

I think it's better to always use subkey for signing commits so that might
be good to add to the tutorial. Also RSA is getting a bit heavy with big
keys so it might be also good to suggest using ECC (e.g. EdDSA) which is
faster and possibly more secure. Although it's still in expert settings but
it works fine and a good tutorial can be found here
https://dev.to/benjaminblack/signing-git-commits-with-modern-encryption-1koh
.

Regards

Jakub

Re: [PHP-DEV] [8.1] Release Manager Election

[Saif Eddin Gmati]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hello,

I would like to introduce myself to those of you who might not know me.

My name is Saif ( also known as "azjezz" ), I'm 21 years old, i have been 
working with PHP for approximately 8 years.

I'm currently employed as a senior developer at [Les 
Tilleuls](https://les-tilleuls.coop/en), before that i worked at Symfony SAS 
for 1 year and 6 months, in total, i have 2 years of professional experience.

I maintain some open source projects ( e.g: 
[PSL](https://github.com/azjezz/psl) ), and help contribute to some other on my 
free time ( e.g: [Symfony](https://github.com/symfony/symfony), 
[Psalm](github.com/vimeo/psalm) ).

I'm mostly interested in type systems, and asynchronous programming.

I have been subscribed to the internals mailing list for few years now ( didn't 
engage much ), but i want to help the community within my powers, and i think i 
can take on this role with the help of the veteran RM.

If you have any additional questions, you can reach me via email, twitter, or 
keybase.

Regards.

GitHub: https://github.com/azjezz
Twitter: https://twitter.com/azjezz
Keybase: https://keybase.io/azjezz

-BEGIN PGP SIGNATURE-
Version: ProtonMail

wsFzBAEBCAAGBQJgcdq1ACEJELAOCkaz8cFXFiEEOazMpP0wDQTIQG6zsA4K
RrPxwVfmgA/9GJ2KhEKBuGoH+W9GwLNjw+BrkmAymmZV1E/mBSSzI3aHmC1s
wLRsqgu6Q9qvReULn9YejhO7CPJNSKiGtvZwcNrZZdMpifhV+LMIpzLPXi0I
/1pON8glFwFJACdA1wfEejJtTlcX5ZtZIL1MZ5sZ6jM+/NTtVp1S8z2Cxwe3
SHWvdrUlH/WI64q4SBPraaHenPIvEhezd4ZUV/xQuPog/aBughXvTjw8ItsT
VYoKs0tWy8WRkGHWg9Y9AhfDaL2kdzdEEytbs7qD0GFrikfrZFJo8vLLxCnm
wW8OR+P4A2Z/Fab8JWt40NCuOGjpBo/G2n+y3WM0XGxiJGKssM3m4QeymkPu
iakn+IE9NsY2cY6p16opUNBDsi/1SCiZUykkMncR4vpKXInpmTdImUykdn2a
fS4qivLBcYPllRUCcjIzXZT7yNq6HSoXOMrwqhHxWIJZv1EA/KotHfoTxT9t
ioTy6Vv6vx/hb/LLlU9X+r+mw+CPRXk/FKmF8a6/anyUvs6JQC1Iw00nrqJl
hurVI4gFn/8yvRdvtgUdZ7LDoQvIAXMJMUHqeTsXOf1ct+WFRT/8eGeBjYRo
eh29ALlgEZd588UqcnLkGcvj8zYjXwSz9JXgcvXUHnc2VwaYb+UC/Q59H52K
VFiKwWb90B5JabF97i4mgwG81CZaGaFvjEc=
=8Xc0
-END PGP SIGNATURE-

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Re: [PHP-DEV] Use MySQL syntax only for parsing MySQL statements in PDO

[Kamil Tekiela]

This is an interesting solution to the problem, but I am unsure if it's the
best one. I didn't look into it in detail yet, but at first glance, it
seems to be a cheap hack to solve a bug. What about backtick character ` ?
What about NO_BACKSLASH_ESCAPES mode? Any question mark within
backtick-escaped column name would still cause a problem. In fact, when I
tested it with NO_BACKSLASH_ESCAPES the PR didn't fix the problem. I
got a different error, but the problem still exists.
This is a naive solution that fixes one tiny bug in a single driver with no
regards for tons of other issues. I am against adding such hacks into the
PHP source code.

What we need is a full SQL parser that is context-aware. This task should
be delegated to the PDO drivers. PDO can't do it reliably at the base
level. The problem is that PDO is never going to be able to reliably parse
the SQL even if we have a separate parser for each driver. PDO is not the
server and it doesn't know how the SQL will be parsed at the end of the
day. At its root, it seems to be an unfixable problem. Emulated prepares
have a lot of issues and this is only one of them. The solution is to use
native prepares whenever they are available on the server.

If you want you can see the effect your PR has on this test case:

$pdo = new \PDO("mysql:host=localhost;dbname=test;charset=utf8mb4", 'user',
'password', [
  \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
  \PDO::ATTR_EMULATE_PREPARES => true
]);
$pdo->exec("SET sql_mode = NO_BACKSLASH_ESCAPES;");
$stmt = $pdo->prepare("SELECT * FROM foo WHERE text='\' and 1=? and
text='\'");
$stmt->execute([1]);

I'm sorry but this is a hard pass from me.

Re: [PHP-DEV] Use MySQL syntax only for parsing MySQL statements in PDO

[Sergei Morozov]

Hi,

> it seems to be a cheap hack

To me, it seems to be within the existing architecture. I agree that the
existing architecture has its flaws.

> What about backtick character ` ? What about NO_BACKSLASH_ESCAPES mode?

These are out of scope. I wasn't aware of the NO_BACKSLASH_ESCAPES but it
indeed seems to be impossible to support on the client side.

> Emulated prepares have a lot of issues and this is only one of them.

This specific issue doesn't seem to be related to the emulation. As far as
I understand, PDO parses SQL as well in order to replace positional
placeholders and vise versa depending on the input and the target platform
capabilities.

> If you want you can see the effect your PR has on this test case

I tried running the example on the stock PHP 8.0.3, and regardless of the
emulation mode and NO_BACKSLASH_ESCAPES mode I saw the same error:

> SQLSTATE[HY093]: Invalid parameter number: number of bound variables does
not match number of tokens in

With my patch, I don't expect to see any changes in parsing MySQL queries
so I don't understand how it's relevant. Am I missing something?

On Sat, Apr 10, 2021 at 3:33 PM Kamil Tekiela  wrote:

> This is an interesting solution to the problem, but I am unsure if it's
> the best one. I didn't look into it in detail yet, but at first glance, it
> seems to be a cheap hack to solve a bug. What about backtick character ` ?
> What about NO_BACKSLASH_ESCAPES mode? Any question mark within
> backtick-escaped column name would still cause a problem. In fact, when I
> tested it with NO_BACKSLASH_ESCAPES the PR didn't fix the problem. I
> got a different error, but the problem still exists.
> This is a naive solution that fixes one tiny bug in a single driver with
> no regards for tons of other issues. I am against adding such hacks into
> the PHP source code.
>
> What we need is a full SQL parser that is context-aware. This task should
> be delegated to the PDO drivers. PDO can't do it reliably at the base
> level. The problem is that PDO is never going to be able to reliably parse
> the SQL even if we have a separate parser for each driver. PDO is not the
> server and it doesn't know how the SQL will be parsed at the end of the
> day. At its root, it seems to be an unfixable problem. Emulated prepares
> have a lot of issues and this is only one of them. The solution is to use
> native prepares whenever they are available on the server.
>
> If you want you can see the effect your PR has on this test case:
>
> $pdo = new \PDO("mysql:host=localhost;dbname=test;charset=utf8mb4",
> 'user', 'password', [
>   \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
>   \PDO::ATTR_EMULATE_PREPARES => true
> ]);
> $pdo->exec("SET sql_mode = NO_BACKSLASH_ESCAPES;");
> $stmt = $pdo->prepare("SELECT * FROM foo WHERE text='\' and 1=? and
> text='\'");
> $stmt->execute([1]);
>
> I'm sorry but this is a hard pass from me.
>


-- 
Sergei Morozov