Edit report at https://bugs.php.net/bug.php?id=74429&edit=1
ID: 74429
Comment by: dominic at varspool dot com
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:
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.)
Previous Comments:
------------------------------------------------------------------------
[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.
------------------------------------------------------------------------
[2017-04-13 09:26:55] martijn dot grendelman at isaac dot nl
Description:
------------
Commit https://github.com/php/php-src/commit/bab0b99f376dac9170ac81382a5ed526938d595a
changes the way remote socket specifiers are parsed for a stream socket.
URIs like 'tcp://localhost:80/uniqueid' now result in an error. That they used to work
seems like an undocumented feature, but I know it to be used in at least one open source project: https://github.com/colinmollenhour/credis.
Was this change intended to break this behaviour?
Best regards,
Martijn Grendelman
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=74429&edit=1