Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:98830
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: softfail (pb1.pair.com: domain golemon.com does not designate 209.85.128.178 as permitted sender)
MIME-Version: 1.0
Sender: php@golemon.com
Date: Thu, 20 Apr 2017 13:56:01 -0700
Message-ID: <CAESVnVpGoG45bBGfcYaDKb-A1k33VEwcK_GX5_TS6JHj0Dve1w@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: On malformed transport strings
From: pollita@php.net (Sara Golemon)

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.

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.

-Sara

https://pbs.twimg.com/media/C9uYe-uUQAAPpbK.jpg