On Fri, Oct 13, 2023, at 11:15 AM, Ilija Tovilo wrote: > Hi everyone > > On Fri, Oct 6, 2023 at 3:44 PM Ilija Tovilo <tovilo.il...@gmail.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 hitting 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 concerns: 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 JSON or GraphQL. This function would be useless in those cases; that's fine, but should the name then suggest that it's for form data only? request_parse_form() or similar? I'm just concerned about misleading people 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 tracks 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 automatic parsing, does that mean request_parse_body() will always fail? Or will it still work, but just be more memory-wasteful? That's not clear 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
