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

> On 14 Mar 2020, at 01:40, Mike Schinkel <mike@newclarity.net> wrote: > > >> On Mar 13, 2020, at 10:55 AM, Paul M. Jones <pmjones@pmjones.io> wrote: >> >> One other alternative John & I contemplated was >> Web{Request,Response,ResponseSender} -- do you think that might be a >> reasonable alternative to HTTP, one that is "adjacent" but not overly-specific? That would >> net us: >> >> - WebRequest >> - WebResponse >> - WebResponseSender >> >> I didn't like the look of it previously, and I don't think I like the look of it >> now, but ... (/me shrugs). >> >> >>> However, that also may be confusing if people expect it to be a construct for making >>> outgoing requests. >> >> Yes, that's another tricky bit in the naming -- making a distinction between the >> objects as representative of client operations (send request, receive response) and server >> operations (receive request, send response). Thus the current Server >> prefix (however unsatisfactory it may be) to indicate their operational context. >> >> Your Incoming and Outgoing >> prefixes, minus the HTTP, would net us: >> >> - IncomingRequest >> - OutgoingResponse >> - OutgoingResponseSender >> >> I will need to ponder on those. >> >>> The user land implementation I’ve been using ’solves’ this by using a >>> HTTP namespace, and then provides 5H���P� >>> ��"?��VRequest and Response (for an outgoing - i.e. >>> curl - HTTP Request, and the corresponding HTTP Response) objects and then >>> CurrentRequest and CurrentResponse >>> for what your RFC proposes (i.e. the active request made to php). >> >> Yes, userland does operate that way. However, I think adding an HTTP namespace to PHP >> itself is something to be avoided, so emulating userland here is not an option. >> >> >>> As with anything any of us has written, I’m not 100% sold on >>> ‘Current{Request,Response}` even after writing it, but I think it’s at least a little more >>> specific about what they do, when the namespace is taken into account. >> >> Current{...} is not something we had previously considered; >> that would net us, in the global namespace: >> >> - CurrentRequest >> - CurrentResponse >> - CurrentResponseSender >> >> I will need to ponder on those as well. >> >> Any further thoughts or opinions on this, Stephen? Or from anyone else? > > One issue that I have is, if we are going to fine-tune the naming to make sure it matches > *exactly* then I think that CurrentRequest->server does not make sense. > > Maybe if you choose one of these names you should break out the server-specific items into > their own class/object? > > -Mike > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > Hi Mike, (I realise some of these points are possibly more addressed to Paul than yourself, this is just where my brain went when I dug into what you were mentioning) I apologise if I’ve missed part of the discussion but what do you mean by “make sure it matches *exactly*. Do you mean how ->server is listed as being a copy of the $_SERVER super global? If so, can someone point me to the specific logic behind that, given how many parts of it are already exposed via ‘dedicated’ properties of the proposed class? From what I can see (and I may have missed some) the parts of $_SERVER not exposed in some way “directly” on ServerRequest (nee CurrentRequest) are the actual “server” parts: the SERVER_* keys, doc root, PHP_SELF; and the ‘client’ parts: REMOTE_*; plus few random stragglers like PATH_INFO, and for some reason REQUEST_TIME[_FLOAT]? Can those two things not be organised as a hash of those values, under server and client (or remote if you want to keep the terminology - yes I know it will be the originating TCP connection host not necessarily the browser host)? As I said, I’ve missed some of the discussion but I fail to see the benefit of a class to make all the details of a web request and it’s response available…. And then just stick the existing superglobals in it untouched. Things like the query params and body params make sense to be accessible essentially (albeit with better names) as-is, because they’re a hash of unknown shape by their very nature - but the keys in _SERVER (barring http headers, which are already special cased) are known, and have pretty clear definition of their meaning. Is the proposal really suggesting that a developer would still need to do if(!empty($request->server[‘HTTPS’]) && $request->server[‘HTTPS’] !== ‘off’) {…} rather than just providing a secureTransport property (or https if you prefer)? One last point, regarding the ‘break out a server specific class’ part. I don’t think it’s “wrong” to access these properties from something that is ostensibly related to the “current request”, but it feels quite ‘wonky’ to me, the way it’s proposed with the full ->server array just copied as-is, AND exposed via dedicated properties. Cheers Stephen