Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:121414
Feedback-ID: i8414410d:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-1019-ged83ad8595-fm-20231002.001-ged83ad85
MIME-Version: 1.0
Message-ID: <4daf6b2a-94dc-4d37-bb4d-512bba5f726b@app.fastmail.com>
In-Reply-To: 
 <CAPyj-LD5=SDXmP5EhR-hfL-FreXXey-CGGoHcqEYq1z44YKRbA@mail.gmail.com>
References: 
 <CAPyj-LD0EDYUZSOM=P9doSZRJ8LipSKKUSBHxV-bRbCV1QsYtQ@mail.gmail.com>
 <CAPyj-LD5=SDXmP5EhR-hfL-FreXXey-CGGoHcqEYq1z44YKRbA@mail.gmail.com>
Date: Wed, 18 Oct 2023 17:25:46 +0000
To: "php internals" <internals@lists.php.net>
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Re: [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
From: larry@garfieldtech.com ("Larry Garfield")

On Fri, Oct 13, 2023, at 11:15 AM, Ilija Tovilo wrote:
> Hi everyone
>
> On Fri, Oct 6, 2023 at 3:44=E2=80=AFPM Ilija Tovilo <tovilo.ilija@gmai=
l.com> wrote:
>> https://wiki.php.net/rfc/rfc1867-non-post
>
> Thank you for the feedback so far. I made a handful of changes to the =
RFC.
>
> * The function is renamed to request_parse_body()
> * The function will now throw instead of emitting warnings when hittin=
g limits
> * The Configuration section was added show how parsing limits may be
> modified per endpoint
> * The php://input is explained better in relation to multipart parsing
>
> Let me know if you have any more feedback.
>
> Ilija

The functionality all seems reasonable to me.  I have a few smaller conc=
erns:

1. Like Derick, I think I'd favor including the config overrides now.

2. Lots of request bodies are not forms these days; they're frequently J=
SON or GraphQL.  This function would be useless in those cases; that's f=
ine, but should the name then suggest that it's for form data only?  req=
uest_parse_form() or similar?  I'm just concerned about misleading peopl=
e into thinking it can parse their JSON bodies, when that's not a thing.=
  (Unless we wanted to provide some kind of callback mechanism, which is=
 probably overkill here.)

3. For an unsupported mime type, I'd recommend a more specific exception=
 than InvalidArgumentException.  Give that a custom sub-class that track=
s what the actual mime type was that the request had and it rejected.

4. I don't quite grok the "input" section.  So if I don't disable the au=
tomatic parsing, does that mean request_parse_body() will always fail?  =
Or will it still work, but just be more memory-wasteful?  That's not cle=
ar to me; I'd prefer if it works but is just memory-wasteful, personally=
, as that would be more portable for projects that want to use it.

--Larry Garfield