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

  108436
February 10, 2020 16:18 pmjones@pmjones.io ("Paul M. Jones")
Hi Internals,

After a couple of years of incubation, I am happy to offer a second version of this RFC:

  https://wiki.php.net/rfc/request_response

It proposes an object-oriented approach around request and response functionality already existing in PHP, in order to reduce the global-state problems that come with superglobals and the various response-related functions.

The SQLite "about" page says, "Think of SQLite not as a replacement for Oracle but as a replacement for fopen()." <https://www.sqlite.org/about.html>  Likewise, think of this RFC not as a replacement for HttpFoundation or PSR-7, or as a model of HTTP messages, but as an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on.

Thanks in advance for your time and consideration while evaluating it.


-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php
  108437
February 10, 2020 16:46 michal.brzuchalski@gmail.com (=?UTF-8?Q?Micha=C5=82_Brzuchalski?=)
Hi Paul,

thanks for bringing it back. Got some questions:

1. In your RFC $_ENV is bound to $request.

Looking at HttpFoundation can see it's build from $_GET, $_POST, $_FILES,
$_COOKIE, $_SESSION
and then looking at PSR ServerSideRequest it's build from $_COOKIE, $_GET,
$_POST, $_FILES, $_SERVER
but none of them bounds $_ENV.

Server environment variables basically don't change over time when server
process runs.
What is the specific use case that requires them to be coupled with
$request instance?

2. In your RFC all headers are mapped to $request properties

This behaviour is unclear and unusual to me, for eg. in summary can see
that if
server request has "Header-Name" it is found in
$_SERVER["HTTP_HEADER_NAME"] and
that one is bound to $request->headers["header-name"], ok but the next line
with example for
"Content-Type" shows that it's bound to $request->contentType - which means
probably that it can
be accessed in $request->headers["content-type"] as well, right? seems
logic.

Going further, for eg. $_SERVER["PHP_AUTH_PW"] is bound to $request->authPw
and then
what will happen if the request will receive "Auth-Pw" header?

a) It'll be bound to $request->headers["auth-pw"] ?

b) It'll be bound to $request->authPw ?

c) None of above.

With those above I would see a set of getters like getContentType() or
getAuthPassword() etc.
instead of ServerRequest instance properties and a single point to retrieve
header values just like
well known ParameterBag's from HttpFoundation to operate on get, post,
headers etc.

BR,
--
Michał Brzuchalski
  108479
February 11, 2020 13:14 pmjones@pmjones.io ("Paul M. Jones")
Hi Michał,

Good to hear from you!

(While writing this, I realized I made a couple of mistakes in the RFC examples related to CONTENT_LENGTH and CONTENT_TYPE that might have caused confusion; I have now fixed them.)


