> On Feb 12, 2020, at 7:47 AM, Paul M. Jones <[email protected]> wrote:
>
> That is essentially what it does now; the difference is that you mimic the
> $GLOBALS array at construction time, and the instance locks automatically:
>
> $request = new ServerRequest([
> '_GET' => ['foo' => 'bar'],
> '_COOKIE' => ['id' => 123],
> '_SERVER' => $_SERVER,
> ]);
> // $request is now locked
>
> The class that contains ServerRequest would then build up that array for the
> constructor.
>
> Do you feel that's close enough to what you're asking for?
No. IMO it should not lock until explicitly locked.
Why? Take a look at WordPress. It does a lot of "fixup" to $_SERVER`
variables — to deal with badly implemented web servers — to ensure all known
variables have a value and that the format of the value is consistent.
If you always lock on instantiation then WordPress would have to default to
continue using $_SERVER. Or subclass it, but that would generate copying
overhead and it would be too easy for programmers to just call ServerRequest()
again.
Another option actually would be to allow changes to $_SERVER prior to
instantiating ServerRequest() the first time.
Frankly though, none of those options should ideal, include your current
proposed solution. Basically making a copy of $_SERVER so ServerRequest() is
not authoritative, but alternately your solution does not allow frameworks to
fix the inconsistency across servers.
Maybe what would be needed is a single ServerRequest::initialize( $closure )
where the ciosure can do the fixup. This would need to be called before new
ServerRequest() is called after which ServerRequest would be locked for changes.
> First, I'd have to decline adding request() (or $request) at all; my opinion
> is that one ought to be reading from $get, $post, $cookies, etc.
> specifically, not from a pool of those values.
That's definitely fair. I almost did not include request() in my suggestion
but did because PHP has $_REQUEST.
> Second, if I understand you correctly, I much prefer the property access over
> the method getter; it just "looks and feels better":
>
> $request->get['foo']
> vs
> $request->get()['foo'];
>
> Let me know if that makes sense to you or not.
I was actually proposing a third option:
$request->get('foo');
Or as I like to do in my own code (to indicate where it comes from):
$request->GET('foo');
Why a method is important is to support filters (I guess I assumed that was
obvious but realize now it was not.) For example:
$request->get('redirect', FILTER_SANITIZE_URL);
$request->get('product_id', FILTER_SANITIZE_NUMBER_INT);
$request->get('email', FILTER_SANITIZE_EMAIL);
You could potentially even have scoped, easier to remember constants that can
work with autocomplete:
$request->get('redirect', ServerRequest::URL);
$request->get('product_id', ServerRequest::INT);
$request->get('email', ServerRequest::EMAIL);
> incorporate the functionality of filter_input(). Otherwise we have to bypass
> the objects to get filtering.
>
> I don't think you'd have to bypass the objects to get filtering; unless I am
> missing something, this ...
>
> $foo = filter_input(INPUT_GET, 'foo', FILTER_SANITIZE_SPECIAL_CHARS);
>
> ... would easily become this:
>
> $foo = filter_var($request->get['foo'], FILTER_SANITIZE_SPECIAL_CHARS);
>
> There might be behavioral nuances between the two, but the point is that you
> can still do filtering.
But wouldn't it be 1000% easier to write and easier to read code like this?
$foo = $request->get( 'foo', ServerRequest::SPECIAL_CHARS);
Since filtering $_GET elements is 100% done when accessing them it makes sense
to streamline the operation. But if a developer does not want to filter, they
just do this:
$foo = $request->get( 'foo' );
>> Would you not also add an option to generate a warning when using them for
>> those who want to deprecate their use in their own code (deprecating across
>> the board would be too extreme give how much CMS and framework code uses
>> them intimately.)
>
> That seems a bit much at this point. ;-)
Really? Seems like this and some guard code is all it would take:
ini_set( "disallow_superglobals", true);
-Mike
