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

> On 14 Mar 2020, at 02:59, Mike Schinkel <mike@newclarity.net> wrote: > >> On Mar 13, 2020, at 3:23 PM, Stephen Reay <php-lists@koalephant.com> wrote: >> >> Hi Mike, >> >> (I realise some of these points are possibly more addressed to Paul than yourself, this is >> just where my brain went when I dug into what you were mentioning) > > (after responding to your reply I think I was commenting on the RFC and I think it is possible > that you may have missed as few aspects of the RFC such as a 'server' property on the > ServerRequest variable. If that is the case then we might be saying the same things. See > https://github.com/pmjones/ext-request#superglobal-related) > > Yes, I should have made it more explicit. As author of the RFC I was speaking to Paul in > suggesting that if we are going to name the proposed objects to be more specific to requests and > response that we move the server-specific aspects out and into their own object. > > Otherwise I feel that dropping the "Server" from the name the object clarifies one > aspect and obfuscates another, albeit clarifying the larger part. Better to make it all clear with > another object named for what it represents; server information. > >> I apologise if I’ve missed part of the discussion but what do you mean by “make sure it >> matches *exactly*. > > I was referencing your comments where my takeaway from reading what you wrote is that you were > asking that the naming match exactly what you were viewing the objects to be doing, and that is to > work with HTTP: > > "This extension and the classes it provides are inherently about HTTP requests made to > a php ‘server’, and the response it sends back - and yet it’s called > Server{Request,Response,Buffer} etc…. The “server” part is superfluous in the context of a php > web application, because it’s all “server” side, and while uncommon it’s not impossible to > write *other* types of network server using PHP." > > "TLDR: if you aren’t concerned about the concept of php-initiated outgoing HTTP > requests, I think HTTP{Request,Response,Buffer} is quite clear in > terms of naming. If you wanted to be more explicit about their purpose (and/or prevent possible > confusion with either user land or potential future extensions handling outgoing requests), > IncomingHTTPRequest and > OutgoingHTTPResponse are very explicit, if a bit verbose." > > My point is simply that the server-specific aspects have nothing to do with HTTP requests or > HTTP responses, and if Paul was to rename his classes to more closely align with HTTP requests and > HTTP responses then he should extract the server aspects out into their own class. I think we’re probably talking about different ’server specific’ parts of the $_SERVER super global array.. I’m talking about the stuff that’s documented (on https://www.php.net/manual/en/reserved.variables.server.php <https://www.php.net/manual/en/reserved.variables.server.php>) and much of which comes from/seems inspired by the CGI spec, which is inherently related to a http request. I.e. the SERVER_* keys, the REMOTE_* keys, the doc root, etc. > > >> Do you mean how ->server is listed as being a copy of >> the $_SERVER super global? If so, can someone point me to the >> specific logic behind that, given how many parts of it are already exposed via ‘dedicated’ >> properties of the proposed class? From what I can see (and I may have missed some) the parts of >> $_SERVER not exposed in some way “directly” on ServerRequest (nee CurrentRequest) are the actual >> “server” parts: the SERVER_* keys, doc root, PHP_SELF; and the >> ‘client’ parts: REMOTE_*; plus few random stragglers like PATH_INFO, and for some reason >> REQUEST_TIME[_FLOAT]? > > I don't think direct exposure is required; indirect exposure via an array still means that > aspects not related to HTTP requests and HTTP responses are currently contained in the > ServerRequest->server which to me is okay if it is called "ServerRequest" but not okay > if it is called "HttpRequest," "WebRequest," "IncomingRequest," or > "CurrentRequest." > >> Can those two things not be organised as a hash of those values, under >> server and client (or >> remote if you want to keep the terminology - yes I know it will be >> the originating TCP connection host not necessarily the browser host)? As I said, I’ve missed some >> of the discussion but I fail to see the benefit of a class to make all the details of a web request >> and it’s response available…. And then just stick the existing superglobals in it untouched. > > Your response is confusing me. > > You may actually be saying the same thing I am saying. I am referring to the current state of > the RFC where this would be possible: > > $request = new ServerRequest(); > echo $request->server['HOME']; // Output the server's home directory. > This is a very weird thing to me. I realise it’s exposed currently via $_SERVER but what you’re accessing there is an environment variable - it’s also available via $_ENV, or getenv(). This is another reason why I find it weird to put the current ‘goody bag’ of $_SERVER into an object intended to in some way represent a web request and response. That information (environment vars) is very useful, undoubtedly - but accessing it from an object essentially maps to the current, incoming http/web request (regardless of what it’s called), is bizarre IMO. > I am saying that it might be better to have those properties in a different object: > > $info = new ServerInfo(); > echo $info->home; // Output the server's home directory. > print_r( $info->properties ); // Output everything found in $_SERVER object > Without clarifying which things specifically (and that’ll be hard as your other reference is to an env var, few of which are standardised) I’m not sure what I think about this. But I agree (maybe more strongly, I think naming is irrelevant) that things like HOME (i.e. environment variables) have little place in the object Paul is proposing, if it’s meant to be related to “the active request”. >> Things like the query params and body params make sense to be accessible essentially >> (albeit with better names) as-is, because they’re a hash of unknown shape by their very nature - >> but the keys in _SERVER (barring http headers, which are already special cased) are known, and have >> pretty clear definition of their meaning. > > Agreed. > >> Is the proposal really suggesting that a developer would still need to do >> if(!empty($request->server[‘HTTPS’]) && >> $request->server[‘HTTPS’] !== ‘off’) {…} rather than just providing a >> secureTransport property (or https >> if you prefer)? > > Not sure. Paul needs to answer that. > >> One last point, regarding the ‘break out a server specific class’ part. I don’t think >> it’s “wrong” to access these properties from something that is ostensibly related to the >> “current request”, but it feels quite ‘wonky’ to me, the way it’s proposed with the full >> ->server array just copied as-is, AND exposed via dedicated properties >> Cheers > > Yes, I was saying that it is wrong *if* we are going to fine-tune the name to a as closely as > possible mirror what is contained within it. As I said above having server' in > ServerRequest class does not both me but it would bother me if server' were in > a HttpRequest class. > > Does this clarify? Yes, mostly, with the caveat of the part about which entries from $_SERVER (and I think you’re referring to env vars, not the cgi specific vars?) > > -Mike Cheers Stephen