Bug #74429 [Asn]: Remote socket URI with unique persistence identifier broken

Edit report at https://bugs.php.net/bug.php?id=74429&edit=1 ID: 74429 Updated by: pollita@php.net Reported by: martijn dot grendelman at isaac dot nl Summary: Remote socket URI with unique persistence identifier broken Status: Assigned Type: Bug Package: Streams related Operating System: Any PHP Version: 7.0.18RC1 Assigned To: pollita Block user comment: N Private report: N New Comment: No. That "undocumented feature" was never intended to work, and the use of it is an abuse of the API. As to allowing multiple persistent connections to a backend resource, I think we could add official support for it via a new API, and perhaps even matching this use case, but it's going to take some discussion because I don't want to trivially walk into codifying something for no better reason that "It worked on accident at some point." Previous Comments: ------------------------------------------------------------------------ [2017-04-18 00:51:55] dominic at varspool dot com This user-contributed note should be amended/removed following an outcome: http://php.net/manual/en/function.stream-socket-client.php#105393 ------------------------------------------------------------------------ [2017-04-18 00:48:13] dominic at varspool dot com Here's another use case (one that I introduced, so I'm feeling guilty about): https://github.com/nrk/predis/pull/139 The problem being that Redis has the concept of multiple databases per server (so, the host, port etc. will match). So, if you want to use a persistent connections, you end up with cross-writes between the databases. (More worked example at that link.) ------------------------------------------------------------------------ [2017-04-14 06:49:26] martijn dot grendelman at isaac dot nl Hi, I'm just a user, and I normally don't run RCs. It takes time for PHP releases to make their way to the distributions and to users' computers, so I only noticed yesterday when upgrading one of our servers to PHP 7.0.18. Cheers, Martijn. ------------------------------------------------------------------------ [2017-04-13 16:27:34] ab@php.net Typo, - otherwise treat anything AFTER "/" as a unique id, if failed to parse Thanks. ------------------------------------------------------------------------ [2017-04-13 16:22:31] ab@php.net @martijn dot grendelman at isaac dot nl, thanks for reporting. Ref to bug #74216 for details. While it's a low security impact in fsockopen(), same piece is used for parsing in stream_socket_client(). Indeed, the "feature" is not documented so was not guaranteed to work. The way it is exploited is misleading, as tcp:// has no optional parameters. Nevertheless, probably we should indeed fix this case, especially the use case looks falling under the current scheme and the functionality itself makes sense. Fe one can say, a new optional parameter is introduced to tcp:// sheme, so then something like tcp://ip:port/id=bla would be a valid use. Or otherwise, for BC, anything after "/" would be read as an id. I'd suggest this - don't affect the current fix to fsockopen - add optional parameter to the tcp:// scheme in all branches, say id=bla - otherwise treat anything before "/" as a unique id, if failed to parse The last one - only for stable branches as a BC measure, master should be clean failing if the optional parameter couldn't be parsed, call it "id" or whatever. Sara, what do you think? @martijn i also can't avoid mentioning, that this patch is around for more than a month and is also present in the RCs. So for one, it is sad to see pure QA participation quote, and it also probably shows the use case makes no wide impact. Thanks. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=74429 -- Edit this bug report at https://bugs.php.net/bug.php?id=74429&edit=1