> -----Original Message----- > From: p...@golemon.com [mailto:p...@golemon.com] On Behalf Of Sara > Golemon > Sent: Wednesday, April 26, 2017 5:35 PM > To: Anatol Belski <a...@php.net> > Cc: PHP internals <internals@lists.php.net>; Joe Watkins <krak...@php.net>; > Davey Shafik <da...@php.net>; Remi Collet <r...@php.net> > Subject: Re: [PHP-DEV] On malformed transport strings > > On Wed, Apr 26, 2017 at 6:20 AM, Anatol Belski <a...@php.net> wrote: > > Thanks for this additional check. My action was actually based on the > comment with the patch link, looks like the situation has now changed a bit. > We're still quite limited in choice in this case. For one, there's a low > security > impact, however the fix uncovered several inconsistent places breaching apps. > For what it matters, there are already 2-3 dups regarding mysqli and stream > client regressions. Given they come so short in time, that's not a good sign. > Though, the reports still came late enough, that an appropriate fix could not > be > done before the next RC. > > > The fact that there are dups tells me that, despite the fact that > bab0b99f3 made into 7.0.18/7.1.4 releases, we should fully revert the hard > error > (leaving a soft warning behind). The security implications of the original > fix are > fairly minor* compared to the much larger regression of actually breaking > sites > which otherwise worked before. > > > In the end, after evaluating the situation, I would still suggest to keep > > your > follow up fix as a temporary solution in the next release. This way at least > one > issue is fixed, the stream client, while the initial patch is a bit > slackened. A better > fix can be worked out till the follow up release, also targeting the mysqli > regression which still persists. This way, one regression is fixed, the > initial patch > is weakened a bit but as the impact was low - it's something one can > temporarily > live with, and a good solution were to expect in the next possible future. An > alternative were to revert the hotfix in the final and keep the regressions. > > > Given that there *is* a release with bab0b99f3 in it, I suppose we're already > regressed and a little clowny looking. 7.0 is your branch, so if you're cool > with > some uses still being slightly borky, then so am I. I'll do up some diffs for > 7.0.20/7.1.6 to downgrade the hard errors to warnings (keep it hard error for > 7.2.0) and address issues like the mysqli_connect implicit port duplication. > Well, there is indeed a breakage in the legacy area, and there is still a security context. The former requires quite some care, and the latter should not be ignored. With cda7dcf4 one regression is solved, even if temporarily. From what I see now, the mysqli part is even more critical, fe the connection strings from the WP docs https://codex.wordpress.org/Editing_wp-config.php#Set_Database_Host . Whereby I can tell, that many major projects handle this properly, fe https://github.com/s9y/Serendipity/blob/master/include/db/mysqli.inc.php#L224 .
Maybe when you've some patch anytime soon, let's test it and then consider whether we can include it into final? Or, at least some minimal specific approach to fix the mysqli part, and care about the general solution for the subsequent release. What I'd basically avoid is making changes in stress, as there might be other beyond places and we shouldn't risk to introduce more breach than there already is. Instead, that requires a cold head and a lot of QA 😉 Regards Anatol
