# Upgrade cURL extension - Thread: https://externals.io/message/58832 - Original archive: https://news-web.php.net/php.internals/58832 - Started by Pierrick Charron on 2012-03-10T16:57:40+00:00 - Messages: 29 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/58832.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #58832 — Pierrick Charron — 2012-03-10T16:57:40+00:00 URL: https://externals.io/message/58832#58832 Hi all, As you may know, the cURL PHP extension is currently not in sync (featurewise) with the original libcurl. I started to work on it to make it as close as possible from the original libcurl. I also did some cleaning to make it easier to maintain (ordered all the constants/features by their libcurl version). All those changes were made on the trunk branch only. I wanted to make this new version available in PHP5.4 but unfortunately I did finish my work when it was already in RC phase. The question now is should we include this new version in PHP5.4.1 or should we wait for PHP 5.5/6/7 or whatever PHP next will be. There is no feature break (AFAIK) so all the previous code should work as expected. You'll find the list of new features attached and the last code in the trunk branch. So please, test the new version of the curl extension, review the code, and give your input on either or not those changes should be merge on 5.4.1 Thanks Pierrick --- ## #58833 — Pierre Joye — 2012-03-10T17:13:54+00:00 URL: https://externals.io/message/58832#58833 · In reply to: #58832 hi Pierrick, I would rather go with php-next. The amount of changes are not safe for a now very stable version in 5.3 and 5.4 (same code base), while the code could be nicer as you did it in trunk. Cheers, > Hi all, > > As you may know, the cURL PHP extension is currently not in sync > (featurewise) with the original libcurl. I started to work on it to > make it as close as possible from the original libcurl. I also did > some cleaning to make it easier to maintain (ordered all the > constants/features by their libcurl version). All those changes were > made on the trunk branch only. > > I wanted to make this new version available in PHP5.4 but > unfortunately I did finish my work when it was already in RC phase. > The question now is should we include this new version in PHP5.4.1 or > should we wait for PHP 5.5/6/7 or whatever PHP next will be. There is > no feature break (AFAIK) so all the previous code should work as > expected. You'll find the list of new features attached and the last > code in the trunk branch. > > So please, test the new version of the curl extension, review the > code, and give your input on either or not those changes should be > merge on 5.4.1 > > Thanks > Pierrick > > -- > > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #58834 — Simon Schick — 2012-03-10T17:28:37+00:00 URL: https://externals.io/message/58832#58834 · In reply to: #58833 2012/3/10 Pierre Joye > > hi Pierrick, > > I would rather go with php-next. The amount of changes are not safe > for a now very stable version in 5.3 and 5.4 (same code base), while > the code could be nicer as you did it in trunk. > > Cheers, > > > > […] > > > > -- > Pierre > > @pierrejoye | http://blog.thepimp.net | http://www.libgd.org > > -- > > > Hi, all I'd like to see a new interface for curl in php ... I have no special idea how, but I heard from several people that they pretty much don't like the way curl is implemented in php. May we have a little brainstorming around that .. may some other language has some good implementation ... When I see the list of constants, I feel like that is way to much ... this could be grouped somehow ... Bye Simon --- ## #58835 — Reindl Harald — 2012-03-10T17:41:42+00:00 URL: https://externals.io/message/58832#58835 · In reply to: #58834 Am 10.03.2012 18:28, schrieb Simon Schick: > I'd like to see a new interface for curl in php ... I have no special > idea how, but I heard from several people that they pretty much don't > like the way curl is implemented in php. many other people would not like to break their perfect working code! --- ## #58836 — Simon Schick — 2012-03-10T17:49:55+00:00 URL: https://externals.io/message/58832#58836 · In reply to: #58835 2012/3/10 Reindl Harald : > > > Am 10.03.2012 18:28, schrieb Simon Schick: > > […] > > many other people would not like to break their > perfect working code! > Hi, Reindl I do not mean to drop the implementation as it is right now - but think of something different for the future. If there would be an additional implementation it would probably be like mysqli, where you have an oop-implmentation and a non-oop. Bye Simon --- ## #58837 — Michael Dowling — 2012-03-10T17:53:00+00:00 URL: https://externals.io/message/58832#58837 · In reply to: #58835 I think all the php curl bindings should do is expose the power of curl in php with an interface similar to the c interface. I think it's up to third party libraries to put an easier to use interface on top of the curl bindings. Thanks, Michael > > > Am 10.03.2012 18:28, schrieb Simon Schick: > > […] > > many other people would not like to break their > perfect working code! --- ## #58838 — David Soria Parra — 2012-03-10T18:22:44+00:00 URL: https://externals.io/message/58832#58838 · In reply to: #58832 > --047d7b2ee19338d79004bae668f3 > Content-Type: text/plain; charset=ISO-8859-1 > > Hi all, > > As you may know, the cURL PHP extension is currently not in sync > (featurewise) with the original libcurl. I started to work on it to > make it as close as possible from the original libcurl. I also did > some cleaning to make it easier to maintain (ordered all the > constants/features by their libcurl version). All those changes were > made on the trunk branch only. > > I wanted to make this new version available in PHP5.4 but > unfortunately I did finish my work when it was already in RC phase. > The question now is should we include this new version in PHP5.4.1 or > should we wait for PHP 5.5/6/7 or whatever PHP next will be. There is > no feature break (AFAIK) so all the previous code should work as > expected. You'll find the list of new features attached and the last > code in the trunk branch. Hi Pierrick, as long as your implementation does not contain crucial security fixes, I prefer not having it in 5.4 Let's try to keep 5.4 as stable as possible and minimize the amount of new code. --- ## #58839 — Stas Malyshev — 2012-03-11T07:33:20+00:00 URL: https://externals.io/message/58832#58839 · In reply to: #58832 Hi! > I wanted to make this new version available in PHP5.4 but > unfortunately I did finish my work when it was already in RC phase. > The question now is should we include this new version in PHP5.4.1 or > should we wait for PHP 5.5/6/7 or whatever PHP next will be. There is > no feature break (AFAIK) so all the previous code should work as > expected. You'll find the list of new features attached and the last > code in the trunk branch. Can't you make it also available as pecl extension, which could be built on 5.4? This way people could enjoy the benefits of your work without stable branch being disrupted and BC problems raised. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #58840 — Tom Boutell — 2012-03-11T15:22:34+00:00 URL: https://externals.io/message/58832#58840 · In reply to: #58839 I'd sure like a PHP extension that didn't have this obvious and nasty bug: https://bugs.php.net/bug.php?id=46439 On Sun, Mar 11, 2012 at 3:33 AM, Stas Malyshev wrote: > Hi! > > > I wanted to make this new version available in PHP5.4 but > > […] > > Can't you make it also available as pecl extension, which could be built > on 5.4? This way people could enjoy the benefits of your work without > stable branch being disrupted and BC problems raised. > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 > > > -- > > > > -- Tom Boutell P'unk Avenue 215 755 1330 punkave.com window.punkave.com --- ## #58841 — Ilia Alshanetsky — 2012-03-11T16:03:50+00:00 URL: https://externals.io/message/58832#58841 · In reply to: #58836 The oop interface to cURl is already available, take a look at http://pecl.php.net/package/pecl_http extension. It provides OOP interface and takes care of many of the input preparation or output parsing tasks normally you need to do in PHP when working with cURL. On Sat, Mar 10, 2012 at 12:49 PM, Simon Schick wrote: > 2012/3/10 Reindl Harald : > > […] > > Hi, Reindl > > I do not mean to drop the implementation as it is right now - but > think of something different for the future. > If there would be an additional implementation it would probably be > like mysqli, where you have an oop-implmentation and a non-oop. > > Bye > Simon --- ## #58842 — Ilia Alshanetsky — 2012-03-11T16:04:45+00:00 URL: https://externals.io/message/58832#58842 · In reply to: #58839 Stas, That could work for people who don't have cURL built-in to their PHP version, but otherwise create a symbol conflict. > Hi! > > > > […] > > > Can't you make it also available as pecl extension, which could be built on > 5.4? This way people could enjoy the benefits of your work without stable > branch being disrupted and BC problems raised. > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 --- ## #58843 — Pierre Joye — 2012-03-11T19:38:12+00:00 URL: https://externals.io/message/58832#58843 · In reply to: #58840 hi Tom, For one, it is mapped to the libcurl constants and behavior. Also this but report contains clear comment about this issue being a documentation problem, contribution welcome :) If you consider it as something that should be changed, then please feel free to add a comment there as well, or a patch :) But that's not really what we discuss here but the new code proposed by Pierrick. Cheers, > I'd sure like a PHP extension that didn't have this obvious and nasty bug: > > https://bugs.php.net/bug.php?id=46439 > > On Sun, Mar 11, 2012 at 3:33 AM, Stas Malyshev wrote: > > > […] > > > -- > Tom Boutell > P'unk Avenue > 215 755 1330 > punkave.com > window.punkave.com -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org --- ## #58846 — Tom Boutell — 2012-03-11T22:09:25+00:00 URL: https://externals.io/message/58832#58846 · In reply to: #58843 I do see now that at curl did introduce the @ craziness. So it is unfair of me to single out PHP for introducing it. I'm not sure if the @ syntax is a sneaky feature of the standard C API, but it's definitely a sneaky feature of the command line utility. I did include a comment there when I first opened that bug report, proposing a more appropriate and sustainable interface. Documentation changes would make it possible to avoid the problem by looking carefully for strange "gotchas" in the fine print - but not to actually submit a value starting with a @, if, y'know, I wanted to do something crazy-nutty like send arbitrary data legal in any other form submission (: Apologies for hijacking the discussion. The '@' business does not come from libcurl. Native C libcurl > hi Tom, > > For one, it is mapped to the libcurl constants and behavior. > > Also this but report contains clear comment about this issue being a > documentation problem, contribution welcome :) > > If you consider it as something that should be changed, then please > feel free to add a comment there as well, or a patch :) > > But that's not really what we discuss here but the new code proposed > by Pierrick. > > Cheers, > > > > […] > bug: > > […] > > > > -- > Pierre > > @pierrejoye | http://blog.thepimp.net | http://www.libgd.org > -- Tom Boutell P'unk Avenue 215 755 1330 punkave.com window.punkave.com --- ## #58847 — Tom Boutell — 2012-03-11T22:10:25+00:00 URL: https://externals.io/message/58832#58847 · In reply to: #58846 Sigh, I could have edited that better, but I think the apology came across (: This is still a thing worth fixing. > I do see now that at curl did introduce the @ craziness. So it is unfair > of me to single out PHP for introducing it. I'm not sure if the @ syntax is > a sneaky feature of the standard C API, but it's definitely a sneaky > feature of the command line utility. > > I did include a comment there when I first opened that bug report, > proposing a more appropriate and sustainable interface. Documentation > changes would make it possible to avoid the problem by looking carefully > for strange "gotchas" in the fine print - but not to actually submit a > value starting with a @, if, y'know, I wanted to do something crazy-nutty > like send arbitrary data legal in any other form submission (: > > Apologies for hijacking the discussion. > > The '@' business does not come from libcurl. Native C libcurl > > > > > […] > > > > -- > Tom Boutell > P'unk Avenue > 215 755 1330 > punkave.com > window.punkave.com > > -- Tom Boutell P'unk Avenue 215 755 1330 punkave.com window.punkave.com --- ## #58849 — Stas Malyshev — 2012-03-11T23:29:45+00:00 URL: https://externals.io/message/58832#58849 · In reply to: #58840 Hi! > I'd sure like a PHP extension that didn't have this obvious and nasty bug: > > https://bugs.php.net/bug.php?id=46439 This doesn't look good. Documentation does say the @ prefix exists, but it has very high potential of creating security holes for unsuspecting people. open_basedir would help limit the impact, but still it's not a good thing. Any ideas on fixing it without breaking the BC? -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #58851 — Pierrick Charron — 2012-03-12T03:55:19+00:00 URL: https://externals.io/message/58832#58851 · In reply to: #58839 I thought about it but I think it may confuse people to have two different extensions with the same name, same interface, but one in pecl and one in the core package (the difference between the 2 versions are not that big). Also as ilia mentioned if someone already have the original version, they will have symbol conflicts. So I guess we could just wait until PHP next unless someone have an other nice solution which will not confuse people :) Thanks all for your input. Pierrick > Hi! > > > > […] > > > Can't you make it also available as pecl extension, which could be built on > 5.4? This way people could enjoy the benefits of your work without stable > branch being disrupted and BC problems raised. > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 --- ## #58852 — Kris Craig — 2012-03-12T04:39:55+00:00 URL: https://externals.io/message/58832#58852 · In reply to: #58851 I think waiting until PHP++ is probably the best approach. It would've been nice to have the current libcurl version in 5.4.0, but since we're not talking about any critical bug/security fixes, I don't think it's that big a deal either way. So we may as well just sit on it for now. --Kris On Sun, Mar 11, 2012 at 8:55 PM, Pierrick Charron wrote: > I thought about it but I think it may confuse people to have two > different extensions with the same name, same interface, but one in > pecl and one in the core package (the difference between the 2 > versions are not that big). Also as ilia mentioned if someone already > have the original version, they will have symbol conflicts. So I guess > we could just wait until PHP next unless someone have an other nice > solution which will not confuse people :) > > Thanks all for your input. > > Pierrick > > > > […] > on > > […] --- ## #58883 — Richard Lynch — 2012-03-12T19:36:40+00:00 URL: https://externals.io/message/58832#58883 · In reply to: #58849 > Hi! > > > […] > > This doesn't look good. Documentation does say the @ prefix exists, > but > it has very high potential of creating security holes for unsuspecting > people. open_basedir would help limit the impact, but still it's not a > good thing. Any ideas on fixing it without breaking the BC? Ouch. Issue an `E_NOTICE` when it happens? Add a new CURLOPT_FILEFIELDS that takes an array of the parameters that are supposed to be files, so the ones that are expected to have "@..." do not fire the E_NOTICE. Issuing `E_NOTICE` is a BC, I suppose, but you'd think people would appreciate an alert about a potential security threat... -- brain cancer update: http://richardlynch.blogspot.com/search/label/brain%20tumor Donate: https://www.paypal.com/cgi-bin/webscr?cmd=_s-xclick&hosted_button_id=FS9NLTNEEKWBE --- ## #58898 — keisial@gmail.com — 2012-03-12T21:58:30+00:00 URL: https://externals.io/message/58832#58898 · In reply to: #58883 > > > […] > Ouch. > > Issue an `E_NOTICE` when it happens? > > Add a new CURLOPT_FILEFIELDS that takes an array of the parameters > that are supposed to be files, so the ones that are expected to have > "@..." do not fire the E_NOTICE. > > Issuing `E_NOTICE` is a BC, I suppose, but you'd think people would > appreciate an alert about a potential security threat... That would only trigger the notice when you transfer data beginning with an @, which would end up being only when finally attacked. I'd make it need another option to make @ options work (eg. CURLOPT_AT_TRANSFERS_FILES) which default to false. Similar to SO_BROADCAST, where binding a socket to a broadcast address is not enough to send the packets there. It *is* a BC break, but the current API is badly provided. I don't see a way to work around that. A one-line fix to get the previous not-too-used(?) behavior back seems as good as can be achieved. It is also possible to make a completely new option API without those problems, and deprecate the old one, but that's still a BC break. --- ## #58912 — Pierrick Charron — 2012-03-13T12:32:35+00:00 URL: https://externals.io/message/58832#58912 · In reply to: #58849 We could add a flag to disable the @ usage but I'm against having the '@' usage disabled by default. This BC break would be in my opinion too big. An other solution would be to have something like (We will also have to add the type and filename support to this solution so this is just a first proposal) : curl_setopt($curl_handle, CURLOPT_HTTPPOSTFIELDS, array( 'firstname' => 'pierrick', 'lastname' => array(CURL_FORMSTR, 'charron'), 'lastname' => array(CURL_FORMFILE, '/home/pierrick/picture.png') ); Pierrick Otherwise the safest way for people is to use http_build_query on their parameter array. Pierrick > Hi! > > > […] > > > This doesn't look good. Documentation does say the @ prefix exists, but it > has very high potential of creating security holes for unsuspecting people. > open_basedir would help limit the impact, but still it's not a good thing. > Any ideas on fixing it without breaking the BC? > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 --- ## #64427 — Pierrick Charron — 2012-12-24T18:03:29+00:00 URL: https://externals.io/message/58832#64427 · In reply to: #58849 Hi all, I know this topic was opened a long time ago, but I would like to get it resolved before 5.5 got released. One solution proposed by Richard Lynch was to add a new CURLOPT_FILEFIELDS that takes an array of the parameters that are supposed to be files, so the ones that are expected to have '@'. One problem that we may have to deal with this solution is that the user will have to add all the post params in 2 steps (first for the string data and then for the files). Internally, libcurl only allow one call to CURLOPT_HTTPPOST (second will override the first one), so it may become unclear either the new data are appended, or overwrite the old one. An other solution proposed by Ángel González was to add a new option to disable the '@' check, problem with this is that it will only prevent uploading unwanted files if someone write something starting with an '@', but it also disable completely the feature. A last solution would be to something similar to libcurl curl_formadd (this one could be added to the previous one so that the old way work but there is a more secure way to do it) : curl_setopt($curl_handle, CURLOPT_POSTFIELDS, array( 'firstname' => 'pierrick', 'lastname' => array(CURLFORM_CONTENTS => 'charron'), 'lastname' => array(CURLFORM_FILENAME => 'name.png', CURLFORM_FILE => '/home/pierrick/picture.png', CURLFORM_CONTENTTYPE => 'image/jpg') ); One thing we have to think about this solution is if at some point we want to allow sending array via curl, will it conflict ? Do someone have an other better idea ? Which one would you prefer and see implemented ? Thanks all for your inputs Pierrick > Hi! > > > […] > > > This doesn't look good. Documentation does say the @ prefix exists, but it > has very high potential of creating security holes for unsuspecting people. > open_basedir would help limit the impact, but still it's not a good thing. > Any ideas on fixing it without breaking the BC? > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 --- ## #64459 — Stas Malyshev — 2012-12-29T01:40:17+00:00 URL: https://externals.io/message/58832#64459 · In reply to: #64427 Hi! > I know this topic was opened a long time ago, but I would like to get > it resolved before 5.5 got released. I agree, it looks like a place where we could use improvement, current API is kind of dangerous. > A last solution would be to something similar to libcurl curl_formadd > (this one could be added to the previous one so that the old way work > but there is a more secure way to do it) : > > curl_setopt($curl_handle, CURLOPT_POSTFIELDS, array( > 'firstname' => 'pierrick', > 'lastname' => array(CURLFORM_CONTENTS => 'charron'), > 'lastname' => array(CURLFORM_FILENAME => 'name.png', CURLFORM_FILE > => '/home/pierrick/picture.png', CURLFORM_CONTENTTYPE => 'image/jpg') > ); > > One thing we have to think about this solution is if at some point we > want to allow sending array via curl, will it conflict ? I don't think we would allow sending arrays through curl, however there's another problem - theoretically, if user can access the data you put in $lastname variable, in many contexts it's not hard to put an array there either - i.e. if you have a form that has element lastname that posts to $lastname and then you do: curl_setopt($curl_handle, CURLOPT_POSTFIELDS, array( 'lastname' => $lastname, /// etc. Then you could also create a form that posts to lastname[filename] and simulate this array too. So it's not a complete solution. I'm thinking maybe using separate option for files and deprecating the current one may be better idea. Unless somebody has even better solution :) -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #64460 — Pierrick Charron — 2012-12-29T16:06:01+00:00 URL: https://externals.io/message/58832#64460 · In reply to: #64459 Hi, You're right the proposed implementation did not removed the issue but was just changing the way to produce it and I agree that the most secure way to do it would be as you suggested to add a separate option but I see some issues that we will have. Usually libcurl doesn't allow to call curl_easy_setopt with the same option twice on the same easy handle, it will overwrite the data set by the first call. php/cURL abstract multiple ways to send post data all of them by the usage of CURLOPT_POSTFIELDS. When you set the CURLOPT_POSTFIELD to an array using the php/curl api, php/curl will internally use the CURLOPT_HTTPPOST option, and when you set the same option to a string, it will call the CURLOPT_COPYPOSTFIELDS. So first we will have to store the value of both CURLOPT_POSTFIELD and CURLOPT_POSTFILEFIELD (or whatever we want to call it) so that we always call CURLOPT_HTTPPOST once with the merge of the 2 options but still, if someone do something like this curl_setopt($ch, CURLOPT_POSTFIELD, 'foo=bar'); curl_setopt($ch, CURLOPT_POSTFILEFIELD, array('upload' => 'filename.jpg')); This will not work because we need to use both CURLOPT_HTTPPOST and CURLOPT_COPYPOSTFIELDS which are not compatible. Idea on how we could solve this are welcome. Thanks Pierrick > Hi! > > > […] > > I agree, it looks like a place where we could use improvement, current > API is kind of dangerous. > > > […] > > I don't think we would allow sending arrays through curl, however > there's another problem - theoretically, if user can access the data you > put in $lastname variable, in many contexts it's not hard to put an > array there either - i.e. if you have a form that has element lastname > that posts to $lastname and then you do: > > curl_setopt($curl_handle, CURLOPT_POSTFIELDS, array( > 'lastname' => $lastname, > /// etc. > > Then you could also create a form that posts to lastname[filename] and > simulate this array too. So it's not a complete solution. I'm thinking > maybe using separate option for files and deprecating the current one > may be better idea. Unless somebody has even better solution :) > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 --- ## #64466 — Stas Malyshev — 2013-01-02T07:15:52+00:00 URL: https://externals.io/message/58832#64466 · In reply to: #64460 Hi! I'm thinking maybe the best solution is to have a new class - say, CurlFile - and do this: $file = new CurlFile("myface.png", "image/png"); curl_setopt($ch, CURLOPT_POSTFIELDS, array("foo" => "bar", "picture" => $file); This would allow us to do two things: 1. Protect ourselves from injection since you can not inject objects (there's still a matter of serialized data, but this can be handled by the class itself). 2. Support much more options in the file - e.g., right now it does not support streams, but libcurl has CURLFORM_STREAM - maybe we could use it, or maybe just read in the stream data and use it as CURLFORM_BUFFER. Of course, that would not work for big files, but here we are able to use much more options than with old @-based API. Any holes in this idea? If not, I'll try to make an RFC for it. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 --- ## #64473 — johannes@schlueters.de — 2013-01-02T15:39:51+00:00 URL: https://externals.io/message/58832#64473 · In reply to: #64466 Stas Malyshev wrote: >Hi! > >I'm thinking maybe the best solution is to have a new class - say, >CurlFile - and do this: > >$file = new CurlFile("myface.png", "image/png"); >curl_setopt($ch, CURLOPT_POSTFIELDS, array("foo" => "bar", "picture" => >$file); What I wonder about in this thread: If we struggle here why not take the full step and abstract curl details comletely away and provide something like pecl/http by default instead? johannes --- Next page: https://externals.io/message/58832.md?page=2