Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108551
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
In-Reply-To: <D3BABA50-AE6A-4711-A595-878CA8A8175F@pmjones.io>
Date: Thu, 13 Feb 2020 15:31:44 -0500
Cc: internals@lists.php.net
Content-Transfer-Encoding: quoted-printable
Message-ID: <C0EED521-5C75-40F0-9723-2B6D5FFA799E@newclarity.net>
References: <50BD013E-CF72-414C-BBC0-A7A2E45CBDDB@pmjones.io>
 <5DEE5A6E-2587-432D-9651-320DF42A7732@newclarity.net>
 <A988787C-9EB6-4BF2-90DF-68F0D3BAC494@pmjones.io>
 <DC294D91-AA70-45C4-A3A5-32454675F42A@newclarity.net>
 <D3BABA50-AE6A-4711-A595-878CA8A8175F@pmjones.io>
To: "Paul M. Jones" <pmjones@pmjones.io>
Subject: Re: [PHP-DEV] RFC: Server-Side Request and Response Objects (v2)
From: mike@newclarity.net (Mike Schinkel)

> On Feb 13, 2020, at 1:34 PM, Paul M. Jones <pmjones@pmjones.io> wrote:
>=20
> Hi Mike,
>=20
> Thanks for your continued evaluation of the RFC.
>=20
>> Take a look at WordPress.  It does a lot of "fixup" to $_SERVER` =
variables =E2=80=94 to deal with badly implemented web servers =E2=80=94 =
to ensure all known variables have a value and that the format of the =
value is consistent. =20
>>=20
>> ...
>>=20
>> Another option actually would be to allow changes to $_SERVER prior =
to instantiating ServerRequest() the first time.
>=20
> Believe it or not, this RFC does make allowance for what you're =
describing, as a result of requiring a $globals array as the first =
parameter. An application-specific builder or factory can modify those =
$globals values as desired, before instantiating ServerRequest with the =
modified values.
>=20
> For example:
>=20
>    namespace App;
>=20
>    use ServerRequest;
>=20
>    class ServerRequestFactory
>    {
>        public function new(array $globals, ?string $content =3D null) =
: ServerRequest
>        {
>            // do fixup to $globals['_SERVER'] ...
>=20
>            // ... then:
>            return new ServerRequest($globals, $content);
>        }
>    }
>=20
>    $request =3D (new \App\ServerRequestFactory())->new($GLOBALS);
>=20
> I can easily imagine many ways to achieve the same end (i.e., =
modification of the constructor arguments before instantiating =
ServerRequest).
>=20
> Do you get where I'm coming from?

Ah, I see now.  That does allow for changing.

OTOH, what it does not do is ensure that everyone looking at the values =
are looking at the fixed-up values, nor does it offer any way to cache =
the original value except to do so at the top of every PHP file that is =
a URL endpoint. =20

I was hoping that your proposal was having a way to capture the original =
values before any PHP code could change them AND also then allow for =
fixed-up values to be provided for _every_ instantiation of =
ServerRequest(). This in case a library were to use it, I would not want =
the library to be getting the non-fixed up values. =20

So in that respect, this is worse than what we have now.

>> Why a method is important is to support filters (I guess I assumed =
that was obvious but realize now it was not.) For example:
>>=20
>> 	$request->get('redirect', FILTER_SANITIZE_URL);
>> 	$request->get('product_id', FILTER_SANITIZE_NUMBER_INT);
>> 	$request->get('email', FILTER_SANITIZE_EMAIL);
>>=20
>> You could potentially even have scoped, easier to remember constants =
that can work with autocomplete:
>>=20
>> 	$request->get('redirect', ServerRequest::URL);
>> 	$request->get('product_id', ServerRequest::INT);
>> 	$request->get('email', ServerRequest::EMAIL);
>=20
> [snip]
>=20
> I do see what you mean. Even so, I think filtering is out-of-scope for =
this RFC.

I would implore you to reconsider.  Without incorporating the =
functionality of filter_input() you have effectively neutered much of =
the value I envision your RFC having.  In the USA we might say "Besides =
that Mrs. Lincoln, how was the play?"

If I had a vote I would vote I would enthusiastically vote for the RFC =
if it includes filter_input() functionality. But I would vote against =
this RFC if it omits filter_input() functionality because it would mean =
another subsystem in core that does not actually address day-to-day =
concerns.

>  Certainly I want to avoid adding methods to the ServerRequest class, =
which as envisioned is a bag of values much the same way $_GET, $_POST, =
$_SERVER, etc. are bags of values.


Unless I misunderstand avoiding methods in ServerRequest is an arbitrary =
constraint which has no real-world requirement.  And if your motivation =
is to only mirror what exists then filter_input() is a function so an =
equivalent in ServerRequest should be a method and thus this in not =
inconsistent with your mirroring aspect.

-Mike=