Re: RFC: Server-Side Request and Response Objects (v2)

> On Feb 12, 2020, at 7:47 AM, Paul M. Jones <pmjones@pmjones.io> 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