-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Added support for PUT method #849
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Doesn't the request object check the |
Good catch. It now uses the "raw" method. |
If we're going to support this, which I think we should, I think it should be in HttpFoundation, not Form. |
I don't think form should be changed to support PUT like this. By the time it gets to the form the framework should have sorted this out. Be that with 'native' PUT or fake _method. |
How about sticking this in an HttpKernel RequestListener? |
I have moved the logic into Request directly. Thoughts? |
I've got concerns about PUT, especially in apache. You need to tell apache a script is okay to support put explicitly, or when you ask it what methods it returns put won't be listed. Jquery for example asks apache first before sending a put and kills the request if it doesn't support it. I wonder how safe it would be to provide an htaccess file that says controllers are put ok. Also this form type flag you are checking for, would that be used in a non HTML form api? Seems an odd thing to set to send from JavaScript or curl, I might be missing something though. |
The application/x-www-form-urlencoded content type should be used whenever you use form data stuff, which look just like query params in the url, but placed in the request body. When you do a POST with jQuery, it will encode the object you pass it in that form, and I sure hope it uses the proper header alongside it. @fabpot: One use case that might not be handled is a PUT with multipart/form-data? Not sure if that's valid, but I don't see why it wouldn't be. |
I wonder if this is too magic? There's no way to create a request object during a PUT request without consuming the stream anymore. I liked the idea of reading the stream in |
I think it's alright in the sense that for those requests, you probably don't want to do anything else than parse_str() on that data, but yes it maybe should not be in core (but rather handled in RestBundle and such, and therefore remain optional). |
The core should provide support for this, and probably do it behind the scenes, but there should be a way to disable it. Imagine someone adding a Silex app in a larger PHP application which already makes use of the input stream, for instance. |
What if a client wishes to |
I'm well aware of that bundle but I just don't understand why |
@marijn then you need to get them via |
@stof why not? The client is requesting to |
@marijn When the are not |
I agree with @kriswallsmith, this makes most sense to be to be in Request::createFromGlobals(). I understand the arguments about different content-types, but this should really just be a "best effort" to capture the name value pairs for a standard PUT request. Anything beyond that would require custom process of the data from Request::getContent(). If this is standard behavior, it would not be difficult for developers to understand that php://input is not valid to use in Symfony, much like $_POST or $_GET is not.
|
Sorry but I still fail to see what the semantic difference is between the following two requests:
Versus:
Why should we separate between the two? |
marijn |
@stof, not it's not. JSON should always be wrapped in object tags. |
@marijn I don't see anything in the JSON spec saying that only objects are valid. |
A JSON request can be any valid JSON data type. It does not need to be an object. |
Trying to get the framework to magically translate all these other data types is just going to bloat the framework. If you're saying that JSON data should be automatically decoded in the PUT request, then wouldn't that also apply to POST? Key / Value pair should be the only thing that's supported by the core framework, since it's just an extension of the very same support that's already implemented in a standard POST request. Anything beyond that should be implemented in Bundles. |
I have moved the logic into the |
"Is it alright for everybody now?" I think Fabien needs a hug! XD |
|
||
if ('application/x-www-form-urlencoded' == $request->server->get('CONTENT_TYPE') | ||
&& in_array(strtoupper($request->server->get('REQUEST_METHOD', 'GET'), array('PUT', 'DELETE')) | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have an typo here (parse error), forgot to close bracket.
if ('application/x-www-form-urlencoded' == $request->server->get('CONTENT_TYPE') && in_array(strtoupper($request->server->get('REQUEST_METHOD', 'GET')), array('PUT', 'DELETE')) ) {
I think this is great and I think that createFromGlobals is where this belongs. |
[WCM] Updated documentation for PR symfony/symfony#849...
…sts. PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226.
…sts. PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226.
…sts. PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few years ago, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226.
PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few years ago, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226.
PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few years ago, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226.
Sorry for the reference spam! |
PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few years ago, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226. Conflicts: src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few years ago, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226. Conflicts: src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
PHP, and by extension, Symfony does not support multipart/form-data requests when using any request method other than POST. This limits the ability to implement RESTful architectures using Symfony. This is a follow up to Pull Request symfony#849 I made a few years ago, and addresses concerns brought up by Saldaek at the time. I have implemented this functionality by manually decoding the php://input stream when the request type is PUT, DELETE or PATCH and the Content-Type header is mutlipart/form-data. The implementation is based on an example by [netcoder at stackoverflow](http://stackoverflow.com/a/9469615). This is necessary due to an underlying limitation of PHP, as discussed here: https://bugs.php.net/bug.php?id=55815. This fixes symfony#9226. __Security Concerns__ The main concern I had while implementing this feature, was working with PHP's assumptions as to what constituted an uploaded file. Since the files that this commit creates are not done so through PHP's usual uploaded file mechanisms, none of the uploaded file functions (such as is_uploaded_file, move_uploaded_file) will work with them. As such, I had to implement a mechanism for recording the uploaded files as they were created, so that Symfony's UploadedFile class could be sure that it was not moving non-uploaded files. To achieve this, I added a static array to the UploadedFile class called $files. Any uploaded files that are created outside of PHP's normal mechanism can be recorded here. UploadedFile can then check this list to ensure that if is_uploaded_file returns false, that the file being moved is in fact an uploaded file. This results in two changes to UploadedFile: Firstly, the is_uploaded_file check had to be expanded to also check if the file was recorded in UploadedFile::$files. Secondly, the move_uploaded_file needed to be changed to rename. The change of move_uploaded_file to rename might raise a few red flags, but I do not believe this is a problem. Before rename is called, UploadedFile calls isValid, which checks that the file either is_uploaded_file, or is a member of UploadedFile::$files. Therefore the use of move_uploaded_file is an unnecessary double check (even without these changes). Fix symfony#9226. Conflicts: src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
This adds proper support for PUT method (see #793)