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

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

« previous php.internals (#109005) next »