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