Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:109006 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 38776 invoked from network); 13 Mar 2020 22:03:32 -0000 Received: from unknown (HELO php-smtp4.php.net) (45.112.84.5) by pb1.pair.com with SMTP; 13 Mar 2020 22:03:32 -0000 Received: from php-smtp4.php.net (localhost [127.0.0.1]) by php-smtp4.php.net (Postfix) with ESMTP id 242871804FD for ; Fri, 13 Mar 2020 13:25:15 -0700 (PDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on php-smtp4.php.net X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,HTML_MESSAGE, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.2 X-Spam-ASN: AS36024 206.123.114.0/23 X-Spam-Virus: No X-Envelope-From: Received: from mail1.25mail.st (mail1.25mail.st [206.123.115.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by php-smtp4.php.net (Postfix) with ESMTPS for ; Fri, 13 Mar 2020 13:25:14 -0700 (PDT) Received: from [10.0.1.201] (unknown [49.48.245.167]) by mail1.25mail.st (Postfix) with ESMTPSA id 42A486034B; Fri, 13 Mar 2020 20:25:04 +0000 (UTC) Message-ID: Content-Type: multipart/alternative; boundary="Apple-Mail=_41370326-84CA-4937-9F0B-58E1664DE730" Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\)) Date: Sat, 14 Mar 2020 03:25:01 +0700 In-Reply-To: <8A04B273-B298-4A2F-B0A5-8DF1F1D129A9@newclarity.net> Cc: "Paul M. Jones" , PHP Internals To: Mike Schinkel References: <50BD013E-CF72-414C-BBC0-A7A2E45CBDDB@pmjones.io> <0B40E6E5-342F-4D81-9CAA-A0C0739A7718@pmjones.io> <102D234E-BC5D-42B2-ABC4-93980C1F76A4@pmjones.io> <71A325BA-3F53-4C9D-BE6A-D9A068A42AEA@koalephant.com> <8A04B273-B298-4A2F-B0A5-8DF1F1D129A9@newclarity.net> X-Mailer: Apple Mail (2.3608.60.0.2.5) Subject: Re: [PHP-DEV] RFC: Server-Side Request and Response Objects (v2) From: php-lists@koalephant.com (Stephen Reay) --Apple-Mail=_41370326-84CA-4937-9F0B-58E1664DE730 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > On 14 Mar 2020, at 02:59, Mike Schinkel wrote: >=20 >> On Mar 13, 2020, at 3:23 PM, Stephen Reay = wrote: >>=20 >> Hi Mike, >>=20 >> (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) >=20 > (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)=20 >=20 > 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. =20 >=20 > 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. >=20 >> I apologise if I=E2=80=99ve missed part of the discussion but what do = you mean by =E2=80=9Cmake sure it matches *exactly*. >=20 > 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: >=20 > "This extension and the classes it provides are inherently about = HTTP requests made to a php =E2=80=98server=E2=80=99, and the response = it sends back - and yet it=E2=80=99s called = Server{Request,Response,Buffer} etc=E2=80=A6. The =E2=80=9Cserver=E2=80=9D= part is superfluous in the context of a php web application, because = it=E2=80=99s all =E2=80=9Cserver=E2=80=9D side, and while uncommon = it=E2=80=99s not impossible to write *other* types of network server = using PHP." >=20 > "TLDR: if you aren=E2=80=99t 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." >=20 > 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=E2=80=99re probably talking about different =E2=80=99server = specific=E2=80=99 parts of the $_SERVER super global array.. I=E2=80=99m = talking about the stuff that=E2=80=99s 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. >=20 >=20 >> 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 = =E2=80=98dedicated=E2=80=99 properties of the proposed class? =46rom = what I can see (and I may have missed some) the parts of $_SERVER not = exposed in some way =E2=80=9Cdirectly=E2=80=9D on ServerRequest (nee = CurrentRequest) are the actual =E2=80=9Cserver=E2=80=9D parts: the = `SERVER_*` keys, doc root, PHP_SELF; and the =E2=80=98client=E2=80=99 = parts: REMOTE_*; plus few random stragglers like PATH_INFO, and for some = reason REQUEST_TIME[_FLOAT]? >=20 > 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." >=20 >> 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=E2=80=99ve 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=E2=80=99s response available=E2=80=A6.= And then just stick the existing superglobals in it untouched. >=20 > Your response is confusing me. =20 >=20 > 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: >=20 > $request =3D new ServerRequest(); > echo $request->server['HOME']; // Output the server's home directory. >=20 This is a very weird thing to me. I realise it=E2=80=99s exposed = currently via $_SERVER but what you=E2=80=99re accessing there is an = environment variable - it=E2=80=99s also available via $_ENV, or = getenv(). This is another reason why I find it weird to put the current = =E2=80=98goody bag=E2=80=99 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=E2=80=99s called), is bizarre = IMO. > I am saying that it might be better to have those properties in a = different object: >=20 > $info =3D new ServerInfo(); > echo $info->home; // Output the server's home directory. > print_r( $info->properties ); // Output everything found in $_SERVER = object >=20 Without clarifying which things specifically (and that=E2=80=99ll be = hard as your other reference is to an env var, few of which are = standardised) I=E2=80=99m 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=E2=80=99s meant to be related to =E2=80=9Cthe active = request=E2=80=9D.=20 >> Things like the query params and body params make sense to be = accessible essentially (albeit with better names) as-is, because = they=E2=80=99re 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. >=20 > Agreed. >=20 >> Is the proposal really suggesting that a developer would still need = to do `if(!empty($request->server[=E2=80=98HTTPS=E2=80=99]) && = $request->server[=E2=80=98HTTPS=E2=80=99] !=3D=3D =E2=80=98off=E2=80=99) = {=E2=80=A6}` rather than just providing a `secureTransport` property (or = `https` if you prefer)?=20 >=20 > Not sure. Paul needs to answer that. >=20 >> One last point, regarding the =E2=80=98break out a server specific = class=E2=80=99 part. I don=E2=80=99t think it=E2=80=99s =E2=80=9Cwrong=E2=80= =9D to access these properties from something that is ostensibly related = to the =E2=80=9Ccurrent request=E2=80=9D, but it feels quite =E2=80=98wonk= y=E2=80=99 to me, the way it=E2=80=99s proposed with the full ->server = array just copied as-is, AND exposed via dedicated properties >> Cheers >=20 > 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. >=20 > Does this clarify? Yes, mostly, with the caveat of the part about which entries from = $_SERVER (and I think you=E2=80=99re referring to env vars, not the cgi = specific vars?)=20 >=20 > -Mike Cheers=20 Stephen=20 --Apple-Mail=_41370326-84CA-4937-9F0B-58E1664DE730--