Hi Larry

On Wed, Oct 18, 2023 at 7:26 PM Larry Garfield <la...@garfieldtech.com> wrote:
> > On Fri, Oct 6, 2023 at 3:44 PM Ilija Tovilo <tovilo.il...@gmail.com> wrote:
> >> https://wiki.php.net/rfc/rfc1867-non-post
>
> 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.

I will check if this can be implemented without too many changes.

> 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.)

request_parse_body() is indeed not aimed at other content types as-is.
A generic name would allow extending the function to support other
content types in the future, although it's currently unclear whether
that's desirable. E.g. for JSON, people might be confused why there's
a file index in the returned array that is always empty.

> 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.

A custom exception class sounds reasonable. The mime-type is contained
in the exception message.

> 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.

Whether request_parse_body() can work repeatedly depends on whether
the input has been buffered. application/x-www-form-urlencoded is
buffered and as such works multiple times. multipart/form-data *can*
be buffered if done manually by opening php://input and reading the
whole input before calling request_parse_body(). Yes, for post this
means one needs to disable enable_post_data_reading.

Since files are stored on disk, buffering files means doubling disk
load. Uploading a 2GB file would require at least 4GB of disk space. I
don't think that's a good trade-off as there shouldn't be a reason to
call this function twice.

Ilija

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to