Hi Sara, > -----Original Message----- > From: p...@golemon.com [mailto:p...@golemon.com] On Behalf Of Sara > Golemon > Sent: Thursday, April 20, 2017 10:56 PM > To: PHP internals <internals@lists.php.net> > Subject: [PHP-DEV] On malformed transport strings > > My fix to https://bugs.php.net/bug.php?id=74216 tightened down the definition > of what a valid transport string looks like. > > Previously, transport strings like > "tcp://127.0.0.1:80:81:82/your/moms/face" would be accepted by PHP as > perfectly valid URIs. Since this was never documented as a feature of > transports, but rather a simple oversight during the Bush era, I added some > sanity checking to verify there was no garbage after the port number in IP > based > transport strings. > > Then they came out of the woodwork... > > https://bugs.php.net/bug.php?id=74429 depends on the previous "ignore > garbage" behavior in order to create named persistent streams. (ex: > "tcp://127.0.0.1:80/main_connection" and > "tcp://127.0.0.1:80/secondary") In short, the persistence key is defined by > the > whole transport string, while the address/port parsing ensures that the > uniqueifying portion doesn't prevent the connection from happening. The > project referenced in that bug report and predis both utilize this behavior. > Thanks for getting onto this. Leaving aside, that the functionality is undocumented, in general it appears to be useful in some situations. Probably some would ask for that anyway, if it were not provided due to the implementation bug 😊
> https://bugs.php.net/bug.php?id=74432 also popped up today as a bug in how > mysqli (specifically via mysqlnd) invokes connections to the backend. > Ultimately, > an address provided blindly gets a (potentially > default) port number tacked onto the end. Again, undocumented behavior > working up until the 74216 fix. > > I'm inclined to revert to prior "ignore garbage" behavior on the 7.0 and 7.1 > branches to avoid BC break trauma (though I do think raising a warning is > advised). What's uncertain in my mind is whether or not we take a hard line > on > "Use the API as documented" for 7.2 or if some other middle ground is > appropriate. Particularly given the use case of named persistent transports. > The > right way to do that would be to have a new API for named transports, possibly > just as a context option. > I'd be suggesting this as well. Either we could make this part only backward compatible, as suggested in your follow up patch, or one could check whether a solution were possible to fix the initial fsockopen() issue without affecting the ip parsing parts globally. Depends probably on how sensible the work amount would be. Please be aware, that an action should be taken next days before Tuesday, so then we can ask for tests on the RCs. And otherwise, an official way closing the actual undocumented gap could be suggested for 7.2. Thanks Anatol
