Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi On 10/18/23 18:07, Derick Rethans wrote: Regarding the cleanup of the files, perhaps the files could be read into a `php://temp` stream (https://www.php.net/manual/en/wrappers.php.php#wrappers.php.memory)? I don't think that reading potentially large files into memory is a great idea. It is precisely because of the memory issue that PHP's existing POST handler writes files to disk, instead of leaving their content available through an entry in $_FILES. To clarify: `php://temp` spills over to disk at a certain size (2 MB by default). Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
On Fri, 6 Oct 2023, Tim Düsterhus wrote: > On 10/6/23 15:44, Ilija Tovilo wrote: > > https://wiki.php.net/rfc/rfc1867-non-post > > > > Regarding the cleanup of the files, perhaps the files could be read into a > `php://temp` stream > (https://www.php.net/manual/en/wrappers.php.php#wrappers.php.memory)? I don't think that reading potentially large files into memory is a great idea. It is precisely because of the memory issue that PHP's existing POST handler writes files to disk, instead of leaving their content available through an entry in $_FILES. cheers, Derick -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi On 10/6/23 18:18, Jakub Zelenka wrote: It should probably explicitly mention that it uses the same inis like max_input_vars, max_file_uploads and max_multipart_body_parts. That reminds me of this thread: https://externals.io/message/118614 I'd love to see some functionality to handle those "input limits" in a well-defined way (e.g. to be able to render a nice error page within my application), but this is currently not (easily) possible, because the error is emitted before my script boots. I'm currently using the error_get_last() workaround mentioned in that thread, but that is everything but great. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi On 10/7/23 14:06, Ilija Tovilo wrote: file. The most common action after a file uploads is arguably to move it to a permanent location using move_uploaded_file(). With a stream I'm not sure if this is actually the most common action, at least in modern applications. Generally there is some kind of processing pipeline. You might want to strip exif metadata from images, check the file contents with a virus scanner, recompress images to save disk usage and in the end upload it to some cloud storage service (e.g. S3). the obvious way to achieve the same is stream_copy_to_stream(). However, as the stream already has a file backing (if big enough, at least) this copy is unnecessary. Please correct me if there's something I have missed. Especially with the "upload to S3" part, folks will generally already work with a stream abstraction, e.g. PSR-7 together with a PSR-18 HTTP client. Flysystem is probably the most widely used file system abstraction, being included in Laravel, and it also has a "writeStream" functionality: https://flysystem.thephpleague.com/docs/usage/filesystem-api/ I also would really like to avoid subtle differences between the automatically and manually invoked files. Given that the overwhelming Yes, that's fair. However there's already some inconsistency, because to my understanding you would need to check for the HTTP method to determine if calling the function is safe or not, no? As an application developer, it would be great if I could safely call this new function also for POST requests. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi Jakub >> https://wiki.php.net/rfc/rfc1867-non-post >> > > It should probably explicitly mention that it uses the same inis like > max_input_vars, max_file_uploads and max_multipart_body_parts. Indeed, I will mention that. Thank you. > It's kind of strange function as I can't decide where it should be placed. I > think it might be better as a stream function if it accepts only stream. It > means it could go to stream funcs and be called stream_parse_post_data > instead but not sure about. But not 100% sure about it as it doesn't exactly > fit there. But seems better than html functions (where it's placed in the > current PR) as it has nothing to do with html IMHO. TBH I have no idea how it landed there. When I created the PoC I just threw it in somewhere, but it's indeed an odd place. I'll try to find something better. I don't think stream is a great place, as the common case of not providing an input does not operate on streams, but on sapi_module.read_post() directly. I also don't think rfc1867.c is a great place as Ben suggested, because the invoked function is actually agnostic to the exact format. Instead, we use the existing functionality that chooses the parser based on the content type, which includes application/x-www-form-urlencoded. Ilija -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi Tim > On 10/6/23 15:44, Ilija Tovilo wrote: > > https://wiki.php.net/rfc/rfc1867-non-post > > > > Regarding the cleanup of the files, perhaps the files could be read into > a `php://temp` stream > (https://www.php.net/manual/en/wrappers.php.php#wrappers.php.memory)? > > While this would cause the function to be incompatible with $_FILES, I > think it would make for a much nicer API and it would also automatically > solve the cleanup problem. php://temp would solve auto-cleanup of files nicely. However, whether they are easier to work with will depend on what you're doing with the file. The most common action after a file uploads is arguably to move it to a permanent location using move_uploaded_file(). With a stream the obvious way to achieve the same is stream_copy_to_stream(). However, as the stream already has a file backing (if big enough, at least) this copy is unnecessary. Please correct me if there's something I have missed. I also would really like to avoid subtle differences between the automatically and manually invoked files. Given that the overwhelming majority will not use PHP with something like RoadRunner, I think it makes more sense to add the special casing (i.e. deleting the files manually) in the uncommon case, than for everybody else to adapt their code for the uncommon case. Ilija -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi Marco, sob., 7 paź 2023 o 00:55 Marco Aurélio Deleu napisał(a): > Just wanted to mention that maybe this is a great opportunity to create a > request_ family and start with request_parse_post_data > My first thought was why the word `_post_` and not for instance instead a`_form_` which better expresses the purpose then? I'd avoid using the "post" word if we tend to provide functionality that is common for other HTTP methods which in fact was the preliminary cause of this discussion. Cheers, Michał Marcin Brzuchalski
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Marco Deleu > On 6 Oct 2023, at 19:39, Ben Ramsey wrote: > > On 10/6/23 11:18, Jakub Zelenka wrote: >> Hi, >>> On Fri, Oct 6, 2023 at 2:44 PM Ilija Tovilo wrote: >>> https://wiki.php.net/rfc/rfc1867-non-post >>> >>> >> It should probably explicitly mention that it uses the same inis like >> max_input_vars, max_file_uploads and max_multipart_body_parts. >> It's kind of strange function as I can't decide where it should be placed. >> I think it might be better as a stream function if it accepts only stream. >> It means it could go to stream funcs and be called stream_parse_post_data >> instead but not sure about. But not 100% sure about it as it doesn't >> exactly fit there. But seems better than html functions (where it's placed >> in the current PR) as it has nothing to do with html IMHO. > > > Maybe it should go in main/rfc1867.c? That's where the php_rfc1867_callback > lives, and this seems somewhat related to that. > > Cheers, > Ben > Just wanted to mention that maybe this is a great opportunity to create a request_ family and start with request_parse_post_data -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
On 10/6/23 11:18, Jakub Zelenka wrote: Hi, On Fri, Oct 6, 2023 at 2:44 PM Ilija Tovilo wrote: https://wiki.php.net/rfc/rfc1867-non-post It should probably explicitly mention that it uses the same inis like max_input_vars, max_file_uploads and max_multipart_body_parts. It's kind of strange function as I can't decide where it should be placed. I think it might be better as a stream function if it accepts only stream. It means it could go to stream funcs and be called stream_parse_post_data instead but not sure about. But not 100% sure about it as it doesn't exactly fit there. But seems better than html functions (where it's placed in the current PR) as it has nothing to do with html IMHO. Maybe it should go in main/rfc1867.c? That's where the php_rfc1867_callback lives, and this seems somewhat related to that. Cheers, Ben OpenPGP_signature Description: OpenPGP digital signature
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi, On Fri, Oct 6, 2023 at 2:44 PM Ilija Tovilo wrote: > https://wiki.php.net/rfc/rfc1867-non-post > > It should probably explicitly mention that it uses the same inis like max_input_vars, max_file_uploads and max_multipart_body_parts. It's kind of strange function as I can't decide where it should be placed. I think it might be better as a stream function if it accepts only stream. It means it could go to stream funcs and be called stream_parse_post_data instead but not sure about. But not 100% sure about it as it doesn't exactly fit there. But seems better than html functions (where it's placed in the current PR) as it has nothing to do with html IMHO. Cheers Jakub Regards Jakub
Re: [PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi On 10/6/23 15:44, Ilija Tovilo wrote: https://wiki.php.net/rfc/rfc1867-non-post Regarding the cleanup of the files, perhaps the files could be read into a `php://temp` stream (https://www.php.net/manual/en/wrappers.php.php#wrappers.php.memory)? While this would cause the function to be incompatible with $_FILES, I think it would make for a much nicer API and it would also automatically solve the cleanup problem. Best regards Tim Düsterhus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] [RFC][Under discussion] RFC1867 for non-POST HTTP verbs
Hi everyone A while ago I wrote an e-mail about RFC1867 (multipart/form-data) not being parsed by PHP for non-POST requests. https://externals.io/message/120641 I'd like to announce an RFC that proposes adding a new function called parse_post_data() to expose the existing functionality to userland, so that the mechanism can be used for other HTTP verbs. https://wiki.php.net/rfc/rfc1867-non-post As opposed to the semantics I suggested in the previous thread, this proposal returns the parsed result instead of populating it directly to the superglobals, and it accepts an optional input stream. Let me know if you have any feedback. Ilija -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
