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

5 years ago by michal.brzuchalski@gmail.com — view source — reply

unread

Hi Paul,

thanks for bringing it back. Got some questions:

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?

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

5 years ago by Paul M. Jones — view source — reply

unread

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.)

Hi Paul,

thanks for bringing it back. Got some questions:

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.

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

5 years ago by Mark Randall — view source — reply

unread

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Rowan Tommins — view source — reply

unread

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:

Fetching the raw information about the incoming HTTP request from the
web server (the "SAPI")
Parsing that raw information into individual fields
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]

5 years ago by Mike Schinkel — view source — reply

unread

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');

Fetching the raw information about the incoming HTTP request from the web server (the "SAPI")

Parsing that raw information into individual fields

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by johannes@schlueters.de — view source — reply

unread

Hi,

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by johannes@schlueters.de — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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 ...

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

5 years ago by Jan Schneider — view source — reply

unread

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/

5 years ago by Albert Casademont — view source — reply

unread

This is very interesting, thanks!

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

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/

5 years ago by Reindl Harald privat — view source — reply

unread

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

5 years ago by Albert Casademont — view source — reply

unread

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?

5 years ago by Paul M. Jones — view source — reply

unread

Hi Albert,

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

5 years ago by Ben Ramsey — view source — reply

unread

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

5 years ago by Paul M.Jones — view source — reply

unread

Hi Ben,

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

5 years ago by Paul M.Jones — view source — reply

unread

Hi Mark,

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

5 years ago by Niklas Keller — view source — reply

unread

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by AllenJB — view source — reply

unread

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

5 years ago by come.chilliet@fusiondirectory.org — view source — reply

unread

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

5 years ago by Ekin B. — view source — reply

unread

Hi all,

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Côme & Niklas,

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

5 years ago by Benjamin Morel — view source — reply

unread

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

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

5 years ago by Bruce Weirdan — view source — reply

unread

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

5 years ago by Benjamin Morel — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi all,

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

5 years ago by Larry Garfield — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi all,

... 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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M. Jones — view source — reply

unread

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.

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi Rowan,

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

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:

It "bundles" several related pieces of data; not just $_POST but $_GET, $_COOKIES, etc., so they can be carried around together.
It separates that data from global state.
It parses some of that data into commonly-needed structures (eg. the $accept* arrays).
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

5 years ago by Rowan Tommins — view source — reply

unread

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

It "bundles" several related pieces of data; not just $_POST but $_GET, $_COOKIES, etc., so they can be carried around together.

It separates that data from global state.

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.

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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi Rowan,

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

5 years ago by Rowan Tommins — view source — reply

unread

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.

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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi Rowan,

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi Rowan,

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M.Jones — view source — reply

unread

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi Rowan,

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

5 years ago by Rowan Tommins — view source — reply

unread

Hi Rowan,

On Feb 27, 2020, at 10:57, Rowan Tommins rowan.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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi Rowan,

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Niklas,

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

5 years ago by come.chilliet@fusiondirectory.org — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Côme,

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

5 years ago by come.chilliet@fusiondirectory.org — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi all,

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

5 years ago by AllenJB — view source — reply

unread

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

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.

5 years ago by Paul M. Jones — view source — reply

unread

Hi Allen & Robin,

Allen, you asked:

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:

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

5 years ago by Ruud Boon — view source — reply

unread

Hi!

I really like this RFC.

Hi Allen & Robin,

Allen, you asked:

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:

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.
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.

--
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

--

Cheers,
Ruud

5 years ago by Paul M. Jones — view source — reply

unread

Hi Ruud,

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Dan Ackroyd — view source — reply

unread

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

5 years ago by Larry Garfield — view source — reply

unread

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:

Raw super globals, possibly with proprietary bits on top.
HttpFoundation
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

5 years ago by Paul M. Jones — view source — reply

unread

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.

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi Rowan,

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:

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.
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.
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.
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

5 years ago by Peter Bowyer — view source — reply

unread

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:

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).
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).
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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Peter,

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

I expect to vote "no". My thoughts are:

The chosen API. We have an OO approach, but headers and query parameters
are accessed through an array-style mechanism.

...

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?

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:

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).
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'.)
Run your regression testing (or smoke-test or spot-check) and correct any errors.
Commit, push, and notify QA.
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

5 years ago by Arvids Godjuks — view source — reply

unread

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

--

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Arvids,

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

5 years ago by Arvids Godjuks — view source — reply

unread

Hi Arvids,

On Mar 5, 2020, at 05:19, Arvids Godjuks arvids.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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Arvids,

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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

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.

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

5 years ago by Stephen Reay — view source — reply

unread

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

--

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Stephen,

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

5 years ago by Stephen Reay — view source — reply

unread

Hi Stephen,

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

--

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

5 years ago by Paul M. Jones — view source — reply

unread

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.

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Stephen Reay — view source — reply

unread

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

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

5 years ago by Mike Schinkel — view source — reply

unread

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 ifserver' were in a HttpRequest class.

Does this clarify?

-Mike

5 years ago by Stephen Reay — view source — reply

unread

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 ifserver' 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

5 years ago by Mike Schinkel — view source — reply

unread

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 ifserver' 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

5 years ago by Paul M. Jones — view source — reply

unread

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Larry Garfield — view source — reply

unread

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

5 years ago by Rowan Tommins — view source — reply

unread

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]

5 years ago by Paul M. Jones — view source — reply

unread

Hi all,

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

5 years ago by Mike Schinkel — view source — reply

unread

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

5 years ago by Paul M. Jones — view source — reply

unread

Hi Mike & Rowan & all,

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

5 years ago by Paul M. Jones — view source — reply

unread

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