> On Feb 10, 2020, at 10:46, Michał Brzuchalski brzuchalski@gmail.com> wrote: > > Hi Paul, > > thanks for bringing it back. Got some questions: > > 1. In your RFC $_ENV is bound to $request. > > Looking at HttpFoundation can see it's build from $_GET, $_POST, $_FILES, $_COOKIE, $_SESSION and then looking at PSR ServerSideRequest it's build from $_COOKIE, $_GET, $_POST, $_FILES, $_SERVER but none of them bounds $_ENV. > > Server environment variables basically don't change over time when server process runs. What is the specific use case that requires them to be coupled with $request instance?
I have no specific use case; I recall that $_ENV showed up in more than a couple of reviewed implementations, and so included it here. If there is no objection from John Boehr (who did the majority of heavy lifting C), or from anyone else, then I would be fine with removing the ENV-related element. I have no strong feelings on $_ENV representation either way.
> 2. In your RFC all headers are mapped to $request properties > > This behaviour is unclear and unusual to me, for eg. in summary can see that if server request has "Header-Name" it is found in $_SERVER["HTTP_HEADER_NAME"] and that one is bound to $request->headers["header-name"], ok but the next line with example for "Content-Type" shows that it's bound to $request->contentType - which means > probably that it can be accessed in $request->headers["content-type"] as well, right? seems logic.
I'm not sure if that is a question/comment/criticism, or a statement of "OK so far, but ...". Please bear with me, so I can make sure I'm addressing your question properly. First: Not *all* headers are mapped to $request properties; the only ones that get special $request properties are the ones that showed up as frequently getting special treatment in other implementations. Next: the $header array is populated by all $_SERVER['HTTP_*'] values, plus $_SERVER['CONTENT_TYPE'] and $_SERVER['CONTENT_LENGTH']. The latter two do not follow the 'HTTP_*' pattern because of RFC 3875; they come in as (e.g.) `Content-Type: text/plain`, so you might expect $_SERVER['HTTP_CONTENT_TYPE'], but RFC 3875 dictates otherwise. Finally: yes, $request->header['content-type'] is available at the same time as $request->contentType, but they are not quite the same thing. The Content-Type header also allows you to specify a charset, which value gets populated into $request->contentCharset. So, you can read these values these ways: - $request->header['content-type'] => 'text/plain; charset=UTF-8' (the original header) - $request->contentType => 'text/plain' (parsed out by ServerRequest) - $request->contentCharset => 'UTF-8' (parsed out by ServerRequest) Likewise, PHP puts `Content-Length: 123' into $_SERVER['CONTENT_LENGTH']; ServerRequest additionally puts it into $request->headers['content-length'] as a string; and then further tries to represent the value as an integer in $request->contentLength. So: - $request->server['CONTENT_LENGTH'] => (string) '123' - $request->headers['content-length'] => (string) '123' - $request->contentLength => (int) 123 My apologies if all of that was stuff you already knew and understood. (And if you did not already know it, maybe that means I should write up a narrative of the logic being applied.)
> Going further, for eg. $_SERVER["PHP_AUTH_PW"] is bound to $request->authPw and then what will happen if the request will receive "Auth-Pw" header?
In that example, PHP would populate two $_SERVER elements: 'PHP_AUTH_PW' from the password, and 'HTTP_AUTH_PW' from the header. ServerRequest only populates PHP_AUTH_PW into $authPw; the HTTP_AUTH_PW value would (per the above description) go into $request->headers['auth-pw']. I hope that made sense; let me know if it did not.
> With those above I would see a set of getters like getContentType() or getAuthPassword() etc. instead of ServerRequest instance properties and a single point to retrieve header values just like well known ParameterBag's from HttpFoundation to operate on get, post, headers etc.
Does the above explanation help to modify your preferences here? If not, we can continue talking about it. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108438
February 10, 2020 17:02 marandall@php.net (Mark Randall)
On 10/02/2020 16:18, Paul M. Jones wrote:
> Hi Internals, > Thanks in advance for your time and consideration while evaluating it.
I suspect this is a similar sentinment to the previous version, but I can personally see no major benefit to having this as a core extension. I think the reality is that discussing it "on its own merits" is to completely ignore the wider ecosystem that already performs these functions, with more capabilities, and with potentially hundreds of thousands of implementations already in place. Does it add capabilities which cannot exist in userland or cannot exist in a reasonably performant way? Doesn't seem so except for a readonly property. If not, leave it to userland. Mark Randall
  108439
February 10, 2020 17:32 mike@newclarity.net (Mike Schinkel)
> On Feb 10, 2020, at 11:18 AM, Paul M. Jones <pmjones@pmjones.io> wrote: > > Hi Internals, > > After a couple of years of incubation, I am happy to offer a second version of this RFC: > > https://wiki.php.net/rfc/request_response > > It proposes an object-oriented approach around request and response functionality already existing in PHP, in order to reduce the global-state problems that come with superglobals and the various response-related functions. > > The SQLite "about" page says, "Think of SQLite not as a replacement for Oracle but as a replacement for fopen()." <https://www.sqlite.org/about.html> Likewise, think of this RFC not as a replacement for HttpFoundation or PSR-7, or as a model of HTTP messages, but as an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on. > > Thanks in advance for your time and consideration while evaluating it.
I like this, a lot. Having a language _standard_ approach means that I could depend on it existing and that all (new) userland code I might want to incorporate into a project would be compatible rather than the current state of 100! different incompatible implementations across frameworks. I had numerous questions and concerns, but after reading through the RFC and the associated links I think you addressed most of them. I do have questions about ServerResponse. It is not clear how that interacts with existing echo, header(), setcookie(), etc? You say it creates a buffer; is $responseSender->send($response) effectively the same as having issued regular echo, header(), setcookie(), etc. all at once? And that there is no global buffer but instead the application would have to manage using a shared $response on its own because each instance of $response has it's own buffer and can be sent at any time into the existing output "stream?" Regarding ServerRequest I saw the comment about using a factory instead of creating an instance and your reason for not going that route. How about instead creating an empty mutable object with the constructor and then a method with an optional array parameter that adds the values and "locks" it to be mutable, i.e. $request = new ServerRequest(); $request->initialize(); // OR $request->initialize([ '_SERVER' => [ 'foo' => 'bar', ], ]); With this approach someone could create a class that contains the ServerRequest and build the object up to be anything they like which could be useful for testing, i.e. $request = new ServerRequest(); $request->get = [ 'foo' => 'bar' ]; $request->cookie = [ 'id' => 123 ]; $request->nv = $_ENV; $request->server = $_SERVER; $request->lock(); I would also suggest considering to add get(), post(), request(), cookie(), server() and end() methods to ServerRequest that would incorporate the functionality of filter_input(). Otherwise we have to bypass the objects to get filtering. In general I think this would definitely be a nice enhancement to PHP, IMO. It would be great to be able to deprecate use of $GLOBALS, etc. with a language _standard_ approach, and flag their use as problematic in any of our code. 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.) Thanks in advance for considering. -Mike
  108507
February 12, 2020 12:47 pmjones@pmjones.io ("Paul M. Jones")
Hi Mike,


> I like this, a lot. ... In general I think this would definitely be a nice enhancement to PHP, IMO.
Thanks for saying!
> I do have questions about ServerResponse. It is not clear how that interacts with existing echo, header(), setcookie(), etc? You say it creates a buffer; is $responseSender->send($response) effectively the same as having issued regular echo, header(), setcookie(), etc. all at once?
That's correct. ServerResponse stores the (mutable) state of the response; ServerResponseSender reads from the ServerResponse and calls header(), setcookie(), etc. with its values.
> each instance of $response has it's own buffer and can be sent at any time
That is also correct.
> Regarding ServerRequest ... How about instead creating an empty mutable object with the constructor and then a method with an optional array parameter that adds the values and "locks" it to be mutable, i.e. > > $request = new ServerRequest(); > $request->initialize(); > // OR > $request->initialize([ > '_SERVER' => [ > 'foo' => 'bar', > ], > ]); > > With this approach someone could create a class that contains the ServerRequest and build the object up to be anything they like which could be useful for testing, i.e. > > $request = new ServerRequest(); > $request->get = [ 'foo' => 'bar' ]; > $request->cookie = [ 'id' => 123 ]; > $request->nv = $_ENV; > $request->server = $_SERVER; > $request->lock();
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?
> I would also suggest considering to add get(), post(), request(), cookie(), server() and end() methods to ServerRequest
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. 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.
> 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.
> 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. ;-) I hope that answers all your questions. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108511
February 12, 2020 18:21 mike@newclarity.net (Mike Schinkel)
> 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
  108548
February 13, 2020 18:34 pmjones@pmjones.io ("Paul M. Jones")
Hi Mike,

Thanks for your continued evaluation of the RFC.

> 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. > > ... > > Another option actually would be to allow changes to $_SERVER prior to instantiating ServerRequest() the first time.
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. For example: namespace App; use ServerRequest; class ServerRequestFactory { public function new(array $globals, ?string $content = null) : ServerRequest { // do fixup to $globals['_SERVER'] ... // ... then: return new ServerRequest($globals, $content); } } $request = (new \App\ServerRequestFactory())->new($GLOBALS); I can easily imagine many ways to achieve the same end (i.e., modification of the constructor arguments before instantiating ServerRequest). Do you get where I'm coming from?
>> 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.
Good enough. :-)
> 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);
[snip] I do see what you mean. Even so, I think filtering is out-of-scope for this RFC. 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.
>>> 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);
Even if that's true (and I think that example masks some complexity) I must maintain that it's out of scope for this RFC. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108551
February 13, 2020 20:31 mike@newclarity.net (Mike Schinkel)
> On Feb 13, 2020, at 1:34 PM, Paul M. Jones <pmjones@pmjones.io> wrote: > > Hi Mike, > > Thanks for your continued evaluation of the RFC. > >> 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. >> >> ... >> >> Another option actually would be to allow changes to $_SERVER prior to instantiating ServerRequest() the first time. > > 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. > > For example: > > namespace App; > > use ServerRequest; > > class ServerRequestFactory > { > public function new(array $globals, ?string $content = null) : ServerRequest > { > // do fixup to $globals['_SERVER'] ... > > // ... then: > return new ServerRequest($globals, $content); > } > } > > $request = (new \App\ServerRequestFactory())->new($GLOBALS); > > I can easily imagine many ways to achieve the same end (i.e., modification of the constructor arguments before instantiating ServerRequest). > > 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. 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. 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: >> >> $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); > > [snip] > > 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
  108627
February 16, 2020 23:34 rowan.collins@gmail.com (Rowan Tommins)
On 13/02/2020 20:31, Mike Schinkel wrote:
> 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.
I think you are over-estimating how central the filter API is to most people's workflow with requests. I think that's partly because designing a good validation API is hard, but also because filter_input in particular is a combination of three different concerns: 1) Fetching the raw information about the incoming HTTP request from the web server (the "SAPI") 2) Parsing that raw information into individual fields 3) Validating those fields against expected type constraints The superglobals already combine concerns 1 and 2, and the filter API adds concern 3; but to do so they all assume that the user is basically writing a CGI wrapper for some HTML forms. The modern reality is rather different, and step 2 in particular is much more variable: - Rather than query string parameters, it might involve extracting parameters from an SEO URL like "/products/123-acme-thingummy" or a RESTish URL like "/products/123/description/en-GB" - Rather than submitted form data, it might involve parsing JSON from an AJAX request or API call I would love to see new APIs that take a step back from the legacy, and tackle each of these concerns separately, based on modern requirements. For concern 1, getting data out of the web server, I'd love to see: - A more intuitive way to get the raw request body than file_get_contents('php://input') - A more reliable way to get the URL the user requested than checking 5 different variables in $_SERVER to handle different deployment methods (see e.g. [1] and [2] for the lengths libraries go to for this) - A proper distinction between HTTP headers, server status variables, and environment variables, because CGI name-mangling is legacy cruft that users shouldn't need to learn For concern 2, parsing that data, I'd love to see: - A better API than parse_str for parsing arbitrary strings in application/x-www-form-urlencoded format - A way to parse data in multipart/form-data format decoupled from the current HTTP request - Tools for working with Content-Type strings, such as a function for correctly parsing things like "text/html;charset=UTF-8", and constants for common MIME types Concern 3, filtering / sanitising / validating, I think is a really hard problem space, and I don't think there will ever be one implementation that suits all cases. A similar "shopping list" could probably be made for responses, but if we decoupled the pieces, we wouldn't have to perfect them all at once; instead, we could provide building blocks that make userland implementations easier. [1] https://github.com/symfony/symfony/blob/9acb06041cc88b5c14d40f8cd9a74dd14d7ac786/src/Symfony/Component/HttpFoundation/Request.php#L1741 [2] https://github.com/laminas/laminas-diactoros/blob/b36d6bf376b03dfc3190b1065630090e57f2e20d/src/functions/marshal_uri_from_sapi.php Regards, -- Rowan Tommins (né Collins) [IMSoP]
  108628
February 17, 2020 00:45 mike@newclarity.net (Mike Schinkel)
> On Feb 16, 2020, at 6:34 PM, Rowan Tommins collins@gmail.com> wrote: > > On 13/02/2020 20:31, Mike Schinkel wrote: >> 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. > > I think you are over-estimating how central the filter API is to most people's workflow with requests. I think that's partly because designing a good validation API is hard, but also because filter_input in particular is a combination of three different concerns:
I think your latter points are orthogonal to this. And that you are taking my advocacy for adding filtering to apply too literally to only the specific implementations in filter_input(). I can see addressing your comments below *and* having a filtering method built into these objects. Possibly even with applicable method names as opposed to a 2nd type parameter, like: $request->getInt('db_id'); $request->getJson('package'); $request->getUrl('return_url');
> 1) Fetching the raw information about the incoming HTTP request from the web server (the "SAPI") > 2) Parsing that raw information into individual fields > 3) Validating those fields against expected type constraints > > The superglobals already combine concerns 1 and 2, and the filter API adds concern 3; but to do so they all assume that the user is basically writing a CGI wrapper for some HTML forms. > > The modern reality is rather different, and step 2 in particular is much more variable: > > - Rather than query string parameters, it might involve extracting parameters from an SEO URL like "/products/123-acme-thingummy" or a RESTish URL like "/products/123/description/en-GB" > - Rather than submitted form data, it might involve parsing JSON from an AJAX request or API call > > > I would love to see new APIs that take a step back from the legacy, and tackle each of these concerns separately, based on modern requirements. > > For concern 1, getting data out of the web server, I'd love to see: > > - A more intuitive way to get the raw request body than file_get_contents('php://input') > - A more reliable way to get the URL the user requested than checking 5 different variables in $_SERVER to handle different deployment methods (see e.g. [1] and [2] for the lengths libraries go to for this) > - A proper distinction between HTTP headers, server status variables, and environment variables, because CGI name-mangling is legacy cruft that users shouldn't need to learn > > For concern 2, parsing that data, I'd love to see: > > - A better API than parse_str for parsing arbitrary strings in application/x-www-form-urlencoded format > - A way to parse data in multipart/form-data format decoupled from the current HTTP request > - Tools for working with Content-Type strings, such as a function for correctly parsing things like "text/html;charset=UTF-8", and constants for common MIME types > > Concern 3, filtering / sanitising / validating, I think is a really hard problem space, and I don't think there will ever be one implementation that suits all cases. > > A similar "shopping list" could probably be made for responses, but if we decoupled the pieces, we wouldn't have to perfect them all at once; instead, we could provide building blocks that make userland implementations easier.
Decoupling is a valid approach. But given how much work it is get to an RFC over the line, it feels like decoupling would end up with a lot more work, lengthen the timeline to achieve base level functionality, and add uncertainty to whether it will even happen whereas handling the 20% now that we need 80% of the time would mean the API would be mostly fully usable out of the gate. -Mike
  108635
February 17, 2020 07:51 rowan.collins@gmail.com (Rowan Tommins)
On 17 February 2020 00:45:26 GMT+00:00, Mike Schinkel <mike@newclarity.net> wrote:
>I think your latter points are orthogonal to this. And that you are >taking my advocacy for adding filtering to apply too literally to only >the specific implementations in filter_input().
Firstly, I deliberately didn't say "the filter API isn't well designed", I said "designing a good validation API is hard". In particular, finding the balance between flexibility and simplicity is key. Including a single blessed validation API in something as fundamental as a request object should take a lot of careful design, not be an afterthought to something like the current RFC. I also talked specifically about moving away from the old assumptions of CGI. What does it mean to "filter" a JSON body? We could check it's valid JSON, but parsing it will reveal that anyway. We could automatically parse it in the request object, and have "filters" apply to individual elements; but where would the user supply parser options, and how would you specify nested elements? Or we could keep it as a dumb string, and leave the validation to other systems, like a JSON Schema validator. Even with a plain HTML form, you might be using a form builder and want to associate your validation with the form definition rather than having it baked into the request object.
>But given how much work it is get to an RFC over the line, it feels >like decoupling would end up with a lot more work, lengthen the >timeline to achieve base level functionality, and add uncertainty to >whether it will even happen whereas handling the 20% now that we need >80% of the time would mean the API would be mostly fully usable out of >the gate.
Funny, I see the exact opposite: trying to build a single set of classes which include a system for getting global state AND a system for parsing it in different ways AND an in-built validation API seems like a mammoth task. And if you keep it monolithic, any feature you miss or make a mistake on is much harder to fix later. Regards, -- Rowan Tommins [IMSoP]
  108440
February 10, 2020 20:44 johannes@schlueters.de (Johannes =?ISO-8859-1?Q?Schl=FCter?=)
Hi,

On Mon, 2020-02-10 at 10:18 -0600, Paul M. Jones wrote:
> After a couple of years of incubation, I am happy to offer a second > version of this RFC: > > https://wiki.php.net/rfc/request_response
What indication is there that this will be more successfull than the filter API? The filter API was our previous attempt to improve upon input handling. With this new API we have three major ways (superglobals, filter, new API) Historically PHP isn't really successful in providing higher level APIs to things and I think leaving this to userland is very viable, since composer and performance improvements in later PHP 5 and PHP 7. Also use of $_* is fast to grep for and gives me directly in the grep an idea about the mess-factor of a code base, tracing all calls to a member of an instance of a class is harder. (and yes, references etc. to super globals aren't trivial to trace, but also rare) Let PHP provide the access to the data and promote the API library of the year. johannes
  108510
February 12, 2020 16:54 pmjones@pmjones.io ("Paul M. Jones")
Hi Johannes,


> What indication is there that this will be more successfull than the > filter API?
Fair question. While I can't say how successful (or not) ext/filter has been, I *can* say that the proposal does not present extraordinary or dramatically different ways of working than PHP does currently. The extent of the RFC is only to provide behaviors similar to what PHP already does, in object-oriented dress. That is, whereas ext/filter might have been been described as "a new and different way of working", the concepts and functions in this RFC (even down to the method and property names!) should be immediately familiar even to junior PHP developers.
> Historically PHP isn't really successful in providing higher level APIs > to things and I think leaving this to userland is very viable, since > composer and performance improvements in later PHP 5 and PHP 7.
I addressed the "userland" argument in my response to Mark Randall; please forgive me for not repeating it here.
> Also use of $_* is fast to grep for and gives me directly in the grep > an idea about the mess-factor of a code base, tracing all calls to a > member of an instance of a class is harder. (and yes, references etc. > to super globals aren't trivial to trace, but also rare)
I feel your pain! I do a lot of legacy work too. The `$_` signature makes grepping easy, so that I can find places where there is spooky-action-at-a-distace from globally mutable state. However, ServerRequest is intended to reduce or remove that globally mutable state. The $request->get property is readonly, and neither global nor superglobal. So, while it is tougher to grep for `->get` and know that you have found a ServerRequest property, the *need* to do so should be much less even in legacy codebases.
> Let PHP provide the access to the data and promote the API library of the year.
"API library of the year" -- heh. To be fair, though, the API presented by ServerRequest and ServerResponse is highly similar to that presented by PHP itself; far from being "API of the year" it honors existing PHP functionality quite closely. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108737
February 24, 2020 13:57 johannes@schlueters.de (Johannes =?ISO-8859-1?Q?Schl=FCter?=)
On Wed, 2020-02-12 at 10:54 -0600, Paul M. Jones wrote:
> Hi Johannes, > > > > What indication is there that this will be more successfull than > > the > > filter API? > > Fair question. While I can't say how successful (or not) ext/filter > has been, I *can* say that the proposal does not present > extraordinary or dramatically different ways of working than PHP does > currently. > > The extent of the RFC is only to provide behaviors similar to what > PHP already does, in object-oriented dress. That is, whereas > ext/filter might have been been described as "a new and different way > of working", the concepts and functions in this RFC (even down to the > method and property names!) should be immediately familiar even to > junior PHP developers.
Is there a way to integrate filter, so we can reduce (over the long term) the interfaces and get to a single one (getting rid of register_globals took almost to the day 10 years from creating super globals, via deprecation and changed default to removal, so I'm not asking for the unity tomorrow but having a plan ...) (Or we could say filter failed completely and we don'T integrate it and just remove it :-p no idea ...)
> > Also use of $_* is fast to grep for and gives me directly in the > > grep > > an idea about the mess-factor of a code base, tracing all calls to > > a > > member of an instance of a class is harder. (and yes, references > > etc. > > to super globals aren't trivial to trace, but also rare) > > I feel your pain! I do a lot of legacy work too. The `$_` signature > makes grepping easy, so that I can find places where there is spooky- > action-at-a-distace from globally mutable state. > > However, ServerRequest is intended to reduce or remove that globally > mutable state. The $request->get property is readonly, and neither > global nor superglobal. So, while it is tougher to grep for `->get` > and know that you have found a ServerRequest property, the *need* to > do so should be much less even in legacy codebases.
As a side note: I love the fact that the low layer is mutable. It allows me to take legacy code and wrap it in a more modern framework and fake the old environment as a migration strategy (one has to be aware this produced debt and needs a plan to get rid of that, but can be practical for a migration rather than rewrite, also useful for testing bad code to allow cleanup)
> > Let PHP provide the access to the data and promote the API library > > of the year. > > "API library of the year" -- heh. > > To be fair, though, the API presented by ServerRequest and > ServerResponse is highly similar to that presented by PHP itself; far > from being "API of the year" it honors existing PHP functionality > quite closely.
So it's only "closely" to what users want? So users will still need to wrap it? So purpose is not to make it simpler to use or anything but only to disallow abuses like I mentioned above? I understand why people hate the mutability there, but I think it is a quality of PHP to not stand in the way when crazy things have to be done. And maybe the cases where the mutability can be solved with a FakeServerRequest implementing the same interface easily ... then the question is whether we get users migrated over ... $_GET is in each and every tutorial and code since 2001, when it was introduced in PHP 4.1 .... johannes
  108741
February 24, 2020 17:11 pmjones@pmjones.io ("Paul M. Jones")
Hi Johannes,

Thanks for continuing to evaluate the proposal.

Am I correct in thinking that your primary objection to ServerRequest is that it is read-only? If so, do you have other objections beyond that? And if not, please correct me.

Meanwhile, to respond to your comments ...

* * *

> On Feb 24, 2020, at 07:57, Johannes Schlüter <johannes@schlueters.de> wrote: > > Is there a way to integrate filter, so we can reduce (over the long > term) the interfaces and get to a single one (getting rid of > register_globals took almost to the day 10 years from creating super > globals, via deprecation and changed default to removal, so I'm not > asking for the unity tomorrow but having a plan ...)
My position is that integrating the filter extension is out-of-scope for this RFC. To sum up prior conversation: - Anyone using ext/filter can still do so, just via filter_var() on a ServerRequest property, instead of via filter_input() on the superglobals directly <https://externals.io/message/108436#108507> - "I think you [Mike] are over-estimating how central the filter API is to most people's workflow with requests. I think that's partly because designing a good validation API is hard, but also because filter_input in particular is a combination of three different concerns." <https://externals.io/message/108436#108627> - "[T]rying to build a single set of classes which include a system for getting global state AND a system for parsing it in different ways AND an in-built validation API seems like a mammoth task. And if you keep it monolithic, any feature you miss or make a mistake on is much harder to fix later." <https://externals.io/message/108436#108635>
> (Or we could say filter failed completely and we don'T integrate it and > just remove it :-p no idea ...)
I have no opinion on whether ext/filter has "failed" or not. While I don't see it everywhere all the time, I do see it occasionally; that occasional use is in domain-level work, using filter_var() on properties and parameters, rather than using filter_input().
> As a side note: I love the fact that the low layer is mutable. It > allows me to take legacy code and wrap it in a more modern framework > and fake the old environment as a migration strategy (one has to be > aware this produced debt and needs a plan to get rid of that, but can > be practical for a migration rather than rewrite, also useful for > testing bad code to allow cleanup)
I'm a fan as well; I don't imagine that I would want the superglobals themselves to be made read-only. However, I *do* think that once they pass a certain boundary, they should no longer be mutable. In general, I think that boundary is at the router or other front controller system. The ServerRequest object exists for working at or beyond that boundary; i.e., once you have gotten the superglobals into the state you want, copy them them into a ServerRequest object and pass *that* object around, instead of reading from and writing to the superglobals themselves.
>>> Let PHP provide the access to the data and promote the API library >>> of the year. >> >> "API library of the year" -- heh. >> >> To be fair, though, the API presented by ServerRequest and >> ServerResponse is highly similar to that presented by PHP itself; far >> from being "API of the year" it honors existing PHP functionality >> quite closely. > > So it's only "closely" to what users want?
That's not quite what I said. ;-) To reiterate, what it "closely" does is "honor existing PHP functionality." For example: - instead of reading from `$_SERVER`, read from `$request->server` - instead of calling `header("Foo: bar")`, call `$response->setHeader("Foo", "bar")` - instead of calling `setcookie("baz", "dib")`, call `$response->setCookie("baz", "dib")` That is, it's not a big jump from using the superglobals and functions, to using the object properties and methods. Making the two ways of working very close to each other is a goal of the RFC.
> So users will still need to wrap it?
I wouldn't think they "need" to wrap it, unless they want it to do something it does not already do. I suspect that some will find it satisfactory as-is, and that others will want to add custom functionality.
> So purpose is not to make it simpler to use or anything but only to disallow abuses like I mentioned above?
The purpose is as stated in the RFC: that is, to provide a more "object-oriented approach around request and response functionality already existing in PHP, in order to reduce the global mutable state problems that come with superglobals and the various response-related functions." So it's not that the RFC proposes a "simpler" way to use the superglobals and response-related functions -- it's that the RFC makes it *possible* to address them in a more object-oriented way, while separating them from global mutable state. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108453
February 11, 2020 09:33 jan@horde.org (Jan Schneider)
Zitat von Paul M. Jones <pmjones@pmjones.io>:

> Hi Internals, > > After a couple of years of incubation, I am happy to offer a second > version of this RFC: > > https://wiki.php.net/rfc/request_response > > It proposes an object-oriented approach around request and response > functionality already existing in PHP, in order to reduce the > global-state problems that come with superglobals and the various > response-related functions.
I like this proposal a lot, since it provides a clear, concise interface to these commonly uses, yet inconveniant to use, existing functions and variables without having to always use a full-featured userland library. Speaking of interfaces: since you suggest using decoration and composition over extension for ServerResponse, I am missing a ServerResponseInterface, so that you can easily typehint such userland decorators. -- Jan Schneider The Horde Project https://www.horde.org/
  108471
February 11, 2020 12:42 albertcasademont@gmail.com (Albert Casademont)
This is very interesting, thanks!

Would it make sense to also add an INI setting to disable superglobals and
response functions?


On Tue, Feb 11, 2020 at 10:34 AM Jan Schneider <jan@horde.org> wrote:

> > Zitat von Paul M. Jones <pmjones@pmjones.io>: > > > Hi Internals, > > > > After a couple of years of incubation, I am happy to offer a second > > version of this RFC: > > > > https://wiki.php.net/rfc/request_response > > > > It proposes an object-oriented approach around request and response > > functionality already existing in PHP, in order to reduce the > > global-state problems that come with superglobals and the various > > response-related functions. > > I like this proposal a lot, since it provides a clear, concise > interface to these commonly uses, yet inconveniant to use, existing > functions and variables without having to always use a full-featured > userland library. > Speaking of interfaces: since you suggest using decoration and > composition over extension for ServerResponse, I am missing a > ServerResponseInterface, so that you can easily typehint such userland > decorators. > > -- > Jan Schneider > The Horde Project > https://www.horde.org/ > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
  108476
February 11, 2020 12:59 harry@rhsoft.net ("Reindl Harald (privat)")
Am 11.02.20 um 13:42 schrieb Albert Casademont:
> This is very interesting, thanks! > > Would it make sense to also add an INI setting to disable superglobals and > response functions?
no because changing basic language behavior that way is not helpful for code meant to run everywhere and not stop working just because tomorrow someone changed a random ini setting
  108491
February 11, 2020 17:22 albertcasademont@gmail.com (Albert Casademont)
On Tue, Feb 11, 2020 at 1:59 PM Reindl Harald (privat) <harry@rhsoft.net>
wrote:

> > > Am 11.02.20 um 13:42 schrieb Albert Casademont: > > This is very interesting, thanks! > > > > Would it make sense to also add an INI setting to disable superglobals > and > > response functions? > > no because changing basic language behavior that way is not helpful for > code meant to run everywhere and not stop working just because tomorrow > someone changed a random ini setting >
That could be said for all INI settings: yes they can break things if you touch them without knowing what they do. I think it might make sense to be able to disable superglobals and response functions if your codebase is committed to using the new classes, much like the old "register_globals" did. Why pollute the global namespace if you don't need them?
  108523
February 12, 2020 22:09 pmjones@pmjones.io ("Paul M. Jones")
Hi Albert,

> On Feb 11, 2020, at 11:22, Albert Casademont <albertcasademont@gmail.com> wrote: > >> Am 11.02.20 um 13:42 schrieb Albert Casademont: >> >>> This is very interesting, thanks! >>> >>> Would it make sense to also add an INI setting to disable superglobals >>> and response functions? >> >> no because changing basic language behavior that way is not helpful for >> code meant to run everywhere and not stop working just because tomorrow >> someone changed a random ini setting > > That could be said for all INI settings: yes they can break things if you > touch them without knowing what they do. > > I think it might make sense to be able to disable superglobals and response > functions if your codebase is committed to using the new classes, much like > the old "register_globals" did. Why pollute the global namespace if you > don't need them?
I share Harald's opinion here. I think a .ini setting to disable superglobals and the response-related functions is out-of-scope for this RFC. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108516
February 12, 2020 19:44 pmjones@pmjones.io (Paul M . Jones)
Hi Jan,


> I like this proposal a lot, since it provides a clear, concise interface to these commonly uses, yet inconveniant to use, existing functions and variables without having to always use a full-featured userland library.
Glad to hear the RFC is hitting the right notes!
> Speaking of interfaces: since you suggest using decoration and composition over extension for ServerResponse, I am missing a ServerResponseInterface, so that you can easily typehint such userland decorators.
I am not opposed to a ServerResponseInterface, if John Boehr is not -- though at the same time, I am wary of expanding the number of new declarations too much. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108489
February 11, 2020 16:58 ben@benramsey.com (Ben Ramsey)
> On Feb 10, 2020, at 10:18, Paul M. Jones <pmjones@pmjones.io> wrote: > > Hi Internals, > > After a couple of years of incubation, I am happy to offer a second version of this RFC: > > https://wiki.php.net/rfc/request_response > > It proposes an object-oriented approach around request and response functionality already existing in PHP, in order to reduce the global-state problems that come with superglobals and the various response-related functions. > > The SQLite "about" page says, "Think of SQLite not as a replacement for Oracle but as a replacement for fopen()." <https://www.sqlite.org/about.html> Likewise, think of this RFC not as a replacement for HttpFoundation or PSR-7, or as a model of HTTP messages, but as an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on. > > Thanks in advance for your time and consideration while evaluating it.
Regarding the array of arrays for $accept* and $forwarded, what are your thoughts on using value objects with properties, rather than arrays with keys? AcceptValue * string $value * string $quality * array $params * ?string $type * ?string $subtype ForwardedValue * string $by * string $for * string $host * string $proto Cheers, Ben
  108527
February 13, 2020 02:08 pmjones@pmjones.io (Paul M . Jones)
Hi Ben,


> On Feb 11, 2020, at 10:58, Ben Ramsey <ben@benramsey.com> wrote: > > Regarding the array of arrays for $accept* and $forwarded, what are your thoughts on using value objects with properties, rather than arrays with keys?
It's a fair suggestion, but I am not keen to expand the number of new declarations any more than necessary. To play with your idea a little bit, let's say we start with ... - ServerRequest - ServerResponse - ServerResponseSender .... then to bring in Jan Schneider's suggestion, we add ServerResponseInterface (4 classes). Then we add the value objects themselves: ServerRequestAcceptValue and ServerRequestForwardValue. That's 6 classes. That's maybe not so bad. But, once we start down this path, it's easy for me to imagine what comes after. For example, since we're trying to do "the right thing," someone will suggest to typehint the properties "correctly." Instead of arrays of value objects, they probably ought to be collection objects. That gets us ServerRequestAcceptValueCollection and ServerRequestForwardValueCollection, and we're at 8 classes. Then someone is likely to say that there ought to be interfaces for all those so their implementations can get swapped out. That's ... - ServerRequest - ServerResponse - ServerResponseInterface - ServerResponseSender - ServerRequestAcceptValue - ServerRequestAcceptValueInterface - ServerRequestAcceptValueCollection - ServerRequestAcceptValueCollectionInterface - ServerRequestForwardValue - ServerRequestForwardValueInterface - ServerRequestForwardValueCollection - ServerRequestForwardValueCollectionInterface .... 12 classes and interfaces. And probably not done yet, once others really sink their teeth into it. Now, that's not a *wrong* approach -- but it does seem like overkill to me. Or, we could avoid starting down that path and stick with arrays, which in ServerRequest are read-only and immutable and get us all the things we actually need for our daily work here. Of those two, I prefer the minimalist approach of arrays for this RFC; the "effort-to-benefit" ratio is much better. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108493
February 11, 2020 18:30 pmjones@pmjones.io (Paul M . Jones)
Hi Mark,

> On Feb 10, 2020, at 11:02, Mark Randall <marandall@php.net> wrote: > > I suspect this is a similar sentinment to the previous version, but I can personally see no major benefit to having this as a core extension. > > I think the reality is that discussing it "on its own merits" is to completely ignore the wider ecosystem that already performs these functions, with more capabilities, and with potentially hundreds of thousands of implementations already in place. > > Does it add capabilities which cannot exist in userland or cannot exist in a reasonably performant way? Doesn't seem so except for a readonly property. > > If not, leave it to userland.
I completely understand that sentiment; I recognize that it is shared by others on this list and elsewhere. But if you'll allow me, I'd like to present a counterargument in relation to previous PHP extensions. When ext/pdo was added to core, there was already a "wider ecosystem that already performs these functions, with more capabilities, and with potentially hundreds of thousands of implementations already in place." Some of those implementations at the time included (I'm working from memory here) AdoDB, Metabase, MDB, PEAR_DB, and many more that I cannot recall. PDO did not (to my knowledge) "add capabilities which cannot exist in userland or cannot exist in a reasonably performant way". (I'll grant that FETCH_INTO_OBJECT setting properties directly without using the constructor was not possible in userland, but that's an exception that tests the rule.) Indeed, PDO had a relatively reduced feature set in comparison to some of those userland libraries, especially AdoDB. And yet, PDO has turned out to be of great benefit, because it brought together features into core that (figuratively speaking) everybody needed and was rewriting in userland over and over. PDO is the strongest example here, but depending on how you count, there are 2-3 other extensions that also serve: ext/date, ext/phar, and (reaching back to antiquity) ext/session. So, there is a long history of widely-needed userland functionality being brought into core. I would say this RFC is a pretty tame example of doing so; the proposal presented here is very similar to the way PHP itself already does things, just wrapped in object properties and methods, and is very similar to how things are being done across a wide swath of userland. Now, it is possible that the objections you raise *should* have prevented PDO (et al.) from going into core. If that is the case, and (in hindsight) we think it was a mistake to allow them, then consistency alone makes your objections valid here as well. However, if (in hindsight) it was *not* a mistake to allow those extensions, then the raised objections are not especially strong arguments against this RFC. That's not to say "because PDO was allowed into core, this RFC must therefore be allowed into core" but to say "those objections alone were not a barrier to PDO, so they alone should not be a barrier to this RFC". I hope that's an understandable counterargument, even if you don't agree with it. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108512
February 12, 2020 18:20 me@kelunik.com (Niklas Keller)
> Hi Internals, > > After a couple of years of incubation, I am happy to offer a second version of this RFC: > > https://wiki.php.net/rfc/request_response > > It proposes an object-oriented approach around request and response functionality already existing in PHP, in order to reduce the global-state problems that come with superglobals and the various response-related functions. > > The SQLite "about" page says, "Think of SQLite not as a replacement for Oracle but as a replacement for fopen()." <https://www.sqlite.org/about.html> Likewise, think of this RFC not as a replacement for HttpFoundation or PSR-7, or as a model of HTTP messages, but as an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on. > > Thanks in advance for your time and consideration while evaluating it.
Hey Paul, I think the request / response API is entirely fine being solved in userland instead of in php-src. However, since you already made such a proposal, I want to give some feedback: Naming I think we shouldn't take over the naming of the super globals, e.g. $_GET really contains the query parameters and has nothing to do with GET or POST, so $request->getQueryParameter(...) would be a better name. Type Safety I think the API should be type safe. Currently $request->get['key'] can be null | string | string[] | ... Most parameters only appear a single time, so a method returning the first parameter value or null could be used instead. API Issues I don't see any reason to keep specials such as $_SERVER['HTTP_X_REQUESTED_WITH'] vs. $request->requestedWith, which is just another HTTP header. If there's $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] => $request->method and $_SERVER['REQUEST_METHOD'] => $request->method, how can I get the original (actual) method? Given 'echo $content; => $response->setContent($content);', shouldn't this rather be something like `addContent()`? How does streaming responses work? There's ServerResponseSender, but I think this should be part of the Response API. The Request object should be mutable, e.g. you might want to change the client IP to be based on a x-forwarded-header instead of the TCP source address. Other Commentary
> A: It supports async exactly as much as PHP itself does.
Not really. PHP has built-in stream_select / non-blocking streams, so supports the tools required for async out of the box. Regards, Niklas
  108513
February 12, 2020 18:39 mike@newclarity.net (Mike Schinkel)
> On Feb 12, 2020, at 1:20 PM, Niklas Keller <me@kelunik.com> wrote: > I think the request / response API is entirely fine being solved in > userland instead of in php-src.
I want to disagree in support of this RFC. While is *has* been solved in userland, but it has been solved 25+ times in 25+ different incompatible ways. This RFC would allow for a *single* solution that is compatible across all frameworks, and frameworks could create a shim to support their existing implementations. And given that it would be in core vs. userland, this would *not* apply: https://xkcd.com/927/ The one concern should be if there is anything about this RFC that would make writing such a shim impossible for any framework vendor?
> I think we shouldn't take over the naming of the super globals, e.g. > $_GET really contains the query parameters and has nothing to do with > GET or POST, so $request->getQueryParameter(...) would be a better > name.
Please no. That is all too Java-esque verbosity. Very commonly used functionality can afford to be concise like $request->get() because it would be well-known and not have to compete with lots of other functionality for mindshare. Thus no need for this to be so precise and have such long names when everyone writing PHP would soon lean what it means.
> I don't see any reason to keep specials such as > $_SERVER['HTTP_X_REQUESTED_WITH'] vs. $request->requestedWith, which > is just another HTTP header.
Reason: $request->requestedWith can be type-safe, something you asked for in another context. $_SERVER['HTTP_X_REQUESTED_WITH'] cannot be type safe without special cases in the language.
> The Request object should be mutable, e.g. you might want to change > the client IP to be based on a x-forwarded-header instead of the TCP > source address.
I totally agree it needs to be mutable, but only prior to first use. After the first use having it be locked would gain us the benefits of robustness that immutability can provide. -Mike
  108520
February 12, 2020 21:40 php.lists@allenjb.me.uk (AllenJB)
On 12/02/2020 18:20, Niklas Keller wrote:
> Type Safety > > I think the API should be type safe. Currently $request->get['key'] > can be null | string | string[] | ... Most parameters only appear a > single time, so a method returning the first parameter value or null > could be used instead.
I may be misunderstanding what you're suggesting here, but it seems you're proposing to have ->get['key'] only return null|string, getting rid of the arrays of parameter values ( ?key[]=value1&key[]=value2 ) functionality here. (And I assume you intend the same for ->post['key'] ) If so, I would say this is a bad idea. I and a lot of other devs use this functionality frequently (more often in POST, but often enough in GET for purposes such as search pages). And how would multiple select inputs be handled? AllenJB
  108534
February 13, 2020 10:52 come.chilliet@fusiondirectory.org (=?ISO-8859-1?Q?C=F4me?= Chilliet)
Le mercredi 12 février 2020, 19:20:56 CET Niklas Keller a écrit :
> Naming > > I think we shouldn't take over the naming of the super globals, e.g. > $_GET really contains the query parameters and has nothing to do with > GET or POST, so $request->getQueryParameter(...) would be a better > name.
I think this remark is really on point. GET and POST are HTTP methods and not ways of passing data. You can have query parameters on any request, and you can have POST data with a lot of other HTTP methods, as is commonly used in REST APIs. I do not like the name getQueryParameter at first sight, but I do not like get() and post() for getting values from $_GET and $_POST. I would expect a method named post to actually post something, not just get a value. -- Côme Chilliet FusionDirectory - https://www.fusiondirectory.org
  108535
February 13, 2020 11:23 me@ekins.space ("Ekin B.")
Hi all,

> On February 13, 2020 1:52 PM Côme Chilliet chilliet@fusiondirectory.org> wrote: > > > Le mercredi 12 février 2020, 19:20:56 CET Niklas Keller a écrit : > > Naming > > > > I think we shouldn't take over the naming of the super globals, e.g. > > $_GET really contains the query parameters and has nothing to do with > > GET or POST, so $request->getQueryParameter(...) would be a better > > name. > > I think this remark is really on point. > GET and POST are HTTP methods and not ways of passing data. You can have query parameters on any request, and you can have POST data with a lot of other HTTP methods, as is commonly used in REST APIs. > > I do not like the name getQueryParameter at first sight, but I do not like get() and post() for getting values from $_GET and $_POST. > I would expect a method named post to actually post something, not just get a value. > > -- > Côme Chilliet > FusionDirectory - https://www.fusiondirectory.org > > --
I agree that this is indeed on point. Personally I would like $request->getQueryParameter() and $request->getBody() over $request->get() and $request->post(). In the former case you are asking for the parameters in the query string (query component of the URI) and the latter is to get the request body. These method names make it very clear what you're asking for. What kind of bothers me in general about this RFC is that... we are talking about Request and Response and these are standard HTTP components, yet the RFC is about providing wrappers for existing global PHP variables and functions. It does not try to model these components as they are in their standard, nor aim to do so. This is why I believe it's better to leave it to userland implementations. Besides the fact that they are widely adopted, they attempt to follow the specification they are providing. Regards, Ekin H. Bayar https://ekins.space https://heap.space
  108581
February 14, 2020 15:47 pmjones@pmjones.io ("Paul M. Jones")
Hi Côme & Niklas,

> On Feb 13, 2020, at 04:52, Côme Chilliet chilliet@fusiondirectory.org> wrote: > > Le mercredi 12 février 2020, 19:20:56 CET Niklas Keller a écrit : > >> Naming >> >> I think we shouldn't take over the naming of the super globals, e.g. >> $_GET really contains the query parameters and has nothing to do with >> GET or POST, so $request->getQueryParameter(...) would be a better >> name. > > I think this remark is really on point. > GET and POST are HTTP methods and not ways of passing data. You can have query parameters on any request, and you can have POST data with a lot of other HTTP methods, as is commonly used in REST APIs.
Your comments on naming are well-made. While working on the implementation, we tried out $query instead of $get, on exactly the premise that you state: i.e., that `$_GET` holds the query parameters, and has nothing to do with the GET method. But in the end, we settled on mapping more directly from `$_GET` => `$get`, and `$_POST => $post`. Having said that, we are willing to revisit that naming decision if there's support for doing so. Perhaps: - rename $get to $query, populating it from `$globals['_GET']`, on the basis stated above - rename $post to $input, populating it from `$globals['_POST']`, on the basis that it typically relates to the parsed form of php://input Your (and/or anyone else's) thoughts on that? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108582
February 14, 2020 16:03 benjamin.morel@gmail.com (Benjamin Morel)
> > Having said that, we are willing to revisit that naming decision if > there's support for doing so. Perhaps: > - rename $get to $query, populating it from `$globals['_GET']`, on the > basis stated above > - rename $post to $input, populating it from `$globals['_POST']`, on the > basis that it typically relates to the parsed form of php://input > Your (and/or anyone else's) thoughts on that?
Symfony uses $request->query and $request->request for $_GET and $_POST, respectively. I like $request->query for query params, but always found $request->request for body parameters confusing. I like $request->input a bit better, although I'd be interested to hear more ideas on this one. — Benjamin On Fri, 14 Feb 2020 at 16:47, Paul M. Jones <pmjones@pmjones.io> wrote:
> Hi Côme & Niklas, > > > On Feb 13, 2020, at 04:52, Côme Chilliet < > come.chilliet@fusiondirectory.org> wrote: > > > > Le mercredi 12 février 2020, 19:20:56 CET Niklas Keller a écrit : > > > >> Naming > >> > >> I think we shouldn't take over the naming of the super globals, e.g. > >> $_GET really contains the query parameters and has nothing to do with > >> GET or POST, so $request->getQueryParameter(...) would be a better > >> name. > > > > I think this remark is really on point. > > GET and POST are HTTP methods and not ways of passing data. You can have > query parameters on any request, and you can have POST data with a lot of > other HTTP methods, as is commonly used in REST APIs. > > Your comments on naming are well-made. > > While working on the implementation, we tried out $query instead of $get, > on exactly the premise that you state: i.e., that `$_GET` holds the query > parameters, and has nothing to do with the GET method. But in the end, we > settled on mapping more directly from `$_GET` => `$get`, and `$_POST => > $post`. > > Having said that, we are willing to revisit that naming decision if > there's support for doing so. Perhaps: > > - rename $get to $query, populating it from `$globals['_GET']`, on the > basis stated above > - rename $post to $input, populating it from `$globals['_POST']`, on the > basis that it typically relates to the parsed form of php://input > > Your (and/or anyone else's) thoughts on that? > > > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com > > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp > > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > >
  108584
February 14, 2020 16:44 weirdan@gmail.com (Bruce Weirdan)
On Fri, Feb 14, 2020 at 5:47 PM Paul M. Jones <pmjones@pmjones.io> wrote:
>
> - rename $get to $query, populating it from `$globals['_GET']`, on the basis stated above > - rename $post to $input, populating it from `$globals['_POST']`, on the basis that it typically relates to the parsed form of php://input
What about $query and $body? That would be closer to the terminology used in HTTP RFCs. -- Best regards, Bruce Weirdan mailto:weirdan@gmail.com
  108585
February 14, 2020 16:47 benjamin.morel@gmail.com (Benjamin Morel)
> > What about $query and $body? That would be closer to the terminology > used in HTTP RFCs.
The problem is that $body is typically used to get the raw message body as a string or stream. I was thinking more something along the lines of $bodyParams, which is more verbose but leaves no ambiguity: *$queryParams* and *$bodyParams*. — Benjamin
  108589
February 14, 2020 22:12 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

> On Feb 14, 2020, at 10:47, Benjamin Morel morel@gmail.com> wrote: > >> >> What about $query and $body? That would be closer to the terminology >> used in HTTP RFCs. > > > The problem is that $body is typically used to get the raw message body as > a string or stream. > > I was thinking more something along the lines of $bodyParams, which is more > verbose but leaves no ambiguity: *$queryParams* and *$bodyParams*.
I get the desire to disambiguate. But as an added consideration, there's a desire for consistency; when adding a -Params suffix to those names, it might then make sense to have $serverParams, $cookieParams, etc. Looking at it that way, I don't think a -Params suffix is necessary. I would think $query would be enough. As for the other name, the one for the $_POST equivalent, $body doesn't seem quite right to me; it seems a little close to $content. I've also been thinking about $values, $params, $parsedContent, $contentValues, $bodyValues, $contentArray, and other variations with and without prefixes and suffixes, but $input is the one that feels like the least-terrible alternative to $post for me, esp. given the connection to php://input. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108591
February 15, 2020 08:01 larry@garfieldtech.com ("Larry Garfield")
On Fri, Feb 14, 2020, at 10:47 AM, Benjamin Morel wrote:
> > > > What about $query and $body? That would be closer to the terminology > > used in HTTP RFCs. > > > > The problem is that $body is typically used to get the raw message body as > a string or stream. > I was thinking more something along the lines of $bodyParams, which is more > verbose but leaves no ambiguity: *$queryParams* and *$bodyParams*. > > — Benjamin
Data point: In PSR-7, the names used are: - queryParams: The query string values. - parsedBody: The body of the message, converted to a meaningful value. If the request type is a form, then it MUST be equivalent to $_POST. If not, it's up to the particular implementation to determine what "parsed" means. (Eg, parsing a JSON body of a POST into some domain object, or whatever.) - The raw body is a stream called "body", or rather an OOP wrapper around a stream since PHP's native stream interface is fugly. - There's specific handling for uploadedFiles, too. cf: https://www.php-fig.org/psr/psr-7/ To the earlier point about existing implementations, while there are a myriad of older, custom implementations of abstractions around superglobals, there's only two that are not decade-old proprietary implementations: HttpFoundation and PSR-7. Those are, realistically, the only implementations that matter. Anything else would be on the same order of magnitude effort to port to one of those as to port to this proposal. In a meaningful sense, those are the only "existing competition". Both also have robust ecosystems that make leveraging them in an entirely custom app pretty straightforward. (Whatever your feelings of the technical merits of either design, that's the current state-of-the-market.) Which therefore begs the question, is this proposal intended to supplant HttpFoundation and PSR-7, or to become a common underpinning that both of them wrap, or to be a third cohabitating implementation in the ecosystem? It doesn't seem robust enough to supplant both of them entirely, there's little value to either HttpFoundation or PSR-7 to rewrite their guts to wrap this object (though it's easier for PSR-7, as an interface, for someone to write a new implementation of it than for HttpFoundation), which would mean we'd end up with a 3rd in-the-wild implementation for user space to keep track of. I am unclear how that is a market win. PDO was mentioned previously as a model. Yes, there were many user-space implementations prior to PDO. PDO effectively supplanted and obsoleted them. However... PDO was also never thought-through enough or robust enough to be used directly, spawning a whole new ecosystem of PDO++ libraries that people actually use (Doctrine, Eloquent, Drupal's DBTNG, Aura..sql, to name but a few). So, not the full win people were hoping for. If that pattern holds, we'd end up with... a new generation of this-RFC++ wrappers that still abstract it away yet aren't compatible with each other. That said, PDO did have the advantage of at least partially unifying disparate SQL APIs. There really aren't multiple incompatible HTTPs to abstract over the way there is for SQL backends, so the analogy is somewhat weak either way. --Larry Garfield
  108607
February 15, 2020 20:10 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

> On Feb 15, 2020, at 02:01, Larry Garfield <larry@garfieldtech.com> wrote: > > ... is this proposal intended to supplant HttpFoundation and PSR-7 ... ?
This is question is answered in the RFC introduction; quoting from there: The SQLite “about” page says, “Think of SQLite not as a replacement for Oracle but as a replacement for fopen().” https://www.sqlite.org/about.html Likewise, think of this RFC not as a replacement for HttpFoundation or PSR-7, or as a model of HTTP messages, but as an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on.
> PDO was mentioned previously as a model.
I did not mention PDO as "a model". I mentioned PDO (along with other extensions) to illustrate a counter-argument to objections based on the availability and comparability of userland implementations. The counter-argument summary was: That's not to say "because PDO was allowed into core, this RFC must therefore be allowed into core" but to say "those objections alone were not a barrier to PDO, so they alone should not be a barrier to this RFC". The argument, and my counter-argument, are here: <https://externals.io/message/108436#108493> -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108613
February 16, 2020 01:37 rowan.collins@gmail.com (Rowan Tommins)
On 15 February 2020 20:10:30 GMT+00:00, "Paul M. Jones" <pmjones@pmjones.io> wrote:
>Hi all, > >> On Feb 15, 2020, at 02:01, Larry Garfield <larry@garfieldtech.com> >wrote: >> >> ... is this proposal intended to supplant HttpFoundation and PSR-7 >... ? > >This is question is answered in the RFC introduction
You've cut Larry's question in half there, and in doing so made it seem like a repeat, when it is not. The second half of the sentence is this:
> ...or to become a common underpinning that both of them wrap, or to be a third cohabitating implementation in the ecosystem?
I haven't seen you answer that part yet: do you expect existing userland libraries to migrate from wrapping $_GET etc to using these built-in wrappers. If so, what benefit does it bring those libraries? If not, who is its intended audience? You said previously:
> PDO did not (to my knowledge) "add capabilities which cannot exist in userland or cannot exist in a reasonably performant way".
I think this is a misjudgement, and a relevant one. PDO didn't take functionality that existed purely in userland and migrate it to an extension; it took functionality that was scattered across a dozen different vendor-specific extensions with different naming and calling conventions, and centralised it into one more-or-less consistent interface. In doing so, it made (or at least tried to make) life easier both for database vendors, who can provide a PDO driver and fit into the ecosystem, and library developers, who can use PDO as a base and have less vendor-specific code. Your other examples - date, phar, and session - took common problems that were possible to solve in userland but tricky to solve well, and provided a standard out-of-the-box implementation. We already have a unified out-of-the-box implementation for the problem "get data out of HTTP request", in the form of superglobals, so neither comparison seems apt. A better comparison might be to features which have been reimplemented multiple times, to fix fundamental problems. A recent example is __serialize, but interestingly $_GET et al are themselves the third implementation of the feature, after "register globals" and the $HTTP_* arrays. As far as I can see, the RFC mentions two things it fixes about the current implementation: - The current implementation is not OO. That's not really surprising, since PHP is not a purely OO language, and treats OO as a matter of style - to the extent of providing hybrid object-procedural APIs like date and mysqli. - The current implementation is based on global state. This is definitely something that would be good to fix, but you can do almost as much in that direction as the RFC by writing "$get=$_GET; unset($_GET);" The hard problem is that the entry point for a request is in global scope, not a main() or handleRequest() function. Introducing these objects as part of a new calling convention for PHP scripts would definitely add value, and make them a true replacement for the superglobals, but that would be a very different RFC. However well designed this extension is within itself, I think it needs a stronger description of who should use it and why. Regards, -- Rowan Tommins [IMSoP]
  108650
February 17, 2020 15:55 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

I apologize in advance for the length of this email. I hate reading walls-of-text, but the answers are necessarily long. I have tried to break it up into bullets where possible for easier reading.


> On Feb 15, 2020, at 19:37, Rowan Tommins collins@gmail.com> wrote: > > On 15 February 2020 20:10:30 GMT+00:00, "Paul M. Jones" <pmjones@pmjones.io> wrote: > >> Hi all, >> >>> On Feb 15, 2020, at 02:01, Larry Garfield <larry@garfieldtech.com> >>> wrote: >>> >>> ... is this proposal intended to supplant HttpFoundation and PSR-7 >>> ... ? >> >> This is question is answered in the RFC introduction > > You've cut Larry's question in half there, and in doing so made it seem like a repeat, when it is not. The second half of the sentence is this: > >> ...or to become a common underpinning that both of them wrap, or to be a third cohabitating implementation in the ecosystem? > > I haven't seen you answer that part yet: do you expect existing userland libraries to migrate from wrapping $_GET etc to using these built-in wrappers. If so, what benefit does it bring those libraries? If not, who is its intended audience?
I really did think the answers to these were obvious, or easily-inferred, but obviously I was wrong. I will attempt to expand. Q: "Do you expect existing userland libraries to migrate ... ?" A: To be clear, I don't *expect* anything from existing published library authors; however ... - I *suspect* that some will choose to ignore this extension, - that others will decorate or extend it, - and that still others may find their own work so close to this extension that they migrate over to it entirely. Further ... - I suspect that developers of in-house unpublished request/response objects may find this useful for their own purposes, - and that developers who are using $_GET, header(), etc. will be pleased to find an OO-ish system that operates much like PHP itself already does, easing their transition away from global state. Finally ... - I suspect that some *consumers* of existing libraries will feel this extension is not their preferred way of working, and continue on with whatever libraries they already use, - while other consumers of those libraries will prefer this extension in their place. On reading over this, I suppose I do have an "expectation" of library authors and their consumers: that as good engineers they will evaluate this extension in reference to their own circumstances, and choose to use it (or not use it) based on the tradeoffs they find for their situation. Q: "What benefit does it bring those libraries?" A: Whatever benefits their authors happen to see in it. For myself, and as noted by Jan Schneider and others, those benefits center around having a built-in OO-ish request/response object set that does pretty much just what PHP itself already does, and that is well-suited to our daily work, without needing to incorporate comparatively large libraries into our projects for what we consider to be relatively simple purposes -- those purposes being "reading the request inputs" and "sending the response outputs". Q: "Who is its intended audience?" A: I always thought of the "intended audience" as the much the same as for any RFC: that is, developers working on a website who want PHP to provide a reasonable set of functionality for doing so, request/response objects being part of that set in a language as closely related to the web as PHP is. (As a fun side note: for all its apparent simplicity, this is an extraordinary question. Unless I have missed something, I don't recall it being asked directly of any RFC before. A Google search against wiki.php.net and externals.io reveals a double-handful of results on the terms "intended audience" and "target audience", but the only even indirect question about it on an RFC is in relation to the JIT, here: <https://externals.io/message/103903#103995> -- and it's by you. :-) * * * Rowan, you quoted Larry asking if this extension was ...
>> to be a third cohabitating implementation in the ecosystem?
.... that is, third after HttpFoundation and PSR-7. (By way of preamble, and to reiterate what I've said before: I would prefer to discuss this RFC on its own terms. But, as you and others want some discussion in context of other projects, I will do so. My apologies if I sound negative or critical toward those projects.) It turns out the question is kind of a funny way of describing the situation, in that PSR-7 is not an implementation of anything. (Full disclosure: I was a sponsor on the PSR-7 proposal.) PSR-7 is instead a set of interfaces; that is, they "are abstractions around HTTP messages and the elements composing them." <https://www.php-fig.org/psr/psr-7/> The meta document notes, "the goal of this proposal is not to obsolete the current interfaces utilized by existing PHP libraries. This proposal is aimed at interoperability between PHP packages for the purpose of describing HTTP messages." <https://www.php-fig.org/psr/psr-7/meta/> So, to refer to PSR-7 as a "cohabiting implementation" is a bit mistaken (but a very easy mistake to make -- I imagine I have made it myself occasionally). If we are going to talk about implementations in an ecosystem, then, there are at least four for PSR-7: - Cake - Guzzle - Slim - Zend/Laminas There are others as well, with varying ranges of adoption; and, as PSR-7 is an interoperability specification, I predict there will be more implementations later. The above implementations are partially but not always fully interchangeable; while they adhere to the PSR-7 method interfaces, they each have their own behavioral idiosyncrasies. In some cases they offer additional methods and behaviors that the others do not have. (This is not a criticism, merely an observation.) Thus, if one wishes to speak in terms of "cohabiting implementations", ext/request would have to be counted as one of at least six (i.e., itself, HttpFoundation, and the above four). I don't know if that strengthens or weakens the ext/request case, but it does help to clarify the world in which it exists. * * * Rowan, you wrote:
> You said previously: > >> PDO did not (to my knowledge) "add capabilities which cannot exist in userland or cannot exist in a reasonably performant way". > > I think this is a misjudgement, and a relevant one. PDO didn't take functionality that existed purely in userland and migrate it to an extension; it took functionality that was scattered across a dozen different vendor-specific extensions with different naming and calling conventions, and centralised it into one more-or-less consistent interface. In doing so, it made (or at least tried to make) life easier both for database vendors, who can provide a PDO driver and fit into the ecosystem, and library developers, who can use PDO as a base and have less vendor-specific code.
That is one valid way of looking at it; but then, on exactly the same terms, it is likewise valid to say that AdoDB "took functionality that was scattered across a dozen different vendor-specific extensions with different naming and calling conventions, and centralised it into one more-or-less consistent interface." So did PEAR DB, Metabase, MDB, and so on. PDO did it as an extension, instead of in userland, but the goals and outcomes noted were identical. I find this more confirmatory than not regarding my earlier counter-argument; i.e., that (in hindsight) objections based on the existence and availability of userland projects would not themselves have been a barrier to PDO, and so should not themselves be a barrier to this RFC.
> Your other examples - date, phar, and session - took common problems that were possible to solve in userland but tricky to solve well, and provided a standard out-of-the-box implementation. > > We already have a unified out-of-the-box implementation for the problem "get data out of HTTP request", in the form of superglobals, so neither comparison seems apt.
Perhaps; working by analogy is always a difficult and imperfect task.
> A better comparison might be to features which have been reimplemented multiple times, to fix fundamental problems. A recent example is __serialize, but interestingly $_GET et al are themselves the third implementation of the feature, after "register globals" and the $HTTP_* arrays. > > As far as I can see, the RFC mentions two things it fixes about the current implementation: > > - The current implementation is not OO. That's not really surprising, since PHP is not a purely OO language, and treats OO as a matter of style - to the extent of providing hybrid object-procedural APIs like date and mysqli.
"As a matter of style" is a good line! In support of that line, this RFC does not seek to remove $_GET and the response-related functions, but rather to provide an additional OO-ish approach that honors the existing (as you say, the third-time's-the-charm) language-level approaches. FWIW, I had considered mentioning the dual OO+procedural APIs of mysqli, date, etc. but it seemed too much in an already-long RFC; I'm glad you brought it up, as I did have it in mind.
> - The current implementation is based on global state. This is definitely something that would be good to fix, but you can do almost as much in that direction as the RFC by writing "$get=$_GET; unset($_GET);" The hard problem is that the entry point for a request is in global scope, not a main() or handleRequest() function.
Another "hard" problem is carrying those values around in the system once they are decoupled from global state; the objects in this RFC purport to do so.
> Introducing these objects as part of a new calling convention for PHP scripts would definitely add value, and make them a true replacement for the superglobals, but that would be a very different RFC.
That does seem rather ambitious. If you feel that's a valuable thing to add to PHP, perhaps it could be part of a future RFC, maybe even one that uses the objects in this RFC as a starting point. * * * Again, my apologies for the wall-of-text. Please let me know if you find those answers are sufficient or not. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108661
February 18, 2020 20:21 rowan.collins@gmail.com (Rowan Tommins)
On 17/02/2020 15:55, Paul M. Jones wrote:
> I apologize in advance for the length of this email. I hate reading walls-of-text, but the answers are necessarily long. I have tried to break it up into bullets where possible for easier reading.
No, thank you for taking the time to respond. I've cherry-picked rearranged the quotes in this reply a bit so I can try not to repeat myself too much; let me know if I've taken your words too far out of context.
> ...it is likewise valid to say that AdoDB "took functionality that was scattered across a dozen different vendor-specific extensions with different naming and calling conventions, and centralised it into one more-or-less consistent interface." So did PEAR DB, Metabase, MDB, and so on. PDO did it as an extension, instead of in userland, but the goals and outcomes noted were identical.
That's a fair point, everything PDO does could be and has been done in userland. There is one thing that couldn't be done outside of an extension, though, which is that PDO doesn't rely on the legacy extensions, it (in theory at least) replaces them. What I think is slightly more relevant, is that PDO makes writing wrappers like ADODB easier, because it contains non-trivial functionality that those wrappers would otherwise have to write.
> I don't *expect* anything from existing published library authors > ... > I always thought of the "intended audience" as the much the same as > for any RFC
I think perhaps my choice of words has caused a bit of confusion. Perhaps "hope" would have been better than "expect", and "use cases" better than "intended audience". I was trying to draw out two pieces of information: - What's the sales pitch - what do YOU think is great about these classes? - When working out the details, what code should we be picturing using the new classes? I wasn't around when PDO was proposed, but an imaginary "sales pitch" might have gone something like this: - Developers should be able to implement multiple database systems without learning the quirks of each vendor's extension - Library authors shouldn't need to implement the basic functionality of every driver from scratch when the vendor could do it for them - Vendors shouldn't need to decide which API to follow, when we can normalise everything internally - We can offer different result formats and error-handling models out of the box ....and so on
> - some [library authors] may find their own work so close to this extension that they migrate over to it entirely.
So, there is at least some hope that this will entirely replace some people's existing libraries, even if it doesn't replace the more powerful ones like HttpFoundation. That's probably reasonable. (Note that I've been thinking of "library" fairly loosely - a single class used in a completely private monolithic repo, but which is written to be generic functionality, has much the same role as a public composer package in this case.)
> For myself, and as noted by Jan Schneider and others, those benefits center around...
This is what I was looking for. Sell it to me! :)
> ...a built-in OO-ish request/response object set...
"OO-ish" is a wise choice of words here. One of the reasons I'm not terribly keen on this proposal - particularly the request half - is that I'm a bit of an OO purist. By that I mean that I prefer objects that have a strong responsibility, and encapsulate useful behaviour, rather than just spitting out what went in. The response part of the proposal is closer to "real" OO, IMO, because it includes behaviour like manipulating individual headers and cookies. The lack of behaviour also makes it less useful to people writing their own request and response objects: if I have a copy of $_POST and want to put it in my own object property, why would I first pass it to a ServerRequest object, and then get it straight back out again, if the object isn't helping me do anything with that data? That said, I know some people are happy with OO-ish code, some even to the extent of preferring stdClass to an array. So it's not unreasonable to say that this will appeal to developers who are less puritanical about objects than me.
> ...that does pretty much just what PHP itself already does...
As you say elsewhere, this is useful for helping people migrate to it. The flipside of that is that it ties us into past decisions, rather than evaluating whether those decisions are still what we want. The approach in providing both $files and $uploads arrays is a good compromise - provide the new better way, but also an easy-to-migrate way. I'd love to see them named more distinctly, though, maybe even calling one of them "legacy". I'd probably also make them methods so that the data can be stored once (in the new format) and re-formatted on demand (again, objects as behaviour rather than data).
> ...easing their transition away from global state
This I find less convincing. To quote from further down the e-mail, I wrote:
>> ...you can do almost as much in that direction as the RFC by writing "$get=$_GET; unset($_GET);" The hard problem is that the entry point for a request is in global scope, not a main() or handleRequest() function.
and you replied:
> Another "hard" problem is carrying those values around in the system once they are decoupled from global state; the objects in this RFC purport to do so.
If all the object does is "carry around" a copy of the superglobal arrays, I can write it in about a dozen lines of code. Admittedly, the implementation wouldn't match anyone else's, but if I wanted interoperability, I would need to follow a standard like PSR-7, or match a popular existing library; this does neither. Again, the response object is more powerful in this regard, because we can't currently pass around a list of prepared cookies and trivially output it to the client. Along the lines of my previous message about decoupling concerns, I would personally like the response parts of the proposal split out and re-cast as a kind of "response buffer". That could be useful directly, and it could provide useful functionality to people implementing a full PSR-7 or similar Response object.
> I had considered mentioning the dual OO+procedural APIs of mysqli, date, etc. but it seemed too much in an already-long RFC; I'm glad you brought it up, as I did have it in mind.
This is an interesting comparison to consider. Just as those extensions have procedural options for people who can't stand objects, the current proposal has objects for people who like "OO-ish".
>> Introducing these objects as part of a new calling convention for PHP scripts would definitely add value, and make them a true replacement for the superglobals, but that would be a very different RFC. > That does seem rather ambitious. If you feel that's a valuable thing to add to PHP, perhaps it could be part of a future RFC, maybe even one that uses the objects in this RFC as a starting point.
One of the things I don't like about the current proposal is that it's modelled so closely around the current superglobals that I fear it would actually be *harder* to replace them. I find this constraint particularly awkward:
> N.b.: It is up to you to make sure the various content-related header > values in |$GLOBALS| match the custom |$content| string.
Parsing a request body from an arbitrary source into arrays that match the structure of $_POST and $_FILES would be a really useful feature. It would be something userland libraries could make use of right now, and would fit in nicely with a future where those arrays aren't populated by default. I also really want to see the day when I never have to interact with $_SERVER again. Other than renaming it, this object's design makes that less likely, not more. Imagine if in future we're able to redesign the internals so that there is a dedicated and reliable field for the requested URL; the current object doesn't have anywhere we can put that. If we add it later, it will be too late, code will be written that passes around ServerRequest objects, but relies on the full array of CGI variables to reconstruct the URL. If this object took a more opinionated view of what behaviour to encapsulate, we could simply hide the "server" array completely. Common use cases would be exposed via methods, and rarer use cases would have to be added in userland with their own copy of $_SERVER. Then my dream of deprecating $_SERVER could mean something other than moving it into an object. I hope I haven't left any unfinished sentences as I went back and forth through this! And thanks again for taking the time to engage with me. Regards, -- Rowan Tommins (né Collins) [IMSoP]
  108689
February 19, 2020 20:21 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

Again, thanks for your time and effort in evaluating this proposal.


> On Feb 18, 2020, at 14:21, Rowan Tommins collins@gmail.com> wrote: > > - What's the sales pitch - what do YOU think is great about these classes? > > ... > > Sell it to me! :)
I don't do sales; occasionally I can present a good narrative. Maybe the narrative below will help.
> - When working out the details, what code should we be picturing using the new classes?
Not to be flippant, but: request-reading and response-writing code?
>> ...a built-in OO-ish request/response object set... > > "OO-ish" is a wise choice of words here. One of the reasons I'm not terribly keen on this proposal - particularly the request half - is that I'm a bit of an OO purist.
Heh -- yes, I am aware of the impurities (both perceived and actual) of ServerRequest. ;-) However ...
> The lack of behaviour also makes it less useful to people writing their own request and response objects: if I have a copy of $_POST and want to put it in my own object property, why would I first pass it to a ServerRequest object, and then get it straight back out again, if the object isn't helping me do anything with that data?
.... in this case, ServerRequest does quite a bit that is easy to take for granted, since it does so quickly and quietly. Among other things: 1. It "bundles" several related pieces of data; not just $_POST but $_GET, $_COOKIES, etc., so they can be carried around together. 2. It separates that data from global state. 3. It parses some of that data into commonly-needed structures (eg. the `$accept*` arrays). 4. Its copies of that data are read-only and immutable, so that once set they cannot be modified at a distance when shared around; this keeps them stable throughout the system. So while it is true that ServerRequest "lacks behavior" in the sense that it doesn't "do" anything *after* construction, but it also true that it does quite a bit *at* construction. Indeed, the vast majority of uses (in practice) for a ServerRequest type of object are "reading" or "getter" uses. There is comparatively little need to write to a ServerRequest instance. Almost all of the time, all you need is to read from it post-construction. Further, there's no need to optimize for calculation-on-demand, since the calculations are so speedy in the first place. And since it will happen only once in the request lifespan, you might as well build all the properties at construction-time. At that point, getter-type methods are just returning what has already been calculated. And at *that* point, read-only properties do the trick quite nicely. For the comparatively-rare (but still necessary) times when you need to write to a ServerRequest type of object, it is open to extension in userland for those application-specific cases.
>> ...that does pretty much just what PHP itself already does... > > As you say elsewhere, this is useful for helping people migrate to it. The flipside of that is that it ties us into past decisions, rather than evaluating whether those decisions are still what we want.
Your comment jogged my memory of an older conversation, <https://externals.io/message/100087>, in which you figure prominently, and in which I pointed out the existence of the prior version of this RFC. Many things there are reminiscent here: - Should superglobals be made read-only? - Should superglobals be made into objects instead of arrays? - Should superglobals be replaced entirely with some new system? - Are additional superglobals needed? - Should there be a layer of indirection between superglobals and their use in applications? This RFC answers from (if you'll pardon the word) a conservative position: no, no, no, maybe, and yes. In the last case, this RFC can be construed to provide ServerRequest as that layer of indirection for some of the superglobals.
> The approach in providing both $files and $uploads arrays is a good compromise - provide the new better way, but also an easy-to-migrate way. I'd love to see them named more distinctly, though, maybe even calling one of them "legacy". I'd probably also make them methods so that the data can be stored once (in the new format) and re-formatted on demand (again, objects as behaviour rather than data).
I appreciate that concession on your part, and thanks for saying. I think $uploads is a pretty distinct name, while being clearly related to files. And, as I mentioned earlier, the cost of reformatting the $_FILES structure to $uploads is so low that it might as well occur at construction time, instead of adding a method to calculate-and-retain the reformatted structure.
>> ...easing their transition away from global state > > This I find less convincing. To quote from further down the e-mail, I wrote: > >>> ...you can do almost as much in that direction as the RFC by writing "$get=$_GET; unset($_GET);" The hard problem is that the entry point for a request is in global scope, not a main() or handleRequest() function. > > and you replied: > >> Another "hard" problem is carrying those values around in the system once they are decoupled from global state; the objects in this RFC purport to do so. > > If all the object does is "carry around" a copy of the superglobal arrays, I can write it in about a dozen lines of code.
I answered this above, but to reiterate: "carrying around" is *one* thing ServerRequest does, but not *all* it does.
> Again, the response object is more powerful in this regard, because we can't currently pass around a list of prepared cookies and trivially output it to the client. > > Along the lines of my previous message about decoupling concerns, I would personally like the response parts of the proposal split out and re-cast as a kind of "response buffer".
I admit I considered this. However, it makes more sense to me in terms of symmetry/complementarity, and in terms of "what we need on a daily basis", to provide both the request object and the response object together.
>> I had considered mentioning the dual OO+procedural APIs of mysqli, date, etc. but it seemed too much in an already-long RFC; I'm glad you brought it up, as I did have it in mind. > > This is an interesting comparison to consider. Just as those extensions have procedural options for people who can't stand objects, the current proposal has objects for people who like "OO-ish".
Yes, I think so too.
>>> Introducing these objects as part of a new calling convention for PHP scripts would definitely add value, and make them a true replacement for the superglobals, but that would be a very different RFC. >> >> That does seem rather ambitious. If you feel that's a valuable thing to add to PHP, perhaps it could be part of a future RFC, maybe even one that uses the objects in this RFC as a starting point. > > One of the things I don't like about the current proposal is that it's modelled so closely around the current superglobals that I fear it would actually be *harder* to replace them.
Maybe? I can similarly imagine that if new-and-different superglobals appear, the ServerRequest object can evolve to contain and/or translate between them.
> Parsing a request body from an arbitrary source into arrays that match the structure of $_POST and $_FILES would be a really useful feature.
Yes, although one can do at least the $_POST portion with ServerRequest as it is now. For example, a naive userland factory might do this: class ServerRequestFactory { public function new(array $globals, ?string $content = null) : ServerRequest { if ($this->isJson($globals['_SERVER']['CONTENT_TYPE'] ?? '') { $globals['_POST'] = json_decode( $content ?? file_get_contents('php://input'), true ); } return new ServerRequest($globals, $content); } protected function isJson(string $contentType) : bool { return $contentType === 'application/json' || substr($contentType, -5) === '+json'; } } Call `$request = (new ServerRequestFactory())->new();` and voila: the equivalent of `$_POST`, populated from JSON content, stored as $request->input.
> I also really want to see the day when I never have to interact with $_SERVER again. Other than renaming it, this object's design makes that less likely, not more. > > Imagine if in future we're able to redesign the internals so that there is a dedicated and reliable field for the requested URL; the current object doesn't have anywhere we can put that. If we add it later, it will be too late, code will be written that passes around ServerRequest objects, but relies on the full array of CGI variables to reconstruct the URL. > > If this object took a more opinionated view of what behaviour to encapsulate, we could simply hide the "server" array completely. Common use cases would be exposed via methods, and rarer use cases would have to be added in userland with their own copy of $_SERVER. Then my dream of deprecating $_SERVER could mean something other than moving it into an object.
Yes, the internals thread I pointed to earlier makes that clear. And though I understand where you're coming from, especially regarding "a dedicated and reliable field for the requested URL", this RFC does not have that kind of ambition, nor do I expect it to in the future. This proposal is satisfied to take only a smaller series of steps to ease a more limited set of problems, rather than revise major and wide-ranging elements of PHP overall. I presume you will find that unsatisfactory, but, well, there it is. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108691
February 19, 2020 22:43 rowan.collins@gmail.com (Rowan Tommins)
On 19/02/2020 20:21, Paul M. Jones wrote:
>> Sell it to me! :) > I don't do sales; occasionally I can present a good narrative. Maybe the narrative below will help.
Hah! Fair enough. :)
>> - When working out the details, what code should we be picturing using the new classes? > Not to be flippant, but: request-reading and response-writing code?
The reason I keep trying to pin you down on this is that I am concerned about the "jack of all trades" effect - that a feature which lacks focus can end up being less useful than if it was refined for one job. From the point of view of a reviewer, it's also hard to answer the question "do you think this is a good design?" when you don't know what it's the design for. To take a specific example, if an aim is for this class to be used as an underpinning for higher-level libraries, we could look at some existing examples. We can think about what new functionality they would gain, or what old code they could delete, by using this class. We can also look at whether the interface as currently proposed would be "comfortable" to integrate, or if there are changes that would make it easier. That doesn't mean you have to declare there to be one single goal, but it's easier to get specific about a design fitting a goal than making an abstract judgement about it.
> >> The lack of behaviour also makes it less useful to people writing their own request and response objects > 1. It "bundles" several related pieces of data; not just $_POST but $_GET, $_COOKIES, etc., so they can be carried around together. > > 2. It separates that data from global state. > > 4. Its copies of that data are read-only and immutable, so that once set they cannot be modified at a distance when shared around; this keeps them stable throughout the system.
You see, here's an example where specifying the goal matters. In that particular comment, I specifically talked about the use case of wrapping a larger object around this one. The boilerplate to wrap this object in a different interface is probably *larger* than the boilerplate to wrap the superglobals directly, so *for that use case* these three features are not relevant.
> 3. It parses some of that data into commonly-needed structures (eg. the `$accept*` arrays).
This, however, is the kind of thing I mean by "behaviour", and the kind of thing I'd like to see more of. Going back to the goal of "libraries could wrap this object and extend it", we can directly see how it would make their lives easier because they wouldn't have to reimplement it. The particular selection of fields feels rather arbitrary, though - for instance, I've never heard of "Content-MD5", but would have expected at least some URL-related properties, like Host.
> Further, there's no need to optimize for calculation-on-demand, since the calculations are so speedy in the first place. And since it will happen only once in the request lifespan, you might as well build all the properties at construction-time. At that point, getter-type methods are just returning what has already been calculated. And at *that* point, read-only properties do the trick quite nicely.
My preference for methods over properties is partly just OO purism, but I do think they have real benefits. One of them is the ability to evolve the implementation over time. For instance, the current design has two arrays of uploaded files, one in $_FILES format, and one in a newer format. What if in future, we want to add a third, where each item is an object? With properties, we'd have to pre-populate all three arrays on construct; but with methods, we could refactor the internals to pull all three from some common reference point. Similarly, methods can have parameters, and that can be a great way of introducing opt-in behaviour. For instance, maybe in future someone requests that getMethod() should have a $allow_override flag, which set to false would ignore the X-HTTP-METHOD-OVERRIDE header. With properties, you need a separate property for every possible combination, and you have to come up with names for them all.
> I think $uploads is a pretty distinct name, while being clearly related to files.
Perhaps "descriptive" would be a better word than "distinct". I can tell $files and $uploads apart at a glance, but the names tell me nothing about why both exist, or which I should be using.
>>> Another "hard" problem is carrying those values around in the system once they are decoupled from global state; the objects in this RFC purport to do so. > I answered this above, but to reiterate: "carrying around" is *one* thing ServerRequest does, but not *all* it does.
I'm not disputing that; I'm disputing that that particular feature is in any way a hard problem.
> I admit I considered this. However, it makes more sense to me in terms of symmetry/complementarity, and in terms of "what we need on a daily basis", to provide both the request object and the response object together.
One oddity of the proposal is that the two objects aren't actually very symmetrical.  For instance, to copy a particular header from a request to a response, I'd read a plain array $request->headers[$name], but write via a method $response->setHeader($name, $value). (It's also not clear whether this would work correctly if the header had multiple values in the request.)
> Maybe? I can similarly imagine that if new-and-different superglobals > appear, the ServerRequest object can evolve to contain and/or > translate between them.
Although we can't predict the future, there are things we can do to make it more likely we could adapt to such a change. We should make a conscious decision whether this is a goal, or something we're happy not to focus on.
>> Parsing a request body from an arbitrary source into arrays that match the structure of $_POST and $_FILES would be a really useful feature. > Yes, although one can do at least the $_POST portion with ServerRequest as it is now. > ... > Call `$request = (new ServerRequestFactory())->new();` and voila: the equivalent of `$_POST`, populated from JSON content, stored as $request->input.
I was actually thinking of the opposite: given a request body which didn't come from global state, but which contains data in multipart/form-data format, extract the key-value pairs and attached files. This is a non-trivial piece of functionality, which would have real-world uses; here it is implemented in ReactPHP: https://github.com/reactphp/http/blob/121abe0558465cc7f2cecdb3027dae959f348409/src/Io/MultipartParser.php Rather than accepting the content body as an optional constructor parameter, what if there were two named constructors: - fromSuperGlobalArrays($server, $cookie, $post, $get, $files) - fromRawRequestBody($server, $cookie, $body) In the first case, accessing the raw body on the object could give null, because none is known - defaulting to global state seems like a mistake if decoupling from global state is an explicit aim. That said, perhaps a third constructor could be a short-hand for just that: fromCurrentRequest, which would be equivalent to fromRawRequestBody($_SERVER, $_COOKIE, file_get_contents('php://input'));
> Yes, the internals thread I pointed to earlier makes that clear. And > though I understand where you're coming from, especially regarding "a > dedicated and reliable field for the requested URL", this RFC does not > have that kind of ambition, nor do I expect it to in the future.
I agree that re-designing the way web servers pass in the URL is a much harder problem, but if you look at pretty much any existing Request wrapper, it will make some attempt to extract a URL from the $_SERVER array. That really feels like a missing feature of this one right now. (I appreciate it's not simple, but that's why it's so useful to have someone else write it!) I hope my comments aren't coming across as too negative. Maybe our visions for what this should be are just too different, but some mixture of hope and stubbornness makes me want to keep nudging it somewhere "better". Regards, -- Rowan Tommins (né Collins) [IMSoP]
  108701
February 20, 2020 14:56 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

> On Feb 19, 2020, at 16:43, Rowan Tommins collins@gmail.com> wrote: > >>> - When working out the details, what code should we be picturing using the new classes? >> >> Not to be flippant, but: request-reading and response-writing code? > > The reason I keep trying to pin you down on this is that I am concerned about the "jack of all trades" effect - that a feature which lacks focus can end up being less useful than if it was refined for one job. From the point of view of a reviewer, it's also hard to answer the question "do you think this is a good design?" when you don't know what it's the design for. > > ... it's easier to get specific about a design fitting a goal than making an abstract judgement about it.
The purpose of the extension is as stated in the RFC: that is, to provide "an object-oriented approach around request and response functionality already existing in PHP ... an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on." At this point, I get the impression that either one of use is underthinking things, or the other one is overthinking them -- or perhaps we just have different premises and visions.
> The particular selection of fields feels rather arbitrary, though - for instance, I've never heard of "Content-MD5",
That's fair; once some of the content-related header fields came into play, it seemed reasonable to bring all of them in.
> but would have expected at least some URL-related properties, like Host.
Aha! If nothing else, then, this conversation has revealed a documentation flaw: specifically, my failure to document the ServerRequest $url property. That failure is now remedied: <https://github.com/pmjones/ext-request#the-url-array>
> Similarly, methods can have parameters, and that can be a great way of introducing opt-in behaviour. For instance, maybe in future someone requests that getMethod() should have a $allow_override flag, which set to false would ignore the X-HTTP-METHOD-OVERRIDE header. With properties, you need a separate property for every possible combination, and you have to come up with names for them all.
I get the need for future extension, which is why the method space is left open for consumers. If they want to provide their own getter methods, calculating from the existing properties or other values, they are in the clear to do so.
>> I think $uploads is a pretty distinct name, while being clearly related to files. > > Perhaps "descriptive" would be a better word than "distinct". I can tell $files and $uploads apart at a glance, but the names tell me nothing about why both exist, or which I should be using.
Which is the purpose of the documentation; it describes the differences between $files and $uploads.
>>>> Another "hard" problem is carrying those values around in the system once they are decoupled from global state; the objects in this RFC purport to do so. >> I answered this above, but to reiterate: "carrying around" is *one* thing ServerRequest does, but not *all* it does. > I'm not disputing that; I'm disputing that that particular feature is in any way a hard problem.
Your disputation is noted.
>> I admit I considered this. However, it makes more sense to me in terms of symmetry/complementarity, and in terms of "what we need on a daily basis", to provide both the request object and the response object together. > > One oddity of the proposal is that the two objects aren't actually very symmetrical.
That's fair; strike "symmetry" and retain "complementarity."
>> Maybe? I can similarly imagine that if new-and-different superglobals appear, the ServerRequest object can evolve to contain and/or translate between them. > > Although we can't predict the future, there are things we can do to make it more likely we could adapt to such a change. We should make a conscious decision whether this is a goal, or something we're happy not to focus on.
The RFC states, under Future Scope, "This extension acts as an object-oriented wrapper around existing PHP request and response functionality; as the scope of that PHP functionality expands, this extension should expand with it."
>>> Parsing a request body from an arbitrary source into arrays that match the structure of $_POST and $_FILES would be a really useful feature. >> Yes, although one can do at least the $_POST portion with ServerRequest as it is now. >> ... >> Call `$request = (new ServerRequestFactory())->new();` and voila: the equivalent of `$_POST`, populated from JSON content, stored as $request->input. > > I was actually thinking of the opposite: given a request body which didn't come from global state, but which contains data in multipart/form-data format, extract the key-value pairs and attached files.
Is that something PHP "itself" already does? If not, I have to consider it out-of-scope for this RFC.
> Rather than accepting the content body as an optional constructor parameter, what if there were two named constructors: > > - fromSuperGlobalArrays($server, $cookie, $post, $get, $files) > - fromRawRequestBody($server, $cookie, $body)
If consumers/users of ext-request wish to create factories or builders to instantiate a ServerRequest instance, it is within their purview to do so.
> In the first case, accessing the raw body on the object could give null, because none is known - defaulting to global state seems like a mistake if decoupling from global state is an explicit aim.
Your point on global state is well-taken; I will try to remember in future to phrase it as "global *mutable* state." (AFAICT, php://input is not mutable, though as you correctly point out, it is global.)
> if you look at pretty much any existing Request wrapper, it will make some attempt to extract a URL from the $_SERVER array. That really feels like a missing feature of this one right now.
Yeah, my bad on not documenting it earlier -- please consider the "missing" feature "found." ;-)
> I hope my comments aren't coming across as too negative.
Not "negative", but as you say ...
> Maybe our visions for what this should be are just too different
.... I think your goals are loftier here than proposal's. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108746
February 24, 2020 21:47 rowan.collins@gmail.com (Rowan Tommins)
Hi Paul,

I left this thread to settle a bit, because I felt we were going round 
in circles a bit. I think there's some fundamental differences in 
outlook that no amount of discussion is going to resolve, so I've 
trimmed this reply to the more specific points.



On 20/02/2020 14:56, Paul M. Jones wrote:
> Aha! If nothing else, then, this conversation has revealed a > documentation flaw: specifically, my failure to document the > ServerRequest $url property. That failure is now remedied: > <https://github.com/pmjones/ext-request#the-url-array>
Aha indeed! When I was talking about "selling it to me", this is exactly the kind of thing I was looking for. This kind of functionality is much more interesting to me than copying half a dozen arrays from constructor parameters into read-only properties.
> Which is the purpose of the documentation; it describes the differences between $files and $uploads.
That's no reason not to *try* for descriptive names, though. I presume the idea is that one array is expected to be more useful for new code, and the other is mostly there for compatibility with old code? If so, perhaps the names could reflect that somehow. Incidentally, the current documentation doesn't describe the differences particularly well, just saying one is "more like $_POST". Some details of the structure, or examples comparing the two arrays, would be useful.
>> I was actually thinking of the opposite: given a request body which didn't come from global state, but which contains data in multipart/form-data format, extract the key-value pairs and attached files. > Is that something PHP "itself" already does? If not, I have to consider it out-of-scope for this RFC.
a) Yes: Every time you submit a form as multipart/form-data, PHP parses it into the global state. If this object is aiming to abstract away from global state, then having a non-global-state parser for that seems consistent. b) No: There is no function which currently takes a string and returns an array for this scenario. However, that's true of other features you have included, such as parsing accept headers, or even extracting just HTTP headers from a copy of the $_SERVER array.
> Your point on global state is well-taken; I will try to remember in future to phrase it as "global *mutable* state." (AFAICT, php://input is not mutable, though as you correctly point out, it is global.)
This distinction seems unnecessary. Once created, the object avoids global state because it is self-contained, and the fact that it's read-only is a separate attribute. We're really just talking about ways to construct that self-contained state, and "everything from global state" or "nothing from global state" seem like more natural options than "one thing from global state, everything else not". Regards, -- Rowan Tommins (né Collins) [IMSoP]
  108771
February 26, 2020 16:42 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

> On Feb 24, 2020, at 15:47, Rowan Tommins collins@gmail.com> wrote: > > Hi Paul, > > I left this thread to settle a bit, because I felt we were going round in circles a bit. I think there's some fundamental differences in outlook that no amount of discussion is going to resolve, so I've trimmed this reply to the more specific points.
I appreciate you taking the trouble; thanks.
>> Which is the purpose of the documentation; it describes the differences between $files and $uploads. > > That's no reason not to *try* for descriptive names, though. I presume the idea is that one array is expected to be more useful for new code, and the other is mostly there for compatibility with old code? If so, perhaps the names could reflect that somehow.
Your presumption is correct! And your point on trying for better names is well-taken -- though I think these are "expected" names, based on my research into existing implementations. The most-common names are ... - the word "files" for unparsed or unmodified $_FILES values, leading me to think $files is well-understood - the words "upload(s)", "fileUpload(s)", or "uploadedFile(s)" for parsed, transformed, or restructured $_FILES values, leading me to think $uploads is well-understood And the current users of the 1.x version have not reported confusion or trouble with the names as they are. Having said that, I am open to suggestion here. What names do you think would be better than the ones presented, contra pre-existing work from other authors?
> Incidentally, the current documentation doesn't describe the differences particularly well, just saying one is "more like $_POST". Some details of the structure, or examples comparing the two arrays, would be useful.
Good call! On your suggestion, I have added details at <https://github.com/pmjones/ext-request#the-uploads-array>. Suggestions for improvement are welcome.
>>> I was actually thinking of the opposite: given a request body which didn't come from global state, but which contains data in multipart/form-data format, extract the key-value pairs and attached files. >> Is that something PHP "itself" already does? If not, I have to consider it out-of-scope for this RFC. > > > a) Yes: Every time you submit a form as multipart/form-data, PHP parses it into the global state. If this object is aiming to abstract away from global state, then having a non-global-state parser for that seems consistent. > > b) No: There is no function which currently takes a string and returns an array for this scenario. However, that's true of other features you have included, such as parsing accept headers, or even extracting just HTTP headers from a copy of the $_SERVER array.
Quite a lot packed into four sentences; I'll try to address all of it. - First off, "yes and no" is a great answer, the most-accurate one possible. It highlights what I'm getting at: PHP transforms the content body into $_POST under some circumstances but not others. The RFC proposes to honor that existing PHP behavior. - "If this object is aiming to abstract away from global state" -- well, global *mutable* state, anyway, per our last exchange (and noted again below). - "There is no function which currently takes a string and returns an array for this scenario." -- True, though (and not to undermine my own case) there is json_decode(), which can return an array. But then the trouble is how to decide when to apply it, and with what parameters, and how to trap & report errors, etc., all of which adds complexity to what I think ought to be a simple object. And then once special treatment is given to JSON, what about XML? Etc., etc. Given all that, it strikes me that the cases not already served by PHP for parsing the body content into $_POST are best left to consumers of ServerRequest. - "However, that's true of other features you have included" -- also true, though those are informed by research into existing implementations, and provide what appear to be commonly- or frequently-needed values and structures. (You may opine this is contradictory, in which case I will respond as Whitman: "Do I contradict myself? Very well, then I contradict myself. I am large, I contain multitudes." ;-)
>> Your point on global state is well-taken; I will try to remember in future to phrase it as "global *mutable* state." (AFAICT, php://input is not mutable, though as you correctly point out, it is global.) > > This distinction seems unnecessary. Once created, the object avoids global state because it is self-contained, and the fact that it's read-only is a separate attribute. We're really just talking about ways to construct that self-contained state, and "everything from global state" or "nothing from global state" seem like more natural options than "one thing from global state, everything else not".
I agree that the default $content value is an exception to everything else in ServerRequest. While it stays read-only, it does get read from php://input each time you refer to it. The alternative is to read from php://input once at construction time (when the construct $content argument is null) and retain that value in $content from the start. But in that case, when you have very large content bodies (as when there are PUT file uploads or other large payloads), then ServerRequest takes up a lot of memory, when it might not ever be used directly. Yes, "it might never be used" could be true of every element on ServerRequest, but the maximum memory use for those other elements tends to be rather smaller. In the end, letting $content read from php://input on the fly is a reasonable tradeoff; an exception that tests the rule, if you will. Over to you! -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108774
February 26, 2020 18:55 rowan.collins@gmail.com (Rowan Tommins)
On Wed, 26 Feb 2020 at 16:42, Paul M. Jones <pmjones@pmjones.io> wrote:

> Your presumption is correct! And your point on trying for better names is > well-taken -- though I think these are "expected" names, based on my > research into existing implementations. The most-common names are ... > > - the word "files" for unparsed or unmodified $_FILES values, leading me > to think $files is well-understood > > - the words "upload(s)", "fileUpload(s)", or "uploadedFile(s)" for parsed, > transformed, or restructured $_FILES values, leading me to think $uploads > is well-understood >
That's a reasonable justification. Just to check, are there other implementations that have both of these names side by side, or do most implementations have one or the other, but using this naming? The main confusion I can see is having to remember which two of these is an error without having to read the docs each time: isset( $request->files['attachment']['name'][0] ); isset( $request->files['attachment'][0]['name'] ); isset( $request->uploads['attachment']['name'][0] ); isset( $request->uploads['attachment'][0]['name'] );
> Having said that, I am open to suggestion here. What names do you think > would be better than the ones presented, contra pre-existing work from > other authors? >
Looking at the examples, the difference is rather similar to the PREG_PATTERN_ORDER and PREG_SET_ORDER options to preg_match_all. If getUploads was a method, it could take a similar behaviour switch - GROUP_BY_ITEM vs GROUP_BY_ATTRIBUTE or similar. For a property, that would have to be part of the name, like $uploadsByItem and $uploadsByAttribute, which are a bit ugly. Alternatively, you could lean more heavily on the legacy aspect, and have $uploads and $uploadsLegacyFormat or something like that.
> Incidentally, the current documentation doesn't describe the differences > particularly well, just saying one is "more like $_POST". Some details of > the structure, or examples comparing the two arrays, would be useful. > > Good call! On your suggestion, I have added details at < > https://github.com/pmjones/ext-request#the-uploads-array>. Suggestions > for improvement are welcome. >
Thanks, that makes it a lot clearer. :)
> - "There is no function which currently takes a string and returns an > array for this scenario." -- True, though (and not to undermine my own > case) there is json_decode(), which can return an array. But then the > trouble is how to decide when to apply it, and with what parameters, and > how to trap & report errors, etc., all of which adds complexity to what I > think ought to be a simple object. And then once special treatment is given > to JSON, what about XML? Etc., etc. Given all that, it strikes me that the > cases not already served by PHP for parsing the body content into $_POST > are best left to consumers of ServerRequest. >
Again, any mention of JSON or XML is drifting away from what I'm asking for. What I'm asking for (or rather, suggesting would be a useful and consistent addition) is a way to do *exactly what PHP does right now*, but based on a given input string, rather than the initial request. I am totally happy with users needing to add any support for JSON, XML, etc, just like they would have to populate $_POST manually.
> I agree that the default $content value is an exception to everything else > in ServerRequest. While it stays read-only, it does get read from > php://input each time you refer to it. > > The alternative is to read from php://input once at construction time > (when the construct $content argument is null) and retain that value in > $content from the start. But in that case, when you have very large content > bodies (as when there are PUT file uploads or other large payloads), then > ServerRequest takes up a lot of memory, when it might not ever be used > directly. Yes, "it might never be used" could be true of every element on > ServerRequest, but the maximum memory use for those other elements tends to > be rather smaller. >
That's a reasonable justification. I guess that's why PSR-7 exposes it as a stream, even though that causes a whole load of pain. To play devil's advocate, does the property belong in the object at all? If it doesn't interact with any of the other properties (e.g. populating $post and $uploads based on form data), could "better syntax for getting raw request for global state" be a separate feature. Then again, maybe the ability to over-ride it in the constructor is enough to make it useful. Regards, -- Rowan Tommins [IMSoP]
  108776
February 26, 2020 19:57 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

> On Feb 26, 2020, at 12:55, Rowan Tommins collins@gmail.com> wrote: > > That's a reasonable justification. Just to check, are there other > implementations that have both of these names side by side, or do most > implementations have one or the other, but using this naming?
My recollection is that they have one or the other, but none with both side-by-side. (A very few have neither.)
> The main confusion I can see is having to remember which two of these is an > error without having to read the docs each time: > > isset( $request->files['attachment']['name'][0] ); > isset( $request->files['attachment'][0]['name'] ); > isset( $request->uploads['attachment']['name'][0] ); > isset( $request->uploads['attachment'][0]['name'] );
/me nods While I too have imagined that, my impression has been that consumers of 1.x use $files unless & until they change their systems to use $uploads, at which point they switch over entirely to $uploads. Given that, my concerns (such as they may have been) are soothed.
> If getUploads was a method, it could take a similar behaviour switch - > GROUP_BY_ITEM vs GROUP_BY_ATTRIBUTE or similar. For a property, that would > have to be part of the name, like $uploadsByItem and $uploadsByAttribute, > which are a bit ugly. > > Alternatively, you could lean more heavily on the legacy aspect, and have > $uploads and $uploadsLegacyFormat or something like that.
Noted. Are there any others here who feel that the names $files and $uploads are "too confusing" (for whatever values of "confusing" you find appropriate) ?
> Again, any mention of JSON or XML is drifting away from what I'm asking > for.
Ah, my bad then.
> What I'm asking for (or rather, suggesting would be a useful and > consistent addition) is a way to do *exactly what PHP does right now*, but > based on a given input string, rather than the initial request.
I'm sorry, I'm still having trouble seeing what you're getting at. Do you mean something like this in the ServerRequest constructor? public function __construct(array $globals, ?string $content = null) { if ( ($globals['_POST'] ?? null) === null && strtolower($globals['_SERVER']['CONTENT_TYPE']) === 'application/x-www-form-urlencoded' ) { if ($content === null) { $content = file_get_contents('php://input'); } $globals['_POST'] = []; parse_str($content, $globals['_POST']); } // ... } If so, it seems unnecessary for the goals of this RFC, even overkill. If not, then I await correction.
> To play devil's advocate, does the [$content] property belong in the object at all? If it doesn't interact with any of the other properties (e.g. populating $post and $uploads based on form data), could "better syntax for getting raw request for global state" be a separate feature. Then again, maybe the ability to over-ride it in the constructor is enough to make it useful.
I continue to think it does belong there. Often enough, API developers will read php://input to decode JSON or XML they find there, so having it easily-available as $request->content is appealing. To boot, the other content-related headers are present, so might as well have the content itself there too. And as you note, it's easy enough to pass in custom $content via the constructor, e.g. for testing. * * * I feel that if these are the discussion points, then we are close to exhausting the topic (even if we do not agree on everything). Thank you for your diligence and attention to detail! -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108777
February 26, 2020 20:12 rowan.collins@gmail.com (Rowan Tommins)
On 26/02/2020 19:57, Paul M. Jones wrote:
> I'm sorry, I'm still having trouble seeing what you're getting at. Do you mean something like this in the ServerRequest constructor? > > public function __construct(array $globals, ?string $content = null) > { > if ( > ($globals['_POST'] ?? null) === null > && > strtolower($globals['_SERVER']['CONTENT_TYPE']) === 'application/x-www-form-urlencoded' > ) { > if ($content === null) { > $content = file_get_contents('php://input'); > } > $globals['_POST'] = []; > parse_str($content, $globals['_POST']); > } > > // ... > }
That's the easy part, yes; the harder part is this: if ( ($globals['_POST'] ?? null) === null && strtolower($globals['_SERVER']['CONTENT_TYPE']) === 'multipart/form-data' ) { if ($content === null) { $content = file_get_contents('php://input'); } [ $globals['_POST'], $globals['_FILE'] ] = parse_multipart_form_data($content); } Where parse_multipart_form_data is a function which doesn't currently exist, but whose logic must all be there in the core somewhere, because everything shows up in $_POST and $_FILES when you process such a request. Recreating that functionality in userland is non-trivial, but is essential for several use cases, e.g. an event-based server like ReactPHP, or a test using a saved request body as a fixture. If both content types (application/x-www-form-urlencoded and multipart/form-data) were handled, it would also mean that the relationship $content -> $input would match the relationship php://input -> $_POST by default, which seems consistent with the aim of matching existing behaviour. Regards, -- Rowan Tommins (né Collins) [IMSoP]
  108779
February 27, 2020 02:43 pmjones@pmjones.io (Paul M . Jones)
> On Feb 26, 2020, at 14:12, Rowan Tommins collins@gmail.com> wrote: > > On 26/02/2020 19:57, Paul M. Jones wrote: > >> Do you mean something like this in the ServerRequest constructor? >> >> ... > > That's the easy part, yes; the harder part is this: > > ...
Yes, that would definitely be the harder part.
> Recreating that functionality in userland is non-trivial, but is essential for several use cases, e.g. an event-based server like ReactPHP, or a test using a saved request body as a fixture. > > If both content types (application/x-www-form-urlencoded and multipart/form-data) were handled, it would also mean that the relationship $content -> $input would match the relationship php://input -> $_POST by default, which seems consistent with the aim of matching existing behaviour.
Yes, it would indeed. However, it strikes me that the thing to do here is not to try and embed that behavior in ServerRequest; rather, it would be to expose the existing functionality on its own, so that ReactPHP (and many others!) can use them, instead of having to roll their own in userland (a la <https://github.com/reactphp/http/blob/master/src/Io/MultipartParser.php>).. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108781
February 27, 2020 09:43 rowan.collins@gmail.com (Rowan Tommins)
On Thu, 27 Feb 2020 at 02:43, Paul M. Jones <pmjones@pmjones.io> wrote:

> > Recreating that functionality in userland is non-trivial, but is > essential for several use cases, e.g. an event-based server like ReactPHP, > or a test using a saved request body as a fixture. > > > > If both content types (application/x-www-form-urlencoded and > multipart/form-data) were handled, it would also mean that the relationship > $content -> $input would match the relationship php://input -> $_POST by > default, which seems consistent with the aim of matching existing behaviour. > > Yes, it would indeed. However, it strikes me that the thing to do here is > not to try and embed that behavior in ServerRequest; rather, it would be to > expose the existing functionality on its own, so that ReactPHP (and many > others!) can use them, instead of having to roll their own in userland (a > la < > https://github.com/reactphp/http/blob/master/src/Io/MultipartParser.php>). >
Why not hope that ReactPHP and others will want to use this object, precisely because it avoids them having to roll their own implementations of things? If your concern is that the object should only wrap up features that are also available via some other mechanism, I'll point again at the $accept array, which AFAIK is a useful piece of parsing which is not available outside the proposed object's constructor. If anything, that's *more* special to this object, because what I'm proposing would directly parallel the existing behaviour of $_POST and $_FILES. If somebody really wanted to use the parser without the rest of the object, they could trivially wrap it in a function: function parse_multipart_form_data($content) { $request = new ServerRequest([], $content); return [ 'input' => $request->input, 'uploads' => $request->uploads ]; } The only downside I can see to adding it is complexity of implementation. But that's at best a reason to say "we'll hope to add this later" rather than "it would be better not to add it". Regards, -- Rowan Tommins [IMSoP]
  108782
February 27, 2020 15:55 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,


> Why not hope that ReactPHP and others will want to use this object, > precisely because it avoids them having to roll their own implementations > of things?
Like I said earlier: if React ends up wanting to use ext/request, with its tradeoffs, of course I would think that's great. But if they want to keep using what they have already, with its own tradeoffs, that's great too.
> If somebody really wanted to use the parser without the rest of the object, > they could trivially wrap it in a function: > > function parse_multipart_form_data($content) { > $request = new ServerRequest([], $content); > return [ 'input' => $request->input, 'uploads' => $request->uploads ]; > }
This is very similar to what I'm saying: to use your phrasing, I opine it is better to "trivially wrap" the existing PHP functionality as part of a separate RFC, rather than try to embed it in ServerRequest (exposed or otherwise). To reiterate what I've said before: this RFC is a relatively conservative one. The vision around it is to stay pretty close to PHP as-it-is, and to incorporate those things from the researched implementations that show up over and over again. I know that does not lead quickly toward (what I surmise is) your vision of overhauling how PHP presents global state, but an overhaul of that kind is just not what this RFC aims to do. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108783
February 27, 2020 16:57 rowan.collins@gmail.com (Rowan Tommins)
On Thu, 27 Feb 2020 at 15:55, Paul M. Jones <pmjones@pmjones.io> wrote:

> > Why not hope that ReactPHP and others will want to use this object, > > precisely because it avoids them having to roll their own implementations > > of things? > > Like I said earlier: if React ends up wanting to use ext/request, with its > tradeoffs, of course I would think that's great. But if they want to keep > using what they have already, with its own tradeoffs, that's great too. >
This is one place our attitudes differ: if I was proposing something like this to be included in every copy of PHP, I'd actively want people to use it, and consider that a measure of success; you seem to be happy to just put it out there and see. As such, I perhaps place greater value on functionality that would make it more attractive, and less value on matching what we already have.
> This is very similar to what I'm saying: to use your phrasing, I opine it > is better to "trivially wrap" the existing PHP functionality as part of a > separate RFC, rather than try to embed it in ServerRequest (exposed or > otherwise). >
That's not really the same, no. I am saying that once you have the parsing functionality somewhere in userland, whether it's inside ServerRequest or its own function doesn't matter, you could still use it to delete 300+ lines of code from ReactPHP. That's a reason to add it *as soon as possible*, even if in an ideal world it would be implemented in a slightly different place, or two different places, or whatever. Regards, -- Rowan Tommins [IMSoP]
  108784
February 27, 2020 17:06 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

> On Feb 27, 2020, at 10:57, Rowan Tommins collins@gmail.com> wrote: > > you seem to be happy to just put it out there and see
Perhaps it was not your intent, but even so: please don't put words in my mouth. We spoke of this before; quoting myself from <https://externals.io/message/108436#108650> ...
> I don't *expect* anything from existing published library authors; however ... > > • I *suspect* that some will choose to ignore this extension, > > • that others will decorate or extend it, > > • and that still others may find their own work so close to this extension that they migrate over to it entirely.
I continue to hold that position. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108785
February 27, 2020 17:27 rowan.collins@gmail.com (Rowan Tommins)
On Thu, 27 Feb 2020 at 17:06, Paul M. Jones <pmjones@pmjones.io> wrote:

> Hi Rowan, > > > On Feb 27, 2020, at 10:57, Rowan Tommins collins@gmail.com> > wrote: > > > > you seem to be happy to just put it out there and see > > Perhaps it was not your intent, but even so: please don't put words in my > mouth. We spoke of this before; quoting myself from < > https://externals.io/message/108436#108650> ... > > > I don't *expect* anything from existing published library authors; > however ... > > > > • I *suspect* that some will choose to ignore this extension, > > > > • that others will decorate or extend it, > > > > • and that still others may find their own work so close to this > extension that they migrate over to it entirely. > > I continue to hold that position. >
Perhaps I am over-interpreting them, but the words you choose always seem to me very passive, as though shrugging and saying "it might happen, or it might not". In the exchange quoted above, I can see why "expect" might have felt too strong, but "suspect" is right at the other extreme; do you also "hope", "desire", or "want" people to use it? Regards, -- Rowan Tommins [IMSoP]
  108786
February 27, 2020 17:52 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

> On Feb 27, 2020, at 11:27, Rowan Tommins collins@gmail.com> wrote: > > On Thu, 27 Feb 2020 at 17:06, Paul M. Jones <pmjones@pmjones.io> wrote: > >> We spoke of this before; quoting myself from <https://externals.io/message/108436#108650> ... >> >>> I don't *expect* anything from existing published library authors; however ... >>> >>> • I *suspect* that some will choose to ignore this extension, >>> >>> • that others will decorate or extend it, >>> >>> • and that still others may find their own work so close to ths extension that they migrate over to it entirely. >> >> I continue to hold that position. > > Perhaps I am over-interpreting them, but the words you choose always seem to me very passive, as though shrugging and saying "it might happen, or it might not".
It could be that you are over-interpreting; in point of fact, some combination of those three things is guaranteed to happen, no "might" about it. In any case, we seem to have drifted from evaluation of the proposal (per se) to other topics. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108787
February 27, 2020 18:04 mike@newclarity.net (Mike Schinkel)
> On Feb 27, 2020, at 12:52 PM, Paul M. Jones <pmjones@pmjones.io> wrote: >> On Feb 27, 2020, at 11:27, Rowan Tommins collins@gmail.com> wrote: >> On Thu, 27 Feb 2020 at 17:06, Paul M. Jones <pmjones@pmjones.io> wrote:
At the risk of being the messenger that gets shot, I feel like that horse is dead. And beating it more won't revive it, or make it any more dead. JMTCW. -Mike
  108778
February 26, 2020 21:46 mike@newclarity.net (Mike Schinkel)
> On Feb 26, 2020, at 1:55 PM, Rowan Tommins collins@gmail.com> wrote: > > On Wed, 26 Feb 2020 at 16:42, Paul M. Jones <pmjones@pmjones.io> wrote: > >> Your presumption is correct! And your point on trying for better names is >> well-taken -- though I think these are "expected" names, based on my >> research into existing implementations. The most-common names are ... >> >> - the word "files" for unparsed or unmodified $_FILES values, leading me >> to think $files is well-understood >> >> - the words "upload(s)", "fileUpload(s)", or "uploadedFile(s)" for parsed, >> transformed, or restructured $_FILES values, leading me to think $uploads >> is well-understood >> > > > That's a reasonable justification. Just to check, are there other > implementations that have both of these names side by side, or do most > implementations have one or the other, but using this naming? > > The main confusion I can see is having to remember which two of these is an > error without having to read the docs each time: > > isset( $request->files['attachment']['name'][0] ); > isset( $request->files['attachment'][0]['name'] ); > isset( $request->uploads['attachment']['name'][0] ); > isset( $request->uploads['attachment'][0]['name'] );
Here is an easy way to remember: $request->files is exactly like $_FILES (and if you can't remember that, blame PHP, not the RFC) $request->uploads is therefore new, and since it is new it follows a reasonable collection-list-item structure (as opposed to an unreasonable collection-item-list structure that PHP implemented in $_FILES.) So from those rules: // Don't use, it's the old way of doing things, unless you are just replacing a reference to $_FILES then then it's a non-issue. isset( $request->files['attachment']['name'][0] ); // Don't use, it's the old way of doing things, unless you are just replacing a reference to $_FILES then then it's a non-issue. isset( $request->files['attachment'][0]['name'] ); //Don't use. Not a reasonable collection-list-item structure. Why would "they" create a new API with a parallel array structure?!? isset( $request->uploads['attachment']['name'][0] ); // This one is golden! It's clearly new because of now $_UPLOADS existing, and it follows a reasonable collection-list-item structure isset( $request->uploads['attachment'][0]['name'] ); #jmtcw -Mike
  108618
February 16, 2020 11:49 mike@newclarity.net (Mike Schinkel)
> On Feb 15, 2020, at 3:01 AM, Larry Garfield <larry@garfieldtech.com> wrote: > > Data point: > > In PSR-7, the names used are: > > - queryParams: The query string values. > - parsedBody: The body of the message, converted to a meaningful value. If the request type is a form, then it MUST be equivalent to $_POST. If not, it's up to the particular implementation to determine what "parsed" means. (Eg, parsing a JSON body of a POST into some domain object, or whatever.) > - The raw body is a stream called "body", or rather an OOP wrapper around a stream since PHP's native stream interface is fugly. > - There's specific handling for uploadedFiles, too. > > cf: https://www.php-fig.org/psr/psr-7/ > > To the earlier point about existing implementations, while there are a myriad of older, custom implementations of abstractions around superglobals, there's only two that are not decade-old proprietary implementations: HttpFoundation and PSR-7. Those are, realistically, the only implementations that matter. Anything else would be on the same order of magnitude effort to port to one of those as to port to this proposal. In a meaningful sense, those are the only "existing competition". Both also have robust ecosystems that make leveraging them in an entirely custom app pretty straightforward. > > (Whatever your feelings of the technical merits of either design, that's the current state-of-the-market.) > > Which therefore begs the question, is this proposal intended to supplant HttpFoundation and PSR-7, or to become a common underpinning that both of them wrap, or to be a third cohabitating implementation in the ecosystem? > > It doesn't seem robust enough to supplant both of them entirely, there's little value to either HttpFoundation or PSR-7 to rewrite their guts to wrap this object (though it's easier for PSR-7, as an interface, for someone to write a new implementation of it than for HttpFoundation), which would mean we'd end up with a 3rd in-the-wild implementation for user space to keep track of. > > I am unclear how that is a market win.
The win is it allows developer to provide a simple to use object oriented interface *in core*. IOW, without having to bring in an external PHP-code implementation that may or may not break in the future. Another big win would be to allow developers to deprecate use of superglobals in their own apps. However Paul do not want to add an ini setting to disable the use of superglobals. But how could disabling superglobals be done in HttpFoundation or PSR-7? Maybe Paul will reconsider adding such an capability to his RFC because it is something we cannot get with HttpFoundation and PSR-7. Or maybe we should be looking at is a core implementation of PSR-7 instead; one that would allow us to disable access to the superglobals? One that people could subclass, of course. If we did that, we might hash out why some developers do not use PSR-7 and possibly fix its (perceived?) faults in a new PSR to amend PSR-7. -Mike
  108540
February 13, 2020 15:16 pmjones@pmjones.io ("Paul M. Jones")
Hi Niklas,

> On Feb 12, 2020, at 12:20, Niklas Keller <me@kelunik.com> wrote: > > I think the request / response API is entirely fine being solved in > userland instead of in php-src. However, since you already made such a > proposal, I want to give some feedback:
Re: userland, I gave a counterargument in my reply to Mark Randall. Even so, thanks for providing feedback in spite of that objection; I appreciate your time and effort.
> Naming > > I think we shouldn't take over the naming of the super globals, e.g. > $_GET really contains the query parameters and has nothing to do with > GET or POST, so $request->getQueryParameter(...) would be a better > name.
/me nods Yeah, naming is one of the hard problems. I considered $query as an alternative property name for $get, but in the end, the `$_GET => $get` symmetry was too great to ignore. If others here feel that $query is a better name for `$_GET` than $get, I will submit to consensus on that point. Using a getQueryParameter() method strikes a little too close to PSR-7, and thereby to charges of favoritism. But let's say we do use a method instead of a property here, call it getQuery(); then, of the following .... $foo = $request->getQuery()['foo']; // vs $foo = $request->query['foo']; .... the latter (using a property) looks and feels more appropriate to me. Thus, the RFC specifies properties over methods for ServerRequest.
> Type Safety > > I think the API should be type safe. Currently $request->get['key'] > can be null | string | string[] | ... Most parameters only appear a > single time, so a method returning the first parameter value or null > could be used instead.
This sounds a little like using `$_REQUEST` instead of `$_GET`, `$_POST`, and `$_COOKIES`. Using the "first" parameter would then depend on the order in which the superglobals get entered into the ServerRequest object, and/or we're in the business of reading and honoring the `variables_order` INI setting, at which point the logic starts sounding rather involved. So while I do get the desire for type-safety, I'd prefer to avoid all that intricacy, and go with something much closer to what PHP itself already does. That is, read $get for $_GET, $post for $_POST, etc., and just go with what is stored there.
> API Issues > > I don't see any reason to keep specials such as > $_SERVER['HTTP_X_REQUESTED_WITH'] vs. $request->requestedWith, which > is just another HTTP header.
I get that; $requestedWith in 1.x was a boolean $xhr, to let you know if `$_SERVER['HTTP_X_REQUESTED_WITH'] === 'XmlHttpRequest'`. It struck me that there might be different values in the future, and so moved to just $requestedWith instead. I am not attached to that particular property; if others agree, I am OK with removing it.
> If there's $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] => $request->method > and $_SERVER['REQUEST_METHOD'] => $request->method, how can I get the > original (actual) method?
The $method property is a calculated value: if there is a method override on a POST, $method is the override; otherwise, it is the "actual" method. So: - $request->server['REQUEST_METHOD'] is the original (actual) method, - $request->server['HTTP_X_METHOD_OVERRIDE'] is the override method, and - $request->method is the "calculated" value between them. You can see the code here: https://github.com/pmjones/ext-request/blob/2.x/php_request.c#L147-L175
> Given 'echo $content; => $response->setContent($content);', shouldn't > this rather be something like `addContent()`?
That looks like poor describing on my part in the RFC. It is more true to say that these are equivalent: echo $content; // => $response->setContent($content); $responseSender->send($response); I will try to make that more apparent in the RFC.
> How does streaming responses work? There's ServerResponseSender, but I think this should > be part of the Response API.
Here's an example: $fh = fopen('/path/to/file.ext', 'rb'); $response->setContent($fh); // ... $responseSender->send($response); When the ServerResponseSender calls $response->getContent() and detects a resource, it calls rewind() on that resource, then fpassthru(). That should stream through nicely. For more information, please see the ServerResponseSender methods at <https://github.com/pmjones/ext-request#methods-2> under the sendContent() bullet: • If the content is a resource, it is sent using rewind() and then fpassthru(). • If the content is a callable object or closure, it is invoked, and then its return value (if any) is echoed as a string; note that object returns will be cast to string at this point, invoking the __toString() method if present. • Otherwise, the content is echoed as a string; note that objects will be cast to string at this point, invoking the __toString() method if present. There are limitations to that approach, but experience has shown that it covers the vast majority of common requirements.
> The Request object should be mutable, e.g. you might want to change > the client IP to be based on a x-forwarded-header instead of the TCP > source address.
That's a great example. First, note that ServerRequest avoids setting a $clientIp property. Further, *extensions* to the ServerRequest object might well be mutable. So, to go with your example, you would be well within bounds to do something like this: class MyServerRequest extends ServerRequest { private $clientIp; public function __construct(array $globals, ?string $content = null) { parent::__construct($globals, $content); if ($this->forwarded) { $this->clientIp = // ... } } public function getClientIp() { return $this->clientIp; } } You could do that for all your custom calculations on a ServerRequest object.
> Other Commentary > >> A: It supports async exactly as much as PHP itself does. > > Not really. PHP has built-in stream_select / non-blocking streams, so > supports the tools required for async out of the box.
I will address this separately, per the resolution of our private email conversation. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108652
February 18, 2020 09:24 come.chilliet@fusiondirectory.org (=?ISO-8859-1?Q?C=F4me?= Chilliet)
Le jeudi 13 février 2020, 09:16:49 CET Paul M. Jones a écrit :
> Yeah, naming is one of the hard problems. I considered $query as an alternative property name for $get, but in the end, the `$_GET => $get` symmetry was too great to ignore. If others here feel that $query is a better name for `$_GET` than $get, I will submit to consensus on that point.
query is definitely better than get. Regarding post, I’m fine with body, parsedBody and input. I get the idea of input to mimic php://input, but if I understand things correctly, php://input is raw body, while $request->post is parsed body, so naming them alike might actually cause confusion?
> > Given 'echo $content; => $response->setContent($content);', shouldn't > > this rather be something like `addContent()`? > > That looks like poor describing on my part in the RFC. It is more true to say that these are equivalent: > > echo $content; > > // => > > $response->setContent($content); > $responseSender->send($response); > > I will try to make that more apparent in the RFC.
I still do not understand this. echo adds content to the response, it does not replace it. So the equivalent function should be $response->addContent. I would expect $response->setContent to replace the content. Can you explicit behavior for this: $response->setContent("a\n"); $response->setContent("b\n"); $responseSender->send($response); Compared to echo "a\n"; echo "b\n"; -- Côme Chilliet FusionDirectory - https://www.fusiondirectory.org
  108657
February 18, 2020 13:33 pmjones@pmjones.io ("Paul M. Jones")
Hi Côme,

> On Feb 18, 2020, at 03:24, Côme Chilliet chilliet@fusiondirectory.org> wrote: > > Le jeudi 13 février 2020, 09:16:49 CET Paul M. Jones a écrit : > >> Yeah, naming is one of the hard problems. I considered $query as an alternative property name for $get, but in the end, the `$_GET => $get` symmetry was too great to ignore. If others here feel that $query is a better name for `$_GET` than $get, I will submit to consensus on that point. > > query is definitely better than get.
Excellent.
> Regarding post, I’m fine with body, parsedBody and input. > > I get the idea of input to mimic php://input, but if I understand things correctly, php://input is raw body, while $request->post is parsed body, so naming them alike might actually cause confusion?
Might, might not. I don't think there is any "good" name here, only names that are less-bad than others.
> I still do not understand this. > echo adds content to the response, it does not replace it. > So the equivalent function should be $response->addContent. > > I would expect $response->setContent to replace the content.
Ah, I see what you are getting at now ...
> Can you explicit behavior for this: > > $response->setContent("a\n"); > $response->setContent("b\n"); > $responseSender->send($response); > > Compared to > > echo "a\n"; > echo "b\n";
.... the output would be "b\n". As you say, setContent() replaces whatever content is already in the ServerResponse. While the comparison for a single echo is accurate, the comparison for multiple echoes would be: $content = "a\n"; $content .= "b\n"; $response->setContent($content); $responseSender->send($content); Does that help to clarify? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108659
February 18, 2020 14:10 come.chilliet@fusiondirectory.org (=?ISO-8859-1?Q?C=F4me?= Chilliet)
Le mardi 18 février 2020, 07:33:37 CET Paul M. Jones a écrit :
> ... the output would be "b\n". As you say, setContent() replaces whatever content is already in the ServerResponse. While the comparison for a single echo is accurate, the comparison for multiple echoes would be: > > $content = "a\n"; > $content .= "b\n"; > $response->setContent($content); > $responseSender->send($content); > > Does that help to clarify?
Yes, but to me that means we also need an addContent method. Otherwise people will have to carry a global $content along side $response, or use setContent(getContent().$additionalContent). -- Côme Chilliet FusionDirectory - https://www.fusiondirectory.org
  108660
February 18, 2020 14:56 pmjones@pmjones.io ("Paul M. Jones")
> On Feb 18, 2020, at 08:10, Côme Chilliet chilliet@fusiondirectory.org> wrote: > > to me that means we also need an addContent method.
I can see why we'd think that; it's symmetrical, if nothing else. Even so, none of the researched implementations have a method like that. As far as I can recall, they call have setContent() and getContent() equivalents, but no addContent() equivalent. They all work much like you point out here ...
> Otherwise people will have to carry a global $content along side $response, or use setContent(getContent().$additionalContent).
.... although usually it's not a global $content. Instead, the $content is built up from a template or other subsystem of some sort, and then assigned to the response when complete. For example: $content = $template->render(); $response->setContent($content); So, I am reluctant to add something that no other implementations, across many years and many authors, have actually found a need for. Any further thoughts on this? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108732
February 23, 2020 21:53 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

> On Feb 12, 2020, at 12:20, Niklas Keller <me@kelunik.com> wrote: > >> A: It supports async exactly as much as PHP itself does. > > Not really. PHP has built-in stream_select / non-blocking streams, so > supports the tools required for async out of the box.
Per private conversation with Niklas, he notes some future conditions under which the API as presented might work with async (e.g. the arrival of fibers) -- but until that time, async is best left out-of-scope. I have updated the RFC to that effect. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108518
February 12, 2020 21:27 php.lists@allenjb.me.uk (AllenJB)
This might be more a "future scope" thing, but I would like to see a way 
to access the raw request body as part of this. Currently (as far as I 
know) the only way is to open/read php://input, which isn't particularly 
intuitive in my opinion.

AllenJB

On 10/02/2020 16:18, Paul M. Jones wrote:
> Hi Internals, > > After a couple of years of incubation, I am happy to offer a second version of this RFC: > > https://wiki.php.net/rfc/request_response > > It proposes an object-oriented approach around request and response functionality already existing in PHP, in order to reduce the global-state problems that come with superglobals and the various response-related functions. > > The SQLite "about" page says, "Think of SQLite not as a replacement for Oracle but as a replacement for fopen()." <https://www.sqlite.org/about.html> Likewise, think of this RFC not as a replacement for HttpFoundation or PSR-7, or as a model of HTTP messages, but as an object-oriented alternative to superglobals, header(), setcookie(), setrawcookie(), and so on. > > Thanks in advance for your time and consideration while evaluating it. > >
  108524
February 12, 2020 22:46 pmjones@pmjones.io ("Paul M. Jones")
Hi Allen & Robin,

Allen, you asked:

> On Feb 12, 2020, at 15:27, AllenJB lists@allenjb.me.uk> wrote: > > This might be more a "future scope" thing, but I would like to see a way to access the raw request body as part of this. Currently (as far as I know) the only way is to open/read php://input, which isn't particularly intuitive in my opinion.
Robin asked the same thing earlier:
> On Feb 11, 2020, at 08:24, Kingsquare.nl - Robin Speekenbrink <robin@kingsquare.nl> wrote: > > What i haven't clearly seen in the RFC nor the interfaces of the proposed objects: how will this handle PUT / php:://input / raw posted data? Or am i missing something?
Answer: the php://input stream is accessible via the ServerRequest $content property; see <https://github.com/pmjones/ext-request#content-related>. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108529
February 13, 2020 06:45 ruudboon@php.net (Ruud Boon)
--Apple-Mail=_9C16F5E4-0018-4038-86FB-47C751F8EEE0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

Hi!

I really like this RFC.
> On 12 Feb 2020, at 23:46, Paul M. Jones <pmjones@pmjones.io> wrote: >=20 > Hi Allen & Robin, >=20 > Allen, you asked: >=20 >> On Feb 12, 2020, at 15:27, AllenJB lists@allenjb.me.uk> wrote: >>=20 >> This might be more a "future scope" thing, but I would like to see a = way to access the raw request body as part of this. Currently (as far as =
I know) the only way is to open/read php://input, which isn't = particularly intuitive in my opinion.
>=20 >=20 > Robin asked the same thing earlier: >=20 >> On Feb 11, 2020, at 08:24, Kingsquare.nl - Robin Speekenbrink = <robin@kingsquare.nl> wrote:
>>=20 >> What i haven't clearly seen in the RFC nor the interfaces of the = proposed objects: how will this handle PUT / php:://input / raw posted =
data? Or am i missing something?
>=20 > Answer: the php://input stream is accessible via the ServerRequest = $content property; see =
<https://github.com/pmjones/ext-request#content-related>. In the the RFC I see that $content will return php://input = <php://input> using file_get_contents on the fly. Can we add the = possibility to get a reference to the stream as well? This allows the = use of stream_copy_to_stream, mime_content_type etc without reading the = fill input to memory.
>=20 >=20 > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com >=20 > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp >=20 > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php >=20 > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php Cheers,
Ruud --Apple-Mail=_9C16F5E4-0018-4038-86FB-47C751F8EEE0 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii Hi!

I really = like this RFC. 
On 12 Feb 2020, at 23:46, Paul M. Jones = <mailto:pmjones@pmjones.io" = class=3D"">pmjones@pmjones.io> wrote:

Hi = Allen & Robin,

Allen, you asked:

On Feb = 12, 2020, at 15:27, AllenJB <mailto:php.lists@allenjb.me.uk"= class=3D"">php.lists@allenjb.me.uk> wrote:

This might be more a "future scope" thing, but I would like = to see a way to access the raw request body as part of this. Currently = (as far as I know) the only way is to open/read php://input" = class=3D"">php://input, which isn't particularly intuitive in my = opinion.


Robin = asked the same thing earlier:

On Feb 11, 2020, at 08:24, http://Kingsquare.nl" class=3D"">Kingsquare.nl - Robin = Speekenbrink <mailto:robin@kingsquare.nl" = class=3D"">robin@kingsquare.nl> wrote:

What i haven't clearly seen in the RFC nor the interfaces of = the proposed objects: how will this handle PUT / php:://input / raw = posted data? Or am i missing something?

Answer: the php://input" class=3D"">php://input = stream is accessible via the ServerRequest $content property; see <https://github.com/pmjones/ext-request#content-related" = class=3D"">https://github.com/pmjones/ext-request#content-related>.=
In the the RFC I see that = $content will return  php://input" = class=3D"">php://input using file_get_contents on the fly. = Can we add the possibility to get a reference to the stream as well? = This allows the use of stream_copy_to_stream, mime_content_type etc = without reading the fill input to memory.



--
Paul M. Jones
mailto:pmjones@pmjones.io" class=3D"">pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving= the N+1 Problem in PHP
https://leanpub.com/sn1php

--
PHP Internals - PHP Runtime = Development Mailing List
To unsubscribe, visit: = http://www.php.net/unsub.php
Cheers,
Ruud
= --Apple-Mail=_9C16F5E4-0018-4038-86FB-47C751F8EEE0--
  108541
February 13, 2020 16:08 pmjones@pmjones.io ("Paul M. Jones")
Hi Ruud,


> On Feb 13, 2020, at 00:45, Ruud Boon <ruudboon@php.net> wrote: > > Hi! > > I really like this RFC.
Glad to hear it!
> In the the RFC I see that $content will return php://input using file_get_contents on the fly. Can we add the possibility to get a reference to the stream as well?
By "get a reference" I presume you mean "an open file handle" (a la `return fopen('php://input', 'rb')`). In which case: maybe? I can see where that would be useful for extremely large request bodies in a memory-constrained environment. But the question becomes: is that needed frequently-enough across a wide-enough range of PHP developers to warrant inclusion in this RFC? I don't recall seeing any userland implementation from my research provide such a thing. That makes me think it's a comparatively infrequent need. If many others disagree with that assessment, I'm happy to entertain the possibility of adding something like it to ServerRequest. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108664
February 19, 2020 14:06 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

Per the past several days of discussion, John Boehr and I have modified the proposal:

- ServerRequest

    - The `$env` property mapping to `$_ENV` has been removed

    - The `$requestedWith` property mapping to `$_SERVER['HTTP_X_REQUESTED_WITH']` has been removed

    - The superglobal `$_GET` now maps to the property `$query` (instead of `$get`)

    - The superglobal `$_POST` now maps to the property `$input` (instead of `$post`)

- A new interface, ServerResponseInterface, is now available; ServerResponse implements it

There are still outstanding comments and questions yet to be resolved.

Thanks to everyone who has participated thus far!


-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php
  108720
February 22, 2020 15:23 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

Off-list discussion with some vendors and consumers indicates that adoption and migration might be eased by the following changes to ServerResponseInterface:

- Add the following methods:

    - `getHeader(string $label) : ?string`
    - `hasHeader(string $label) : bool`
    - `getCookie(string $name) : ?array`
    - `hasCookie(string $name) : bool`

- Modify `add*()`, `set*()`, and `unset*()` to be fluent (allow method chaining)

Since these changes fit in well with the RFC goals, and do not substantially modify prior usage expectations, John Boehr has applied them to the extension, with documentation and tests.

Further, one evaluator noted that ServerResponseSender::sendContent() did not allow for iterables/generators as content; that has been remedied as well.


-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php
  108702
February 20, 2020 15:26 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

One of the "open questions" on this RFC is: are the class names ServerRequest, ServerResponse, and ServerResponseSender "good enough" for our purposes, or are there names that are "better" in some qualitative way?

The original thought was Request and Response, but I thought it might be too easy to think of them as client-related, rather than server-related. Then I tried PhpRequest and PhpResponse, but having "PHP" in the name seemed unnecessary, as this is PHP after all. I settled on ServerRequest and ServerResponse to point out that they are server-related.

Having said that, would something like, say, RequestContext and ResponseBuffer (with ResponseSender) be "better" somehow? Or perhaps some other names? Or are readers here satisfied that the existing names are sufficient?

And thanks to the many folks here who have already provided such valuable feedback!


-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php
  108708
February 21, 2020 00:42 mike@newclarity.net (Mike Schinkel)
> On Feb 20, 2020, at 10:26 AM, Paul M. Jones <pmjones@pmjones.io> wrote: > > One of the "open questions" on this RFC is: are the class names ServerRequest, ServerResponse, and ServerResponseSender "good enough" for our purposes, or are there names that are "better" in some qualitative way? > > Having said that, would something like, say, RequestContext and ResponseBuffer (with ResponseSender) be "better" somehow? Or perhaps some other names? Or are readers here satisfied that the existing names are sufficient?
I would pick the latter. They are — to me — more descriptive of actually what they are accomplishing than the former. -Mike
  108721
February 22, 2020 15:26 pmjones@pmjones.io ("Paul M. Jones")
> On Feb 20, 2020, at 18:42, Mike Schinkel <mike@newclarity.net> wrote: > >> On Feb 20, 2020, at 10:26 AM, Paul M. Jones <pmjones@pmjones.io> wrote: >> >> One of the "open questions" on this RFC is: are the class names ServerRequest, ServerResponse, and ServerResponseSender "good enough" for our purposes, or are there names that are "better" in some qualitative way? >> >> Having said that, would something like, say, RequestContext and ResponseBuffer (with ResponseSender) be "better" somehow? Or perhaps some other names? Or are readers here satisfied that the existing names are sufficient? > > > I would pick the latter. They are — to me — more descriptive of actually what they are accomplishing than the former.
Thanks for that. As it hasn't seemed to pique anyone else's interest, maybe I will add this as a secondary vote, conditional on the primary one passing. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108790
February 28, 2020 15:25 pmjones@pmjones.io ("Paul M. Jones")
Hi everyone,

All outstanding issues on this RFC appear to be resolved one way or another. With that in mind:

Are there any members here who currently expect to vote "no", who have not yet chimed in? I'd like to hear your criticisms and objections, so I can at least attempt to resolve your concerns.

I am especially interested to hear about technical or developer-experience shortcomings, but of course variations on "this is better left to userland" are understandable.

Again, and as always, my thanks for your time and attention.


-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php
  108837
March 3, 2020 22:58 Danack@basereality.com (Dan Ackroyd)
Paul M. Jones <pmjones@pmjones.io> wrote:

> Are there any members here who currently expect to vote "no",
The only reason for doing this as C code appears to be make it have read-only properties. This is a hack that would confuse a lot of developers, and instantly be added to most lists of "how PHP is bad". I really think this doesn't belong in core PHP. Tying this into core means that: - BC breaking changes could only happen on major versions. - Discussion of those changes would take up a lot of time, or more likely never happen. See all of our core extensions that are desperately in need of a maintainer to work on them. - It might be impossible (or at least annoyingly difficult) to write code that was compatible with two versions of PHP, if they used this object. As you were involved and so know, a huge amount of energy was spent designing PSR-7, and still it was definitely not a perfect design. The chances of the design of this API being even as close as PSR-7 was, seems minimal. Which means that we would be likely to regret bringing it into core. For stuff to be added to core, to overcome the burden of supporting in the future, there needs to be a strong reason to add it. Not just something that could be done. In case anyone is interested, there is a book called 'Systemantics' that's actually quite applicable to open source projects....despite being written before personal computers were really a thing: https://en.wikipedia.org/wiki/Systemantics#First_principles https://www.amazon.co.uk/Systems-Bible-Beginners-Guide-Large/dp/0961825170/ref cheers Dan Ack
  108838
March 3, 2020 23:45 larry@garfieldtech.com ("Larry Garfield")
On Tue, Mar 3, 2020, at 4:58 PM, Dan Ackroyd wrote:
> Paul M. Jones <pmjones@pmjones.io> wrote: > > > Are there any members here who currently expect to vote "no", > > The only reason for doing this as C code appears to be make it have > read-only properties. This is a hack that would confuse a lot of > developers, and instantly be added to most lists of "how PHP is bad". > > I really think this doesn't belong in core PHP. Tying this into core means that: > > - BC breaking changes could only happen on major versions. > > - Discussion of those changes would take up a lot of time, or more > likely never happen. See all of our core extensions that are > desperately in need of a maintainer to work on them. > > - It might be impossible (or at least annoyingly difficult) to write > code that was compatible with two versions of PHP, if they used this > object. > > As you were involved and so know, a huge amount of energy was spent > designing PSR-7, and still it was definitely not a perfect design. The > chances of the design of this API being even as close as PSR-7 was, > seems minimal. Which means that we would be likely to regret bringing > it into core. > > For stuff to be added to core, to overcome the burden of supporting in > the future, there needs to be a strong reason to add it. Not just > something that could be done. > > In case anyone is interested, there is a book called 'Systemantics' > that's actually quite applicable to open source projects....despite > being written before personal computers were really a thing: > https://en.wikipedia.org/wiki/Systemantics#First_principles > https://www.amazon.co.uk/Systems-Bible-Beginners-Guide-Large/dp/0961825170/ref > > cheers > Dan > Ack
I don't have a vote, but here are my thoughts (and speaking only for myself, not on behalf of FIG or any other group): Right now, there are effectively 3 ways of handling request/response information in PHP: 1) Raw super globals, possibly with proprietary bits on top. 2) HttpFoundation 3) PSR-7 Each has its own sub-ecosystem around it, which means that there are effectively 3 different, incompatible HTTP ecosystems. (Or 2, depending on if you count the super globals as having an ecosystem.) Paul has made it very clear that the goal of the RFC is not to supplant HttpFoundation or PSR-7, and there's no sign of plans to remove the superglobals any time soon (that would take at least 5 years/1 major of deprecation to phase out, even if we wanted to). Being a better underpinning for HttpFoundation/PSR-7 is a non-goal of the RFC, and I agree it wouldn't really buy them much compared to pulling data from the superglobals. That means there would effectively become 4 mini-ecosystems: HttpFoundation, PSR-7, superglobals, and ServerRequest. If you only ever work on one framework or application, that doesn't really impact you much. If you work on multiple, or are developing a library you want to be as agnostic as possible, or if you're just learning PHP, the addition of a 4th mini-ecosystem is a negative. The RFC does, however, have some positives to it. In particular: * objects really are nicer to work with (subjective, but I usually find that to be the case) * they discourage global behavior * ServerRequest is read-only which avoids some bugs (but may make other things more difficult) * using it exclusively gives you a nicer way to avoid buffer and header-already-sent issues All of those could be said about PSR-7, of course, and all but the read-only point could be said about HttpFoundation. The key question, then, is if on balance the positives listed above outweighs the negative of yet-another-mini-ecosystem around HTTP handling, does so by a large enough margin to justify its inclusion. That is a balance question on which reasonable people can disagree. For my part, I feel it is on net a negative, and thus should not be approved. I am not against improving PHP's native HTTP handling; on the contrary, I would be very much in favor of a proposal that goes much further, with the aim that it can eventually deprecate PSR-7 and HttpFoundation, or at least large portions of them. That is a substantially higher lift, of course; it would likely need to start with overhauling PHP's stream handling to make it cleaner and more OOPy, something that has been discussed on and off for years but with no serious movement yet. That would be a topic for another RFC. --Larry Garfield
  108852
March 4, 2020 20:01 pmjones@pmjones.io ("Paul M. Jones")
Hi Dan,

Thanks for taking the time to bring up your concerns. While I don't expect to change your mind (nice though that would be!) perhaps other readers here will find my counter-positions useful.


> On Mar 3, 2020, at 16:58, Dan Ackroyd <Danack@basereality.com> wrote: > > Paul M. Jones <pmjones@pmjones.io> wrote: > >> Are there any members here who currently expect to vote "no", > > The only reason for doing this as C code appears to be make it have > read-only properties.
Not the *only* reason, though it's true that it would be difficult-to-impossible to do outside of C. As Rowan mentioned earlier, and as I opined in an unpublished version of the RFC, one other reason would be to provide a language-level OO-ish representation of PHP functions, similar to what ext/mysqli and ext/date do.
> This is a hack
I dispute the word "hack" -- a readonly feature is requested here from time to time -- though I do understand that it is an uncommon approach.
> that would confuse a lot of developers
I think you're right that it might confuse some developers, but I also opine that just as many or more would "get it" either right away or very quickly.
> and instantly be added to most lists of "how PHP is bad".
Adding one point to the many thousands that already exist on such ill-considered lists, so not much of a change there. ;-)
> Tying this into core means that: > > - BC breaking changes could only happen on major versions. > > - Discussion of those changes would take up a lot of time, or more > likely never happen. See all of our core extensions that are > desperately in need of a maintainer to work on them. > > - It might be impossible (or at least annoyingly difficult) to write > code that was compatible with two versions of PHP, if they used this > > ... > > For stuff to be added to core, to overcome the burden of supporting in > the future, there needs to be a strong reason to add it. Not just > something that could be done.
All of your points are completely true. In fact, they are true for every RFC presented on this list; past, present, and future. So while you're correct, I have a hard time seeing it as a criticism specific to this proposal. (I'd be happy to see refinement of your points toward more specificity, if feel so inclined.) Having said that, I'll present a past example that may tie it more specifically to this RFC. The Same Site Cookie proposal <https://wiki.php.net/rfc/same-site-cookie> hits around the points you mention: a change to long-standing behavior that might have entailed a BC break, discussion of that change, and code compatibility. All of those were managed quite successfully and in a reasonable timeframe. And although you voted against that proposal, it did pass, and has turned out to be pretty good; I imagine the same kind of change-success, if it is ever needed after adoption, is just as likely for this RFC as well.
> As you were involved and so know, a huge amount of energy was spent > designing PSR-7, and still it was definitely not a perfect design. The > chances of the design of this API being even as close as PSR-7 was, > seems minimal.
I know you're not especially a fan of PSR-7 <http://basereality.com/blog/17/PSR7_and_airing_of_grievances> so arguing in its defense, against interest, is laudable. Good form! But your phrasing about this RFC "being even as close as PSR-7 was" raises the question: "as close" to what? The PSR-7 interfaces, and their many differing implementations, attempt to model HTTP messages as immutable objects. Indeed, the PSR-7 design process ended up discounting and discarding all pre-existing ways-of-working in PHP land, to the point of ignoring all previous implementations in the FIG member projects and elsewhere. Instead, PSR-7 declared "year zero" and presented an entirely new way of working that was completely different from and unrelated to anything else in PHP-land at that time. (For a brief history of the PSR-7 design evolution, see the first few slides of my talk on PSR-7 and ADR at <http://paul-m-jones.com/talks/psr7adr.pdf>.) Among other things, that meant there was no applicable real-world experience as to how well PSR-7 would work in the mid-to-long term. That historical context may make it easier to see why "a huge amount of energy was spent designing PSR-7", while resulting in something that was "definitely not a perfect design", in order to get close to a model of HTTP messages. This RFC, in contrast, does not attempt to model HTTP messages. It does not attempt to discard previous ways of working. Instead, it proposes a more object-oriented representation of functionality that already exists in PHP, honoring that previously-existing approach. There is quite a bit of real-world experience as to how well it will work, since it takes into account many commonalities between existing userland projects. Thus, what the RFC purports to get close to is that existing way-of-working, and I think it succeeds about as well as can be possible for its goals.
> In case anyone is interested, there is a book called 'Systemantics' > that's actually quite applicable to open source projects....despite > being written before personal computers were really a thing: > https://en.wikipedia.org/wiki/Systemantics#First_principles > https://www.amazon.co.uk/Systems-Bible-Beginners-Guide-Large/dp/0961825170/ref
I will add this to my reading backlog; thanks for the recommendation! -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108858
March 4, 2020 23:13 rowan.collins@gmail.com (Rowan Tommins)
On 04/03/2020 20:01, Paul M. Jones wrote:
> This RFC, in contrast, does not attempt to model HTTP messages. It does not attempt to discard previous ways of working. Instead, it proposes a more object-oriented representation of functionality that already exists in PHP, honoring that previously-existing approach. There is quite a bit of real-world experience as to how well it will work, since it takes into account many commonalities between existing userland projects. Thus, what the RFC purports to get close to is that existing way-of-working, and I think it succeeds about as well as can be possible for its goals.
This is something you have said a few times, and is definitely a worthy goal, but it's not something that comes across very strongly in the RFC itself. There is a comparison to Symfony HttpFoundation, but it mentions as many differences as similarities; and there is a list of 13 other implementations, but no information on how this proposal compares to them. I suspect that's just because you didn't want to burden the RFC with too many details, but if you have any notes about what functionality is common in existing libraries, perhaps that could be posted somewhere as a kind of appendix to show where the current design came from? Regards, -- Rowan Tommins (né Collins) [IMSoP]
  108889
March 7, 2020 16:26 pmjones@pmjones.io ("Paul M. Jones")
Hi Rowan,

> On Mar 4, 2020, at 17:13, Rowan Tommins collins@gmail.com> wrote: > >> On 04/03/2020 20:01, Paul M. Jones wrote: >> >> This RFC, in contrast, does not attempt to model HTTP messages. It does not attempt to discard previous ways of working. Instead, it proposes a more object-oriented representation of functionality that already exists in PHP, honoring that previously-existing approach. There is quite a bit of real-world experience as to how well it will work, since it takes into account many commonalities between existing userland projects. Thus, what the RFC purports to get close to is that existing way-of-working, and I think it succeeds about as well as can be possible for its goals. > > This is something you have said a few times, and is definitely a worthy goal, but it's not something that comes across very strongly in the RFC itself. There is a comparison to Symfony HttpFoundation, but it mentions as many differences as similarities; and there is a list of 13 other implementations, but no information on how this proposal compares to them. > > I suspect that's just because you didn't want to burden the RFC with too many details, but if you have any notes about what functionality is common in existing libraries, perhaps that could be posted somewhere as a kind of appendix to show where the current design came from?
Your suspicion is correct; I thought the RFC was pretty long as it was. And your suggestion is a good one; here is a Google Sheets doc of summary findings (copied from my local spreadsheets): https://docs.google.com/spreadsheets/d/e/2PACX-1vQzJP00bOAMYGSVQ8QIIJkXVdAg-OMEfkgna7-b2IsuoWN8x_TazxEYn-yVDF2XQIqnzmHqdDO3KEKx/pubhtml When you see $foo, it means the feature is presented as a property, or as a method on a public property object. When you see foo(), it means the feature is presented as a public method on the object itself. Other notes: 1. On the Request and Response objects, I have a line for "nominal public mode": - A nominal "readonly" is for the superglobal-derived public properties. The object might include one or two mutable properties, but those properties are application-specific, not superglobal-derived. Gurther, the superglobal-derived properties might be mutable within a protected scope but not a public one. - A nominal "mutable" means the superglobal-derived values are mutable from public scope. - A nominal "immutable" means the object is intended to be immutable (whether it succeeds or not). - Of the 14 projects, 8 have nominally readonly Request objects, and all but one Response object are mutable. 2. Several projects retain "Forwarded" information internally on the Request objects, but almost none of them expose it publicly. As it appears to be an commonly-calculated set of values, I did include it in ServerRequest, and made it available for public reading, so that extenders/decorators can use it. 3. Likewise, several projects expose a "client IP" or "remote address" value, but don't calculate it the same way. So even though it's a common need, and I'd like to have it in ServerRequest, there's not broad agreement on what that value ought to be. 4. Finally, several projects have some kind of Response cache methods, usually in the form of setCache() etc., and then often with individual cache directive methods. I recall that they differed in how they retained the caching information; some wrote to headers directly, others retained the directives individually and "compiled" them into a caching header at sending time, etc. So while I would like to incorporate the caching stuff into ServerResponse, I don't think there's enough commonality between the implementations to warrant that. I hope the rest of it is self-explanatory, or easily-understood; if not, let me know, and I'll answer your questions. And of course, if you notice discrepancies or inaccuracies, tell me so I can correct them. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108860
March 5, 2020 09:00 phpmailinglists@gmail.com (Peter Bowyer)
On Fri, 28 Feb 2020 at 15:25, Paul M. Jones <pmjones@pmjones.io> wrote:

> Are there any members here who currently expect to vote "no", who have not > yet chimed in? I'd like to hear your criticisms and objections, so I can at > least attempt to resolve your concerns. >
I expect to vote "no". My thoughts are: 1. The chosen API. We have an OO approach, but headers and query parameters are accessed through an array-style mechanism. I'm not a fan of the way everything maps onto ServerRequest, with certain $_SERVER vars promoted to be part of it (but not all AFAICT). 2. The OO-ish approach. For a core feature which will be around for 20+ years I would like a pure(r) OO approach taken, and time to be taken to get that right. What you have is pragmatic, but as I say in (1) not something I'm a fan of. Rowan Tommins on 18 Feb 2020 at 20:21 expressed this well (link: https://externals.io/message/108436#108661). 3. WRT the RFC: a) there is no section on where this came from. With your involvement in PSR-7 it looks to be an area you've given much thought to. Why did you develop ext/request? What problem(s) did you see that this is the answer to? I skimmed the thread but didn't pick out a clear answer to this. b) Did you look at how other languages and/or frameworks deal with requests and responses, and was there inspiration and lessons you took from their successes (or failures)? [For me this is a key omission in most PHP RFCs] Thinking about porting existing codebases across, ServerRequest holding copies rather than references would make it hard to interop with existing code using the superglobals. I've had this rewriting legacy apps that use $_SESSION and the new framework uses an OO session handler; it's not fun but with references can usually be made to work. In this case, what would the migration path look like? Johannes Schlüter commented on this on 24 Feb but I didn't understand his comment (link: https://externals.io/message/108436#108737). In summary, I like the idea of steering people away from superglobals, appreciate the work you've put in, and am not persuaded by the approach. Peter
  108893
March 7, 2020 22:29 pmjones@pmjones.io ("Paul M. Jones")
Hi Peter,

Thanks for the detailed critique. I'm not sure I can remedy everything you've pointed out, but even so ...

> On Mar 5, 2020, at 03:00, Peter Bowyer <phpmailinglists@gmail.com> wrote: > > I expect to vote "no". My thoughts are: > > 1. The chosen API. We have an OO approach, but headers and query parameters > are accessed through an array-style mechanism. > > ... > > 2. The OO-ish approach. For a core feature which will be around for 20+ > years I would like a pure(r) OO approach taken, and time to be taken to get > that right. What you have is pragmatic, but as I say in (1) not something > I'm a fan of. Rowan Tommins on 18 Feb 2020 at 20:21 expressed this well > (link: https://externals.io/message/108436#108661).
And not just headers and query parameters, but the $_POST-derived parameters, the files, the accept-related values, and frankly everything else in ServerRequest. I really do get the objection; methods, if nothing else, are just what we're used to. The only thing I have to say in return is that if the only methods are getters on properties, that are not themselves mutable, it makes sense to just expose the properties directly. Especially so when they are arrays; then a wide range of array functions can be brought to bear on them, instead of adding more methods to duplicate array-related functionality. Having said that, what changes would you like to see on the API, that you feel might be more worthy of your vote to accept the proposal? Getter methods on ServerRequest? Something else? (I'm not making promises, but perhaps knowing what you'd prefer will give me some ideas.)
> I'm not a fan of the way everything maps onto ServerRequest, with certain $_SERVER vars promoted to be part of it (but not all AFAICT).
You're correct: not every element of $_SERVER gets promoted to a property. The `HTTP_*` elements go into $headers, and some of *those* (the ones that bear restructuring into arrays) get promoted to properties of their own. Then, once some elements get promoted to properties, it makes sense to promote related elements (e.g., once you have $contentType and $contentCharset, might as well have $contentLength et al.). What *might* you be a fan of, in terms of what gets promoted to a ServerRequest property, and what does not?
> 3. WRT the RFC: > a) there is no section on where this came from. With your involvement in > PSR-7 it looks to be an area you've given much thought to. Why did you > develop ext/request? What problem(s) did you see that this is the answer > to? I skimmed the thread but didn't pick out a clear answer to this.
This is one of those places where it's tough to "thread the needle." I want to reduce the chances of the RFC being read as a purely negative criticism of existing projects, but at the same time some level of compare-and-contrast is necessary. One set of readers wants to know what's so terrible about the existing projects, and another set of readers demands positive reasoning only within the proposal. It's a "damned if you do, damned if you don't" kind of situation. My earlier attempt at answering "what problem(s) did you see?" is blogged here: http://paul-m-jones.com/post/2017/01/05/psr-7-vs-the-serverrequestresponse-rfc/ Quoting and summarizing from there: - PSR-7 was born to answer the question, “How can we model HTTP messages in PHP for sending a request, and getting back a response?” That is, how can we standardize the model of an HTTP request message for sending, and the model of the returned the HTTP response, when using PHP as an HTTP client? ... PSR-7 [then] expanded to answer a second question: “How can we model HTTP messages for receiving a request, and sending back a response?” This is in addition to the original goal, but idea is the same: building a standard model of HTTP messages. - The proposed RFC starts out by asking a different question. It is not concerned with modeling HTTP messages, whether when sending or receiving them. Instead, it asks: “How can we take the request-related superglobals in PHP, and the various response-related global functions in PHP, and encapsulate them in objects, to make them at least a little more object-oriented?” Because the RFC begins with a different question, it leads to a different answer.
> b) Did you look at how other languages and/or frameworks deal with requests > and responses, and was there inspiration and lessons you took from their > successes (or failures)? [For me this is a key omission in most PHP RFCs]
I did look at how other frameworks in PHP handle this. Rowan very rightly pointed out that a summary of my research would be helpful, and based on his suggestion I have published this comparison document: https://docs.google.com/spreadsheets/d/e/2PACX-1vQzJP00bOAMYGSVQ8QIIJkXVdAg-OMEfkgna7-b2IsuoWN8x_TazxEYn-yVDF2XQIqnzmHqdDO3KEKx/pubhtml Here are some notes about it: https://externals.io/message/108436#108889 As far as other languages, I did the barest bit of a survey, but since the goal of this RFC is very directly tied to PHP itself, researching other frameworks in other languages was not especially fruitful.
> Thinking about porting existing codebases across, ServerRequest holding > copies rather than references would make it hard to interop with existing > code using the superglobals.
/me nods along Maybe? Certainly that's true if the existing code depends on superglobal mutability at some point past bootstrapping the application. But speaking from personal experience only, that kind of mutability dependence seems relatively rare. That is, I myself do not recall seeing applications that co-opt $_GET, $_POST, $_COOKIE, etc. to continuously modify them throughout the application. *Sometimes* I see that in $_ENV and $_SERVER, but even then it is a limited sort of thing that's easily extracted to an application-specific object. However, if an application *does* depend on that kind of thing, you're right: ServerRequest would be disconnected from the superglobals, meaning that application would not be a candidate for migration to ext/request. Do note, though, that more than half of the researched projects are nominally readonly in public scope, much like ServerRequest. As such, I am encouraged to think that the interop problem you describe is not often encountered.
> I've had this rewriting legacy apps that use > $_SESSION and the new framework uses an OO session handler; it's not fun > but with references can usually be made to work. In this case, what would > the migration path look like? Johannes Schlüter commented on this on 24 Feb > but I didn't understand his comment (link: > https://externals.io/message/108436#108737).
$_SESSION and the session-related functions are a whole different beast, one that this RFC intentionally avoids. Much as I like ext/session, it combines too many different concerns. As you say, you cannot simply make a copy of $_SESSION and have everything else keep working; it and the session-related functions are intimately intertwined. But leaving aside $_SESSION, I opine that the migration path in a system that already treats the non-session superglobals in a read-only fashion might be tedious but not difficult. For example: 1. In your setup code, after you have made any changes to the superglobals that your application requires, instantiate ServerRequest via whatever object-sharing system you have in place (whether by DI container or by any other approach). 2. In one script (or function, or class) inject or otherwise acquire the ServerRequest instance; then, line by line, replace `$_GET` with `$request->query`, etc. Repeat for the other superglobals in that script/function/class. It is *almost* at the level of automated search-and-replace. (And along the way, you might do things like replacing `isset($_GET['foo']) ? $_GET['foo'] : 'default')` with `$request->query['foo'] ?? 'default'`.) 3. Run your regression testing (or smoke-test or spot-check) and correct any errors. 4. Commit, push, and notify QA. 5. Repeat 2 & 3 using the next script/function/class. The benefit of this approach, at least if you are treating the superglobals as read-only, is that `$_GET` continues to work alongside `$request->query`, so you need not change the entire application at once. Baby steps and iterative improvement are possible, even preferred. To be clear, I have not tried this approach on any applications myself. But, it is similar to the methodology used throughout my book on modernizing legacy PHP applications, and I imagine it would be pretty close to what would actually be needed for a migration.
> In summary, I like the idea of steering people away from superglobals, > appreciate the work you've put in
Thanks for saying ...
> and am not persuaded by the approach.
.... and noted. After reading this, what modifications to the proposal *might* persuade you (if any) short of rewriting how PHP does superglobals in the first place? Regardless, my thanks for your time and effort writing a good critique. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108864
March 5, 2020 11:19 arvids.godjuks@gmail.com (Arvids Godjuks)
On Fri, 28 Feb 2020 at 16:25, Paul M. Jones <pmjones@pmjones.io> wrote:

> Hi everyone, > > All outstanding issues on this RFC appear to be resolved one way or > another. With that in mind: > > Are there any members here who currently expect to vote "no", who have not > yet chimed in? I'd like to hear your criticisms and objections, so I can at > least attempt to resolve your concerns. > > I am especially interested to hear about technical or developer-experience > shortcomings, but of course variations on "this is better left to userland" > are understandable. > > Again, and as always, my thanks for your time and attention. > > > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com > > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp > > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > Hello Paul,
one question I do have here is about how PHP-PM process manager will be able to interact with this layer? Cause right now it does rewrite the $_SERVER variable for each incoming request for its child processes that handle the requests https://github.com/php-pm/php-pm/blob/a2872e13c3901401d5c2386a8ed62502db23d5f2/src/ProcessSlave.php#L463 It kind'a makes this not possible if there is no way to re-init the object. While read-only is, obviously, a good idea, there are some caveats like this that not only limit the possibilities but also what will the ServerRequest object even look like when $_SERVER is being rewritten like this? They relly on https://github.com/php-pm/php-pm-httpkernel package to handle the GET/POST/COOKIE/SESSION data for each supported framework trying to avoid the usage of the superglobals, but I see $_COOKIE being used. This, in turn, relies on https://github.com/guzzle/psr7/ and that one reads the data from the $_GET/$_POST/$_FILES the data: https://github.com/guzzle/psr7/blob/c8676a22904ebc3143b8485364cbe9ac97dc3f3e/src/ServerRequest.php#L170 Basically to put in in a single sentence - so far it seems that this RFC does not allow such usage at all and long-running processing has not been taken into account at all? Thanks! -- Arvīds Godjuks +371 26 851 664 arvids.godjuks@gmail.com Skype: psihius Telegram: @psihius https://t.me/psihius
  108894
March 7, 2020 22:50 pmjones@pmjones.io ("Paul M. Jones")
Hi Arvids,

> On Mar 5, 2020, at 05:19, Arvids Godjuks godjuks@gmail.com> wrote: > > one question I do have here is about how PHP-PM process manager will be > able to interact with this layer? > Cause right now it does rewrite the $_SERVER variable for each incoming > request for its child processes that handle the requests > https://github.com/php-pm/php-pm/blob/a2872e13c3901401d5c2386a8ed62502db23d5f2/src/ProcessSlave.php#L463 > > It kind'a makes this not possible if there is no way to re-init the object. > While read-only is, obviously, a good idea, there are some caveats like > this that not only limit the possibilities but also what will the > ServerRequest object even look like when $_SERVER is being rewritten like > this? > > ... > > it seems that this RFC does not allow such usage at all and long-running processing has not been taken into account at all?
I have never used PHP-PM, so I am talking outside my competence here. Even so, I see the ProcessSlave::prepareEnvironment() method rewrites $_SERVER. Instead of re-initializing an existing object, would it be sufficient to create a new ServerRequest object after that point, with the newly-modified $GLOBALS values (in which $_SERVER is represented)? You would then use that new instance in the new child process. Regarding a long-running process, I could see where the entry point might create a new instance of ServerRequest as needed, using the current $GLOBALS state, and pass that new instance into whatever handler gets invoked. I am probably missing your point; if so, my apologies, and please guide me. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108986
March 11, 2020 21:22 arvids.godjuks@gmail.com (Arvids Godjuks)
On Sat, 7 Mar 2020 at 23:50, Paul M. Jones <pmjones@pmjones.io> wrote:

> Hi Arvids, > > > On Mar 5, 2020, at 05:19, Arvids Godjuks godjuks@gmail.com> > wrote: > > > > one question I do have here is about how PHP-PM process manager will be > > able to interact with this layer? > > Cause right now it does rewrite the $_SERVER variable for each incoming > > request for its child processes that handle the requests > > > https://github.com/php-pm/php-pm/blob/a2872e13c3901401d5c2386a8ed62502db23d5f2/src/ProcessSlave.php#L463 > > > > It kind'a makes this not possible if there is no way to re-init the > object. > > While read-only is, obviously, a good idea, there are some caveats like > > this that not only limit the possibilities but also what will the > > ServerRequest object even look like when $_SERVER is being rewritten like > > this? > > > > ... > > > > it seems that this RFC does not allow such usage at all and long-running > processing has not been taken into account at all? > > I have never used PHP-PM, so I am talking outside my competence here. > > Even so, I see the ProcessSlave::prepareEnvironment() method rewrites > $_SERVER. Instead of re-initializing an existing object, would it be > sufficient to create a new ServerRequest object after that point, with the > newly-modified $GLOBALS values (in which $_SERVER is represented)? You > would then use that new instance in the new child process. > > Regarding a long-running process, I could see where the entry point might > create a new instance of ServerRequest as needed, using the current > $GLOBALS state, and pass that new instance into whatever handler gets > invoked. > > I am probably missing your point; if so, my apologies, and please guide me. >
I took a deeper look into the GitHub repo and it answered a few questions - RFC just didn't give any examples of usage/instantiation and how you can make a new object. I was thinking it was a read-only global type object that lives and dies with the start and end of the request.
> > > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com > > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp > > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php > > > >
-- Arvīds Godjuks +371 26 851 664 arvids.godjuks@gmail.com Skype: psihius Telegram: @psihius https://t.me/psihius
  108987
March 11, 2020 21:31 pmjones@pmjones.io ("Paul M. Jones")
Hi Arvids,

> On Mar 11, 2020, at 16:22, Arvids Godjuks godjuks@gmail.com> wrote: > > I took a deeper look into the GitHub repo and it answered a few questions - > RFC just didn't give any examples of usage/instantiation and how you can > make a new object. I was thinking it was a read-only global type object > that lives and dies with the start and end of the request.
That's my failure, then; I will add a couple of examples like that to the RFC before calling the vote. Thanks for taking the time to examine the repo, and let me know if you think of anything else. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108983
March 11, 2020 16:36 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

Conversation on this RFC seems to have diminished. As far as I know, I have answered all criticisms/concerns/complaints one way or another.

So if there are no more questions, and there is no objection, I will plan to call the vote on this proposal some time tomorrow or Friday.

Thanks to everyone who has participated!


-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php
  108996
March 12, 2020 14:33 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

> On Mar 11, 2020, at 11:36, Paul M. Jones <pmjones@pmjones.io> wrote: > > Conversation on this RFC seems to have diminished. As far as I know, I have answered all criticisms/concerns/complaints one way or another.
FWIW, I have updated the Q&A points based on new discussion over the past day or so. The updated points are: Q: The proposal compares and contrasts with HttpFoundation and the various PSR-7 implementations; how does it compare to other projects? A: See this message for a starting point: https://externals.io/message/108436#108889. In short, the proposed functionality is representative of functionality common to the dozen-plus researched projects. Q: Are these global single-instance objects? A: No, you can create as many instances as you like, in whatever scopes you like. Q: Do these objects replace the superglobals? A: No. Q: Do these objects deal with $_SESSION and the session functions? A: No; it is explicitly out of scope for this RFC. Q: Readonly properties are unusual for PHP. A: Granted, though not unheard of. PdoStatement::$queryString is one precedent here. Further, of the researched userland projects, more than half of them present the relevant superglobals as nominally readonly in the public scope, making readonly access a reasonable choice here. Q: Why is ServerRequest readonly, and ServerResponse mutable? A: It makes sense that you would not want to change what you have received as a request; however, as you are in charge of creating the response, modifying it as needed seems reasonable. Further, the “readonly request with mutable response” matches half or more of the researched userland projects. Q: What would a migration path look like? A: Something like the one outlined in the later portion of this message: https://externals.io/message/108436#108893 -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  108998
March 13, 2020 07:41 php-lists@koalephant.com (Stephen Reay)
> On 11 Mar 2020, at 23:36, Paul M. Jones <pmjones@pmjones.io> wrote: > > Hi all, > > Conversation on this RFC seems to have diminished. As far as I know, I have answered all criticisms/concerns/complaints one way or another. > > So if there are no more questions, and there is no objection, I will plan to call the vote on this proposal some time tomorrow or Friday. > > Thanks to everyone who has participated! > > > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com > > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp > > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Hi Paul, I appreciate what this is trying to achieve (I think - like others I’ve written user land wrappers that achieve similar things, so having a usable implementation in core is likely helpful), but - and I realise this is just bike shedding - the naming seems quite odd to me. This extension and the classes it provides are inherently about HTTP requests made to a php ‘server’, and the response it sends back - and yet it’s called Server{Request,Response,Buffer} etc…. The “server” part is superfluous in the context of a php web application, because it’s all “server” side, and while uncommon it’s not impossible to write *other* types of network server using PHP. Cheers Stephen
  109000
March 13, 2020 13:39 pmjones@pmjones.io ("Paul M. Jones")
Hi Stephen,

> On Mar 13, 2020, at 02:41, Stephen Reay <php-lists@koalephant.com> wrote: > > I realise this is just bike shedding - the naming seems quite odd to me. > > This extension and the classes it provides are inherently about HTTP requests made to a php ‘server’, and the response it sends back - and yet it’s called Server{Request,Response,Buffer} etc…. The “server” part is superfluous in the context of a php web application, because it’s all “server” side, and while uncommon it’s not impossible to write *other* types of network server using PHP.
I share your feeling here, and I don't think it's bike shedding. The naming is still an open question on the RFC. I mentioned some other candidate names here ... https://externals.io/message/108436#108702 .... and re-reading your comment above, it looks like you saw that one. Do you have alternative suggestions or preferences on the names? Or, do you feel that "Request" and "Response" and "ResponseSender" (without any prefixes at all) would be sufficiently obvious? Let me know, and thanks for bringing it up! -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  109001
March 13, 2020 14:23 php-lists@koalephant.com (Stephen Reay)
> On 13 Mar 2020, at 20:39, Paul M. Jones <pmjones@pmjones.io> wrote: > > Hi Stephen, > >> On Mar 13, 2020, at 02:41, Stephen Reay <php-lists@koalephant.com> wrote: >> >> I realise this is just bike shedding - the naming seems quite odd to me. >> >> This extension and the classes it provides are inherently about HTTP requests made to a php ‘server’, and the response it sends back - and yet it’s called Server{Request,Response,Buffer} etc…. The “server” part is superfluous in the context of a php web application, because it’s all “server” side, and while uncommon it’s not impossible to write *other* types of network server using PHP. > > I share your feeling here, and I don't think it's bike shedding. The naming is still an open question on the RFC. > > I mentioned some other candidate names here ... > > https://externals.io/message/108436#108702 > > ... and re-reading your comment above, it looks like you saw that one. > > Do you have alternative suggestions or preferences on the names? Or, do you feel that "Request" and "Response" and "ResponseSender" (without any prefixes at all) would be sufficiently obvious? > > Let me know, and thanks for bringing it up! > > > -- > Paul M. Jones > pmjones@pmjones.io > http://paul-m-jones.com > > Modernizing Legacy Applications in PHP > https://leanpub.com/mlaphp > > Solving the N+1 Problem in PHP > https://leanpub.com/sn1php > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Hi Paul, TLDR: if you aren’t concerned about the concept of php-initiated outgoing HTTP requests, I think `HTTP{Request,Response,Buffer}` is quite clear in terms of naming. If you wanted to be more explicit about their purpose (and/or prevent possible confusion with either user land or potential future extensions handling outgoing requests), `IncomingHTTPRequest` and `OutgoingHTTPResponse` are very explicit, if a bit verbose. I think I did see that message, but I must admit I haven’t followed all the responses in the discussion. Personally I think `HTTP` makes a pretty obvious prefix (I’m not gonna get into a capitalisation debate), because these things are explicitly related to HTTP request and response messages. However, that also may be confusing if people expect it to be a construct for making outgoing requests. The user land implementation I’ve been using ’solves’ this by using a `HTTP` namespace, and then provides `Request` and `Response` (for an outgoing - i.e. curl - HTTP Request, and the corresponding HTTP Response) objects and then `CurrentRequest` and `CurrentResponse` for what your RFC proposes (i.e. the active request made to php). As with anything any of us has written, I’m not 100% sold on ‘Current{Request,Response}` even after writing it, but I think it’s at least a little more specific about what they do, when the namespace is taken into account. Cheers Stephen
  109002
March 13, 2020 14:55 pmjones@pmjones.io ("Paul M. Jones")
Hi Stephen,

Good stuff. You have stepped through my & John's earlier thought-process in much the same way that led to the current naming.


> On Mar 13, 2020, at 09:23, Stephen Reay <php-lists@koalephant.com> wrote: > >> On 13 Mar 2020, at 20:39, Paul M. Jones <pmjones@pmjones.io> wrote: >> >> Do you have alternative suggestions or preferences on the names? Or, do you feel that "Request" and "Response" and "ResponseSender" (without any prefixes at all) would be sufficiently obvious? > > Hi Paul, > > TLDR: if you aren’t concerned about the concept of php-initiated outgoing HTTP requests, I think `HTTP{Request,Response,Buffer}` is quite clear in terms of naming. If you wanted to be more explicit about their purpose (and/or prevent possible confusion with either user land or potential future extensions handling outgoing requests), `IncomingHTTPRequest` and `OutgoingHTTPResponse` are very explicit, if a bit verbose. > > I think I did see that message, but I must admit I haven’t followed all the responses in the discussion. > > Personally I think `HTTP` makes a pretty obvious prefix (I’m not gonna get into a capitalisation debate), because these things are explicitly related to HTTP request and response messages.
I follow your reasoning. The main problem I see with "HTTP" (I won't argue capitalization either) is that I can guarantee you there will be a very vocal subset of users who will complain loudly and continuously that the objects do not model HTTP messages. I would prefer to pre-empt that, especially since it is true. ;-) One other alternative John & I contemplated was `Web{Request,Response,ResponseSender}` -- do you think that might be a reasonable alternative to HTTP, one that is "adjacent" but not overly-specific? That would net us: - WebRequest - WebResponse - WebResponseSender I didn't like the look of it previously, and I don't think I like the look of it now, but ... (/me shrugs).
> However, that also may be confusing if people expect it to be a construct for making outgoing requests.
Yes, that's another tricky bit in the naming -- making a distinction between the objects as representative of client operations (send request, receive response) and server operations (receive request, send response). Thus the current `Server` prefix (however unsatisfactory it may be) to indicate their operational context. Your `Incoming` and `Outgoing` prefixes, minus the HTTP, would net us: - IncomingRequest - OutgoingResponse - OutgoingResponseSender I will need to ponder on those.
> The user land implementation I’ve been using ’solves’ this by using a `HTTP` namespace, and then provides `Request` and `Response` (for an outgoing - i.e. curl - HTTP Request, and the corresponding HTTP Response) objects and then `CurrentRequest` and `CurrentResponse` for what your RFC proposes (i.e. the active request made to php).
Yes, userland does operate that way. However, I think adding an HTTP namespace to PHP itself is something to be avoided, so emulating userland here is not an option.
> As with anything any of us has written, I’m not 100% sold on ‘Current{Request,Response}` even after writing it, but I think it’s at least a little more specific about what they do, when the namespace is taken into account.
`Current{...}` is not something we had previously considered; that would net us, in the global namespace: - CurrentRequest - CurrentResponse - CurrentResponseSender I will need to ponder on those as well. Any further thoughts or opinions on this, Stephen? Or from anyone else? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  109003
March 13, 2020 18:40 mike@newclarity.net (Mike Schinkel)
> On Mar 13, 2020, at 10:55 AM, Paul M. Jones <pmjones@pmjones.io> wrote: > > One other alternative John & I contemplated was `Web{Request,Response,ResponseSender}` -- do you think that might be a reasonable alternative to HTTP, one that is "adjacent" but not overly-specific? That would net us: > > - WebRequest > - WebResponse > - WebResponseSender > > I didn't like the look of it previously, and I don't think I like the look of it now, but ... (/me shrugs). > > >> However, that also may be confusing if people expect it to be a construct for making outgoing requests. > > Yes, that's another tricky bit in the naming -- making a distinction between the objects as representative of client operations (send request, receive response) and server operations (receive request, send response). Thus the current `Server` prefix (however unsatisfactory it may be) to indicate their operational context. > > Your `Incoming` and `Outgoing` prefixes, minus the HTTP, would net us: > > - IncomingRequest > - OutgoingResponse > - OutgoingResponseSender > > I will need to ponder on those. > >> The user land implementation I’ve been using ’solves’ this by using a `HTTP` namespace, and then provides `Request` and `Response` (for an outgoing - i.e. curl - HTTP Request, and the corresponding HTTP Response) objects and then `CurrentRequest` and `CurrentResponse` for what your RFC proposes (i.e. the active request made to php). > > Yes, userland does operate that way. However, I think adding an HTTP namespace to PHP itself is something to be avoided, so emulating userland here is not an option. > > >> As with anything any of us has written, I’m not 100% sold on ‘Current{Request,Response}` even after writing it, but I think it’s at least a little more specific about what they do, when the namespace is taken into account. > > `Current{...}` is not something we had previously considered; that would net us, in the global namespace: > > - CurrentRequest > - CurrentResponse > - CurrentResponseSender > > I will need to ponder on those as well. > > Any further thoughts or opinions on this, Stephen? Or from anyone else?
One issue that I have is, if we are going to fine-tune the naming to make sure it matches *exactly* then I think that CurrentRequest->server does not make sense. Maybe if you choose one of these names you should break out the server-specific items into their own class/object? -Mike
  109004
March 13, 2020 19:23 php-lists@koalephant.com (Stephen Reay)
> On 14 Mar 2020, at 01:40, Mike Schinkel <mike@newclarity.net> wrote: > > >> On Mar 13, 2020, at 10:55 AM, Paul M. Jones <pmjones@pmjones.io> wrote: >> >> One other alternative John & I contemplated was `Web{Request,Response,ResponseSender}` -- do you think that might be a reasonable alternative to HTTP, one that is "adjacent" but not overly-specific? That would net us: >> >> - WebRequest >> - WebResponse >> - WebResponseSender >> >> I didn't like the look of it previously, and I don't think I like the look of it now, but ... (/me shrugs). >> >> >>> However, that also may be confusing if people expect it to be a construct for making outgoing requests. >> >> Yes, that's another tricky bit in the naming -- making a distinction between the objects as representative of client operations (send request, receive response) and server operations (receive request, send response). Thus the current `Server` prefix (however unsatisfactory it may be) to indicate their operational context. >> >> Your `Incoming` and `Outgoing` prefixes, minus the HTTP, would net us: >> >> - IncomingRequest >> - OutgoingResponse >> - OutgoingResponseSender >> >> I will need to ponder on those. >> >>> The user land implementation I’ve been using ’solves’ this by using a `HTTP` namespace, and then provides `Request` and `Response` (for an outgoing - i.e. curl - HTTP Request, and the corresponding HTTP Response) objects and then `CurrentRequest` and `CurrentResponse` for what your RFC proposes (i.e. the active request made to php). >> >> Yes, userland does operate that way. However, I think adding an HTTP namespace to PHP itself is something to be avoided, so emulating userland here is not an option. >> >> >>> As with anything any of us has written, I’m not 100% sold on ‘Current{Request,Response}` even after writing it, but I think it’s at least a little more specific about what they do, when the namespace is taken into account. >> >> `Current{...}` is not something we had previously considered; that would net us, in the global namespace: >> >> - CurrentRequest >> - CurrentResponse >> - CurrentResponseSender >> >> I will need to ponder on those as well. >> >> Any further thoughts or opinions on this, Stephen? Or from anyone else? > > One issue that I have is, if we are going to fine-tune the naming to make sure it matches *exactly* then I think that CurrentRequest->server does not make sense. > > Maybe if you choose one of these names you should break out the server-specific items into their own class/object? > > -Mike > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Hi Mike, (I realise some of these points are possibly more addressed to Paul than yourself, this is just where my brain went when I dug into what you were mentioning) I apologise if I’ve missed part of the discussion but what do you mean by “make sure it matches *exactly*. Do you mean how `->server` is listed as being a copy of the `$_SERVER` super global? If so, can someone point me to the specific logic behind that, given how many parts of it are already exposed via ‘dedicated’ properties of the proposed class? From what I can see (and I may have missed some) the parts of $_SERVER not exposed in some way “directly” on ServerRequest (nee CurrentRequest) are the actual “server” parts: the `SERVER_*` keys, doc root, PHP_SELF; and the ‘client’ parts: REMOTE_*; plus few random stragglers like PATH_INFO, and for some reason REQUEST_TIME[_FLOAT]? Can those two things not be organised as a hash of those values, under `server` and `client` (or `remote` if you want to keep the terminology - yes I know it will be the originating TCP connection host not necessarily the browser host)? As I said, I’ve missed some of the discussion but I fail to see the benefit of a class to make all the details of a web request and it’s response available…. And then just stick the existing superglobals in it untouched. Things like the query params and body params make sense to be accessible essentially (albeit with better names) as-is, because they’re a hash of unknown shape by their very nature - but the keys in _SERVER (barring http headers, which are already special cased) are known, and have pretty clear definition of their meaning. Is the proposal really suggesting that a developer would still need to do `if(!empty($request->server[‘HTTPS’]) && $request->server[‘HTTPS’] !== ‘off’) {…}` rather than just providing a `secureTransport` property (or `https` if you prefer)? One last point, regarding the ‘break out a server specific class’ part. I don’t think it’s “wrong” to access these properties from something that is ostensibly related to the “current request”, but it feels quite ‘wonky’ to me, the way it’s proposed with the full ->server array just copied as-is, AND exposed via dedicated properties. Cheers Stephen
  109005
March 13, 2020 19:59 mike@newclarity.net (Mike Schinkel)
> On Mar 13, 2020, at 3:23 PM, Stephen Reay <php-lists@koalephant.com> wrote: > > Hi Mike, > > (I realise some of these points are possibly more addressed to Paul than yourself, this is just where my brain went when I dug into what you were mentioning)
(after responding to your reply I think I was commenting on the RFC and I think it is possible that you may have missed as few aspects of the RFC such as a 'server' property on the ServerRequest variable. If that is the case then we might be saying the same things. See https://github.com/pmjones/ext-request#superglobal-related) Yes, I should have made it more explicit. As author of the RFC I was speaking to Paul in suggesting that if we are going to name the proposed objects to be more specific to requests and response that we move the server-specific aspects out and into their own object. Otherwise I feel that dropping the "Server" from the name the object clarifies one aspect and obfuscates another, albeit clarifying the larger part. Better to make it all clear with another object named for what it represents; server information.
> I apologise if I’ve missed part of the discussion but what do you mean by “make sure it matches *exactly*.
I was referencing your comments where my takeaway from reading what you wrote is that you were asking that the naming match exactly what you were viewing the objects to be doing, and that is to work with HTTP: "This extension and the classes it provides are inherently about HTTP requests made to a php ‘server’, and the response it sends back - and yet it’s called Server{Request,Response,Buffer} etc…. The “server” part is superfluous in the context of a php web application, because it’s all “server” side, and while uncommon it’s not impossible to write *other* types of network server using PHP." "TLDR: if you aren’t concerned about the concept of php-initiated outgoing HTTP requests, I think `HTTP{Request,Response,Buffer}` is quite clear in terms of naming. If you wanted to be more explicit about their purpose (and/or prevent possible confusion with either user land or potential future extensions handling outgoing requests), `IncomingHTTPRequest` and `OutgoingHTTPResponse` are very explicit, if a bit verbose." My point is simply that the server-specific aspects have nothing to do with HTTP requests or HTTP responses, and if Paul was to rename his classes to more closely align with HTTP requests and HTTP responses then he should extract the server aspects out into their own class.
> Do you mean how `->server` is listed as being a copy of the `$_SERVER` super global? If so, can someone point me to the specific logic behind that, given how many parts of it are already exposed via ‘dedicated’ properties of the proposed class? From what I can see (and I may have missed some) the parts of $_SERVER not exposed in some way “directly” on ServerRequest (nee CurrentRequest) are the actual “server” parts: the `SERVER_*` keys, doc root, PHP_SELF; and the ‘client’ parts: REMOTE_*; plus few random stragglers like PATH_INFO, and for some reason REQUEST_TIME[_FLOAT]?
I don't think direct exposure is required; indirect exposure via an array still means that aspects not related to HTTP requests and HTTP responses are currently contained in the ServerRequest->server which to me is okay if it is called "ServerRequest" but not okay if it is called "HttpRequest," "WebRequest," "IncomingRequest," or "CurrentRequest."
> Can those two things not be organised as a hash of those values, under `server` and `client` (or `remote` if you want to keep the terminology - yes I know it will be the originating TCP connection host not necessarily the browser host)? As I said, I’ve missed some of the discussion but I fail to see the benefit of a class to make all the details of a web request and it’s response available…. And then just stick the existing superglobals in it untouched.
Your response is confusing me. You may actually be saying the same thing I am saying. I am referring to the current state of the RFC where this would be possible: $request = new ServerRequest(); echo $request->server['HOME']; // Output the server's home directory. I am saying that it might be better to have those properties in a different object: $info = new ServerInfo(); echo $info->home; // Output the server's home directory. print_r( $info->properties ); // Output everything found in $_SERVER object
> Things like the query params and body params make sense to be accessible essentially (albeit with better names) as-is, because they’re a hash of unknown shape by their very nature - but the keys in _SERVER (barring http headers, which are already special cased) are known, and have pretty clear definition of their meaning.
Agreed.
> Is the proposal really suggesting that a developer would still need to do `if(!empty($request->server[‘HTTPS’]) && $request->server[‘HTTPS’] !== ‘off’) {…}` rather than just providing a `secureTransport` property (or `https` if you prefer)?
Not sure. Paul needs to answer that.
> One last point, regarding the ‘break out a server specific class’ part. I don’t think it’s “wrong” to access these properties from something that is ostensibly related to the “current request”, but it feels quite ‘wonky’ to me, the way it’s proposed with the full ->server array just copied as-is, AND exposed via dedicated properties > Cheers
Yes, I was saying that it is wrong *if* we are going to fine-tune the name to a as closely as possible mirror what is contained within it. As I said above having `server' in ServerRequest class does not both me but it would bother me if `server' were in a HttpRequest class. Does this clarify? -Mike
  109006
March 13, 2020 20:25 php-lists@koalephant.com (Stephen Reay)
> On 14 Mar 2020, at 02:59, Mike Schinkel <mike@newclarity.net> wrote: > >> On Mar 13, 2020, at 3:23 PM, Stephen Reay <php-lists@koalephant.com> wrote: >> >> Hi Mike, >> >> (I realise some of these points are possibly more addressed to Paul than yourself, this is just where my brain went when I dug into what you were mentioning) > > (after responding to your reply I think I was commenting on the RFC and I think it is possible that you may have missed as few aspects of the RFC such as a 'server' property on the ServerRequest variable. If that is the case then we might be saying the same things. See https://github.com/pmjones/ext-request#superglobal-related) > > Yes, I should have made it more explicit. As author of the RFC I was speaking to Paul in suggesting that if we are going to name the proposed objects to be more specific to requests and response that we move the server-specific aspects out and into their own object. > > Otherwise I feel that dropping the "Server" from the name the object clarifies one aspect and obfuscates another, albeit clarifying the larger part. Better to make it all clear with another object named for what it represents; server information. > >> I apologise if I’ve missed part of the discussion but what do you mean by “make sure it matches *exactly*. > > I was referencing your comments where my takeaway from reading what you wrote is that you were asking that the naming match exactly what you were viewing the objects to be doing, and that is to work with HTTP: > > "This extension and the classes it provides are inherently about HTTP requests made to a php ‘server’, and the response it sends back - and yet it’s called Server{Request,Response,Buffer} etc…. The “server” part is superfluous in the context of a php web application, because it’s all “server” side, and while uncommon it’s not impossible to write *other* types of network server using PHP." > > "TLDR: if you aren’t concerned about the concept of php-initiated outgoing HTTP requests, I think `HTTP{Request,Response,Buffer}` is quite clear in terms of naming. If you wanted to be more explicit about their purpose (and/or prevent possible confusion with either user land or potential future extensions handling outgoing requests), `IncomingHTTPRequest` and `OutgoingHTTPResponse` are very explicit, if a bit verbose." > > My point is simply that the server-specific aspects have nothing to do with HTTP requests or HTTP responses, and if Paul was to rename his classes to more closely align with HTTP requests and HTTP responses then he should extract the server aspects out into their own class.
I think we’re probably talking about different ’server specific’ parts of the $_SERVER super global array.. I’m talking about the stuff that’s documented (on https://www.php.net/manual/en/reserved.variables.server.php <https://www.php.net/manual/en/reserved.variables.server.php>) and much of which comes from/seems inspired by the CGI spec, which is inherently related to a http request. I.e. the SERVER_* keys, the REMOTE_* keys, the doc root, etc.
> > >> Do you mean how `->server` is listed as being a copy of the `$_SERVER` super global? If so, can someone point me to the specific logic behind that, given how many parts of it are already exposed via ‘dedicated’ properties of the proposed class? From what I can see (and I may have missed some) the parts of $_SERVER not exposed in some way “directly” on ServerRequest (nee CurrentRequest) are the actual “server” parts: the `SERVER_*` keys, doc root, PHP_SELF; and the ‘client’ parts: REMOTE_*; plus few random stragglers like PATH_INFO, and for some reason REQUEST_TIME[_FLOAT]? > > I don't think direct exposure is required; indirect exposure via an array still means that aspects not related to HTTP requests and HTTP responses are currently contained in the ServerRequest->server which to me is okay if it is called "ServerRequest" but not okay if it is called "HttpRequest," "WebRequest," "IncomingRequest," or "CurrentRequest." > >> Can those two things not be organised as a hash of those values, under `server` and `client` (or `remote` if you want to keep the terminology - yes I know it will be the originating TCP connection host not necessarily the browser host)? As I said, I’ve missed some of the discussion but I fail to see the benefit of a class to make all the details of a web request and it’s response available…. And then just stick the existing superglobals in it untouched. > > Your response is confusing me. > > You may actually be saying the same thing I am saying. I am referring to the current state of the RFC where this would be possible: > > $request = new ServerRequest(); > echo $request->server['HOME']; // Output the server's home directory. >
This is a very weird thing to me. I realise it’s exposed currently via $_SERVER but what you’re accessing there is an environment variable - it’s also available via $_ENV, or getenv(). This is another reason why I find it weird to put the current ‘goody bag’ of $_SERVER into an object intended to in some way represent a web request and response. That information (environment vars) is very useful, undoubtedly - but accessing it from an object essentially maps to the current, incoming http/web request (regardless of what it’s called), is bizarre IMO.
> I am saying that it might be better to have those properties in a different object: > > $info = new ServerInfo(); > echo $info->home; // Output the server's home directory. > print_r( $info->properties ); // Output everything found in $_SERVER object >
Without clarifying which things specifically (and that’ll be hard as your other reference is to an env var, few of which are standardised) I’m not sure what I think about this. But I agree (maybe more strongly, I think naming is irrelevant) that things like HOME (i.e. environment variables) have little place in the object Paul is proposing, if it’s meant to be related to “the active request”.
>> Things like the query params and body params make sense to be accessible essentially (albeit with better names) as-is, because they’re a hash of unknown shape by their very nature - but the keys in _SERVER (barring http headers, which are already special cased) are known, and have pretty clear definition of their meaning. > > Agreed. > >> Is the proposal really suggesting that a developer would still need to do `if(!empty($request->server[‘HTTPS’]) && $request->server[‘HTTPS’] !== ‘off’) {…}` rather than just providing a `secureTransport` property (or `https` if you prefer)? > > Not sure. Paul needs to answer that. > >> One last point, regarding the ‘break out a server specific class’ part. I don’t think it’s “wrong” to access these properties from something that is ostensibly related to the “current request”, but it feels quite ‘wonky’ to me, the way it’s proposed with the full ->server array just copied as-is, AND exposed via dedicated properties >> Cheers > > Yes, I was saying that it is wrong *if* we are going to fine-tune the name to a as closely as possible mirror what is contained within it. As I said above having `server' in ServerRequest class does not both me but it would bother me if `server' were in a HttpRequest class. > > Does this clarify?
Yes, mostly, with the caveat of the part about which entries from $_SERVER (and I think you’re referring to env vars, not the cgi specific vars?)
> > -Mike
Cheers Stephen
  109008
March 13, 2020 21:38 mike@newclarity.net (Mike Schinkel)
> On Mar 13, 2020, at 4:25 PM, Stephen Reay <php-lists@koalephant.com> wrote: > >> >> On 14 Mar 2020, at 02:59, Mike Schinkel <mike@newclarity.net> wrote: >> >>> On Mar 13, 2020, at 3:23 PM, Stephen Reay <php-lists@koalephant.com> wrote: >>> >>> Hi Mike, >>> >>> (I realise some of these points are possibly more addressed to Paul than yourself, this is just where my brain went when I dug into what you were mentioning) >> >> (after responding to your reply I think I was commenting on the RFC and I think it is possible that you may have missed as few aspects of the RFC such as a 'server' property on the ServerRequest variable. If that is the case then we might be saying the same things. See https://github.com/pmjones/ext-request#superglobal-related) >> >> Yes, I should have made it more explicit. As author of the RFC I was speaking to Paul in suggesting that if we are going to name the proposed objects to be more specific to requests and response that we move the server-specific aspects out and into their own object. >> >> Otherwise I feel that dropping the "Server" from the name the object clarifies one aspect and obfuscates another, albeit clarifying the larger part. Better to make it all clear with another object named for what it represents; server information. >> >>> I apologise if I’ve missed part of the discussion but what do you mean by “make sure it matches *exactly*. >> >> I was referencing your comments where my takeaway from reading what you wrote is that you were asking that the naming match exactly what you were viewing the objects to be doing, and that is to work with HTTP: >> >> "This extension and the classes it provides are inherently about HTTP requests made to a php ‘server’, and the response it sends back - and yet it’s called Server{Request,Response,Buffer} etc…. The “server” part is superfluous in the context of a php web application, because it’s all “server” side, and while uncommon it’s not impossible to write *other* types of network server using PHP." >> >> "TLDR: if you aren’t concerned about the concept of php-initiated outgoing HTTP requests, I think `HTTP{Request,Response,Buffer}` is quite clear in terms of naming. If you wanted to be more explicit about their purpose (and/or prevent possible confusion with either user land or potential future extensions handling outgoing requests), `IncomingHTTPRequest` and `OutgoingHTTPResponse` are very explicit, if a bit verbose." >> >> My point is simply that the server-specific aspects have nothing to do with HTTP requests or HTTP responses, and if Paul was to rename his classes to more closely align with HTTP requests and HTTP responses then he should extract the server aspects out into their own class. > > I think we’re probably talking about different ’server specific’ parts of the $_SERVER super global array.. I’m talking about the stuff that’s documented (on https://www.php.net/manual/en/reserved.variables.server.php) and much of which comes from/seems inspired by the CGI spec, which is inherently related to a http request. I.e. the SERVER_* keys, the REMOTE_* keys, the doc root, etc. > >> >> >>> Do you mean how `->server` is listed as being a copy of the `$_SERVER` super global? If so, can someone point me to the specific logic behind that, given how many parts of it are already exposed via ‘dedicated’ properties of the proposed class? From what I can see (and I may have missed some) the parts of $_SERVER not exposed in some way “directly” on ServerRequest (nee CurrentRequest) are the actual “server” parts: the `SERVER_*` keys, doc root, PHP_SELF; and the ‘client’ parts: REMOTE_*; plus few random stragglers like PATH_INFO, and for some reason REQUEST_TIME[_FLOAT]? >> >> I don't think direct exposure is required; indirect exposure via an array still means that aspects not related to HTTP requests and HTTP responses are currently contained in the ServerRequest->server which to me is okay if it is called "ServerRequest" but not okay if it is called "HttpRequest," "WebRequest," "IncomingRequest," or "CurrentRequest." >> >>> Can those two things not be organised as a hash of those values, under `server` and `client` (or `remote` if you want to keep the terminology - yes I know it will be the originating TCP connection host not necessarily the browser host)? As I said, I’ve missed some of the discussion but I fail to see the benefit of a class to make all the details of a web request and it’s response available…. And then just stick the existing superglobals in it untouched. >> >> Your response is confusing me. >> >> You may actually be saying the same thing I am saying. I am referring to the current state of the RFC where this would be possible: >> >> $request = new ServerRequest(); >> echo $request->server['HOME']; // Output the server's home directory. >> > > This is a very weird thing to me. I realise it’s exposed currently via $_SERVER but what you’re accessing there is an environment variable - it’s also available via $_ENV, or getenv(). This is another reason why I find it weird to put the current ‘goody bag’ of $_SERVER into an object intended to in some way represent a web request and response. > > That information (environment vars) is very useful, undoubtedly - but accessing it from an object essentially maps to the current, incoming http/web request (regardless of what it’s called), is bizarre IMO. > >> I am saying that it might be better to have those properties in a different object: >> >> $info = new ServerInfo(); >> echo $info->home; // Output the server's home directory. >> print_r( $info->properties ); // Output everything found in $_SERVER object >> > > Without clarifying which things specifically (and that’ll be hard as your other reference is to an env var, few of which are standardised) I’m not sure what I think about this. But I agree (maybe more strongly, I think naming is irrelevant) that things like HOME (i.e. environment variables) have little place in the object Paul is proposing, if it’s meant to be related to “the active request”. > >>> Things like the query params and body params make sense to be accessible essentially (albeit with better names) as-is, because they’re a hash of unknown shape by their very nature - but the keys in _SERVER (barring http headers, which are already special cased) are known, and have pretty clear definition of their meaning. >> >> Agreed. >> >>> Is the proposal really suggesting that a developer would still need to do `if(!empty($request->server[‘HTTPS’]) && $request->server[‘HTTPS’] !== ‘off’) {…}` rather than just providing a `secureTransport` property (or `https` if you prefer)? >> >> Not sure. Paul needs to answer that. >> >>> One last point, regarding the ‘break out a server specific class’ part. I don’t think it’s “wrong” to access these properties from something that is ostensibly related to the “current request”, but it feels quite ‘wonky’ to me, the way it’s proposed with the full ->server array just copied as-is, AND exposed via dedicated properties >>> Cheers >> >> Yes, I was saying that it is wrong *if* we are going to fine-tune the name to a as closely as possible mirror what is contained within it. As I said above having `server' in ServerRequest class does not both me but it would bother me if `server' were in a HttpRequest class. >> >> Does this clarify? > > Yes, mostly, with the caveat of the part about which entries from $_SERVER (and I think you’re referring to env vars, not the cgi specific vars?)
Larry just clarified it in his reply to this thread. Basically calling it "CgiRequest/CGIRequest (ignoring capitalization)" would probably be the right fit from a naming perspective, at least with respect to Paul's RFC. -Mike
  109061
March 16, 2020 15:00 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

That escalated quickly. :-)  I'm going to reply to the only request for clarification I could identify in this exchange -- if I missed others, please let me know.

>> Is the proposal really suggesting that a developer would still need to do `if(!empty($request->server[‘HTTPS’]) && $request->server[‘HTTPS’] !== ‘off’) {…}` rather than just providing a `secureTransport` property (or `https` if you prefer)? > > Not sure. Paul needs to answer that.
This is another one of those places where several of the individual projects have something similar, but not close-enough to each other to represent it in the ServerRequest object. Some honor '1' as 'on', some do not, etc. So, yes, I'm saying that existing code using $_SERVER could use this as a replacement: $secure = ($request->server['HTTPS'] ?? null) === 'on'; I'm open to discussion about additions to the object for this or other purposes, though, if we can decide on "the right way" to calculate new proposed values. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  109062
March 16, 2020 15:11 rowan.collins@gmail.com (Rowan Tommins)
On Mon, 16 Mar 2020 at 15:01, Paul M. Jones <pmjones@pmjones.io> wrote:

> That escalated quickly. :-) I'm going to reply to the only request for > clarification I could identify in this exchange -- if I missed others, > please let me know. >
Hi Paul, Did you see the naming suggestion from Larry of CgiRequest/CGIRequest, and my follow-up that the "server" array could be named "cgi"? Any thoughts? Regards, -- Rowan Tommins [IMSoP]
  109007
March 13, 2020 20:36 larry@garfieldtech.com ("Larry Garfield")
On Fri, Mar 13, 2020, at 9:55 AM, Paul M. Jones wrote:

> > As with anything any of us has written, I’m not 100% sold on ‘Current{Request,Response}` even after writing it, but I think it’s at least a little more specific about what they do, when the namespace is taken into account. > > `Current{...}` is not something we had previously considered; that > would net us, in the global namespace: > > - CurrentRequest > - CurrentResponse > - CurrentResponseSender > > I will need to ponder on those as well. > > Any further thoughts or opinions on this, Stephen? Or from anyone else?
I am still negative on the RFC overall, but I'll throw this out there: * The RFC by design is trying to take the super-globals and make them OOPy; no more, no less. * The super-globals are not based on HTTP. They're based on CGI, which is sort of but not quite an HTTP request. * So... call it CgiRequest/CGIRequest? (I am also ignoring capitalization.) Because that's what it is: It's CGI request data wrapped up into an object, no more, no less. (The fact that it is no more is the reason I'm not a fan, but at least then it's accurate as to what it is, and isn't, and doesn't namespace clobber those libraries that are modeling HTTP, rather than CGI.) The response object is less CGI-bound, so it could still be ServerResponse since it's a response from a server, and not intended for any other uses (like HTTP client libraries). --Larry Garfield
  109013
March 14, 2020 15:18 rowan.collins@gmail.com (Rowan Tommins)
On 13/03/2020 20:36, Larry Garfield wrote:
> * The RFC by design is trying to take the super-globals and make them OOPy; no more, no less. > * The super-globals are not based on HTTP. They're based on CGI, which is sort of but not quite an HTTP request. > * So... call it CgiRequest/CGIRequest? (I am also ignoring capitalization.) Because that's what it is: It's CGI request data wrapped up into an object, no more, no less.
This makes a lot of sense to me. I also just realised that it would make sense to rename $_SERVER in the object, the same way $_GET has become "query" and $_POST has become "input". Maybe $request->cgi or something? CGIRequest->cgi might feel awkward, but it's no worse than the current ServerRequest->server, and I think naming it that way gives a better hint as to why the array has such an odd collection of things in it. Regards, -- Rowan Tommins (né Collins) [IMSoP]
  109065
March 16, 2020 16:04 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

> On Mar 14, 2020, at 10:18, Rowan Tommins collins@gmail.com> wrote: > >> On 13/03/2020 20:36, Larry Garfield wrote: >> >>> * The RFC by design is trying to take the super-globals and make them OOPy; no more, no less. >>> * The super-globals are not based on HTTP. They're based on CGI, which is sort of but not quite an HTTP request. >>> * So... call it CgiRequest/CGIRequest? (I am also ignoring capitalization.) Because that's what it is: It's CGI request data wrapped up into an object, no more, no less. > > This makes a lot of sense to me.
I too find this appealing. That prefix would net these class names: - CgiRequest - CgiResponse - CgiResponseSender The only confusion I think might arise is a consumer saying, "I'm not using CGI, I'm using (fcgi|apache|etc)". Turning it over in my mind, I wonder if maybe a `Sapi` prefix would be a better alternative along this path, especially since these objects relate to the "interface between web server and PHP." <https://www.php.net/manual/en/function.php-sapi-name.php> A `Sapi` prefix would net us: - SapiRequest - SapiResponse - SapiResponseSender The only obvious objection is that the SAPI list includes 'cli'. While it not strictly outside the realm of the RFC, it certainly is not the focus; but then, tests running at the command line will be under the 'cli' SAPI, so there's at least some sort of rationalization for it. Overall, I think `Sapi` looks better than `Cgi`, and certainly better than `Server` or the other names we've considered. Any further thoughts on naming?
> I also just realised that it would make sense to rename $_SERVER in the object, the same way $_GET has become "query" and $_POST has become "input". Maybe $request->cgi or something?
I sympathize with the desire to rename (or even restructure) the apparent catchall of `$_SERVER`. But I am still reluctant to do so, especially as no other researched projects use a different name here (whereas some have replaced `$_GET` with `$query`, etc). Does anyone besides Rowan long for a different property name here? What are your suggestions, if any? -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  109077
March 16, 2020 18:35 mike@newclarity.net (Mike Schinkel)
> On Mar 16, 2020, at 12:04 PM, Paul M. Jones <pmjones@pmjones.io> wrote: > > Turning it over in my mind, I wonder if maybe a `Sapi` prefix would be a better alternative along this path, especially since these objects relate to the "interface between web server and PHP." <https://www.php.net/manual/en/function.php-sapi-name.php> A `Sapi` prefix would net us: > > - SapiRequest > - SapiResponse > - SapiResponseSender > > The only obvious objection is that the SAPI list includes 'cli'. While it not strictly outside the realm of the RFC, it certainly is not the focus; but then, tests running at the command line will be under the 'cli' SAPI, so there's at least some sort of rationalization for it. > > Overall, I think `Sapi` looks better than `Cgi`, and certainly better than `Server` or the other names we've considered. > > Any further thoughts on naming?
That works well IMO.
>> I also just realised that it would make sense to rename $_SERVER in the object, the same way $_GET has become "query" and $_POST has become "input". Maybe $request->cgi or something? > > I sympathize with the desire to rename (or even restructure) the apparent catchall of `$_SERVER`. But I am still reluctant to do so, especially as no other researched projects use a different name here (whereas some have replaced `$_GET` with `$query`, etc). > > Does anyone besides Rowan long for a different property name here? What are your suggestions, if any?
I don't think this is important anymore if we call it Sapi or CGI. It was only an issue IMO if if we were still wanting to call it Request. But again, just my opinion. -Mike
  109114
March 17, 2020 21:10 pmjones@pmjones.io ("Paul M. Jones")
Hi Mike & Rowan & all,

> On Mar 16, 2020, at 13:35, Mike Schinkel <mike@newclarity.net> wrote: > >> Does anyone besides Rowan long for a different property name here? What are your suggestions, if any? > > I don't think this is important anymore if we call it Sapi or CGI. It was only an issue IMO if if we were still wanting to call it Request. > > But again, just my opinion.
Noted, and agreed. After checking with John Boehr, and having put together a 2.x branch using the `Sapi` prefix, I think it looks pretty good. Unless there are objections, we'll keep the new prefix, and I'll update the RFC accordingly. -- Paul M. Jones pmjones@pmjones.io http://paul-m-jones.com Modernizing Legacy Applications in PHP https://leanpub.com/mlaphp Solving the N+1 Problem in PHP https://leanpub.com/sn1php
  109128
March 18, 2020 16:05 pmjones@pmjones.io ("Paul M. Jones")
Hi all,

With the name change from `ServerRequest` to `SapiRequest` (et al.), discussion appears to have waned again.

I believe all relevant questions have been answered and recorded at this point. Please review the updated RFC and implementation, and present any more questions/comments/concerns you may have.

Unless there are objections (or new substantive discussion) I plan to open voting tomorrow or Friday. 

Thanks to everyone who has participated so far!


-- 
Paul M. Jones
pmjones@pmjones.io
http://paul-m-jones.com

Modernizing Legacy Applications in PHP
https://leanpub.com/mlaphp

Solving the N+1 Problem in PHP
https://leanpub.com/sn1php