# PR 1217: Add support for upload files from buffer string in curl extenion - Thread: https://externals.io/message/85859 - Original archive: https://news-web.php.net/php.internals/85859 - Started by Alexander Moskalev on 2015-04-18T10:46:18+00:00 - Messages: 11 - 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 --- ## #85859 — Alexander Moskalev — 2015-04-18T10:46:18+00:00 URL: https://externals.io/message/85859#85859 Hi all! I have few questions for my pull request: https://github.com/php/php-src/pull/1217 Short story: in some special cases we need to upload just created small file(s) by CURL. For example it's autogenerated image or pdf. And libcurl give for us this options: CURLFORM_BUFFER, CURLFORM_BUFFERPTR and CURLFORM_BUFFERLENGTH. My request about adding this options to curl php extension. For basement I used class CURLFile, but can't find solution to use same class, because this class require file name on disc parameter in constructor. So I created new class, now it called as CURLBufferFile. So, my questions are: 1) Can we still integrate this functionality to exists class CURLFile? 2) If no, what do you think about class name? a) CURLBufferFile b) CURLBuffer c) CURLFileBuffer d) ... other 3) What do you think about unserialization for this class? a) Do not accept it (becouse object can contain very big file) b) Accept Thanks! -- With regards, Alexander Moskalev irker@irker.net irker@php.net a.moskalev@corp.badoo.com --- ## #85866 — Stanislav Malyshev — 2015-04-18T21:35:42+00:00 URL: https://externals.io/message/85859#85866 · In reply to: #85859 Hi! > I have few questions for my pull request: > https://github.com/php/php-src/pull/1217 > > Short story: in some special cases we need to upload just created small > file(s) by CURL. For example it's autogenerated image or pdf. And libcurl > give for us this options: CURLFORM_BUFFER, CURLFORM_BUFFERPTR and > CURLFORM_BUFFERLENGTH. My request about adding this options to curl php > extension. I wonder if it's be hard to make streams work there... Then you could just pass php://memory or data:// as the file. Not sure about security implications though, needs some thinking. > For basement I used class CURLFile, but can't find solution to use same > class, because this class require file name on disc parameter in > constructor. So I created new class, now it called as CURLBufferFile. While it does require file name as parameter, nobody says anything about disc there. So in addition to the above, you just could add setFileContents() API (or setFileBuffer? feel free to bikeshed :) that would make it take existing buffer instead of reading the file. > So, my questions are: > 1) Can we still integrate this functionality to exists class CURLFile? I think, yes. Or at least we should try to. The meaning of CURLFile is not "file on disk", it's "something to be fed to CURL to be uploaded as a file", so having buffer does not violate the semantics. > 3) What do you think about unserialization for this class? > a) Do not accept it (becouse object can contain very big file) > b) Accept I don't see big issue in serializing/unserializing it. Yes, it can be big. So can be any string you can pass to unserializer, any object can contain tons of data, nothing different here. This object is just container for its values, so I don't see anything special here. -- Stas Malyshev smalyshev@gmail.com --- ## #85870 — kelerest123@gmail.com — 2015-04-19T15:41:08+00:00 URL: https://externals.io/message/85859#85870 · In reply to: #85866 So, get directly to your questions: > > 1) Can we still integrate this functionality to existing class CURLFile? I think it would be possible, because like @smalyshev said, CURLFile represents symbolic file, that is going to be uploaded with CURL. So my suggestion would be creating a static factory method on this class, something like `CURLFile::createFromBuffer( string )` or `CURLFile::loadFromBuffer( string )`, which would create a CURLFile object and initialize it with the passed contents („buffer“). 3) What do you think about unserialization for this class? > a) Do not accept it (becouse object can contain very big file) > b) Accept I am for (un)serialization for this class. I think it's upon its user to think whether to use it or not (because it could be a benefit, but there are also situations, when it's not -- like that case, in which you have very large file buffer). Best reagards, Kubo2 2015-04-18 23:35 GMT+02:00 Stanislav Malyshev : > Hi! > > > […] > > I wonder if it's be hard to make streams work there... Then you could > just pass php://memory or data:// as the file. Not sure about security > implications though, needs some thinking. > > > […] > > While it does require file name as parameter, nobody says anything about > disc there. So in addition to the above, you just could add > setFileContents() API (or setFileBuffer? feel free to bikeshed :) that > would make it take existing buffer instead of reading the file. > > > […] > > I think, yes. Or at least we should try to. The meaning of CURLFile is > not "file on disk", it's "something to be fed to CURL to be uploaded as > a file", so having buffer does not violate the semantics. > > > […] > > I don't see big issue in serializing/unserializing it. Yes, it can be > big. So can be any string you can pass to unserializer, any object can > contain tons of data, nothing different here. This object is just > container for its values, so I don't see anything special here. > -- > Stas Malyshev > smalyshev@gmail.com --- ## #85882 — Alexander Moskalev — 2015-04-22T08:26:20+00:00 URL: https://externals.io/message/85859#85882 · In reply to: #85870 Thanks to all for feedback! Let's try to integrate new feature to old class. We have constructor in CURLFile with one required parameter: $filename . To avoid BC break we cannot replace this parameter. So I suggest to do it optional and add setBuffer() method. So we can create CURLFile with empty parameters in constructor and fill it with setters. Have two more questions: 1) If we not fill all options or fille not compatible options, when we must throw error? ? And what level of this error? (Sorry, I'm just php coder and know about C language and php source so little) For file from disk(or other source) required $filename. For file from buffer required $buffer and $postname. 2) Curently CURLFile cannot be unserialized, because it contains $filename (see source code). How it compatible with our dicussion? Jakub, I think about static factory methods, but not sure if it will be great. I think using empty constructor more clear. --- ## #85912 — Michael Wallner — 2015-04-23T08:59:33+00:00 URL: https://externals.io/message/85859#85912 · In reply to: #85882 > > Thanks to all for feedback! > > Let's try to integrate new feature to old class. > We have constructor in CURLFile with one required parameter: $filename . > To avoid BC break we cannot replace this parameter. So I suggest to do it > optional and add setBuffer() method. > > So we can create CURLFile with empty parameters in constructor and fill it > with setters. > Have two more questions: > 1) If we not fill all options or fille not compatible options, when we must > throw error? ? And what level of this error? (Sorry, I'm just php coder and > know about C language and php source so little) > For file from disk(or other source) required $filename. For file from > buffer required $buffer and $postname. > 2) Curently CURLFile cannot be unserialized, because it contains $filename > (see source code). How it compatible with our dicussion? > Why not a ctor as in: function __construct ($filename, $buffer = null) { if (isset ($ buffer)) { // use $ buffer } else { // use file contents } } The file name parameter can be of use anyway for posted file contents from buffer. Cheers, Mike --- ## #85913 — Alexey Zakhlestin — 2015-04-23T09:37:01+00:00 URL: https://externals.io/message/85859#85913 · In reply to: #85912 > > > Why not a ctor as in: > > function __construct ($filename, $buffer = null) { > if (isset ($ buffer)) { > // use $ buffer > } else { > // use file contents > } > } > > The file name parameter can be of use anyway for posted file contents from > buffer. Looks good! So, it work like this (+ error handling) function __construct($filename, $buffer = null) { $this->filename = $filename; if (is_null($buffer)) { $this->data = file_get_contents($filename); } else { $this->data = buffer; } } А. --- ## #85919 — Alexander Moskalev — 2015-04-23T11:26:01+00:00 URL: https://externals.io/message/85859#85919 · In reply to: #85912 Because currently CURLFile have this constructor: public __construct ( string $filename [, string $mimetype [, string $postname ]] ) And we cannot replace this arguments to avoid BC break. 2015-04-23 11:59 GMT+03:00 Michael Wallner : > > > > […] > it > > […] > it > > […] > must > > […] > and > > […] > $filename > > […] > > Why not a ctor as in: > > function __construct ($filename, $buffer = null) { > if (isset ($ buffer)) { > // use $ buffer > } else { > // use file contents > } > } > > The file name parameter can be of use anyway for posted file contents from > buffer. > > Cheers, > Mike > -- With regards, Alexander Moskalev irker@irker.net irker@php.net a.moskalev@corp.badoo.com --- ## #85936 — Alexey Zakhlestin — 2015-04-24T07:21:03+00:00 URL: https://externals.io/message/85859#85936 · In reply to: #85919 > > > Because currently CURLFile have this constructor: > public __construct ( > string $filename [, string $mimetype [, string $postname ]] ) > > And we cannot replace this arguments to avoid BC break. (Do not top-post please.) Well… We can introduce another class, which would inherit from CURLFile. Similar to how SplTempFileObject extends SplFileObject. А. --- ## #85939 — Alexander Moskalev — 2015-04-24T08:10:57+00:00 URL: https://externals.io/message/85859#85939 · In reply to: #85936 I think It same as my current pull-request - another class with duplicate functionality and duplicate code. We save only few strings from base class with $postname field. What benefits we will add using inheriting from CURLFile for php developers? пятница, 24 апреля 2015 г. пользователь Alexey Zakhlestin написал: > > > […] > > wrote: > > […] > > (Do not top-post please.) > > Well… We can introduce another class, which would inherit from CURLFile. > Similar to how SplTempFileObject extends SplFileObject. > > > > А. > > -- With regards, Alexander Moskalev irker@irker.net irker@php.net a.moskalev@corp.badoo.com --- ## #85940 — Lester Caine — 2015-04-24T08:16:54+00:00 URL: https://externals.io/message/85859#85940 · In reply to: #85939 > (Do not top-post please.) Unfortunately some people will never be courteous enough to follow the agreed rules of the list :( Yes some email clients are so naff they can't cope with those rules, but perhaps in that case they should be asked simply to switch off quoting altogether? -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #85942 — Alexander Moskalev — 2015-04-24T08:47:34+00:00 URL: https://externals.io/message/85859#85942 · In reply to: #85939 2015-04-24 11:10 GMT+03:00 Alexander Moskalev : > I think It same as my current pull-request - another class with duplicate > functionality and duplicate code. We save only few strings from base class > with $postname field. > What benefits we will add using inheriting from CURLFile for php > developers? > Sorry, was from mobile client. So if we will inherit from CURLFile, we can inherit setters and getters for post filename and mime type. It's OK. But also we will have inherited method "getFilename", that is not logical correct for buffer file. More correct to have another base class for both CURLFile and CURLFIleBuffer. But it so complicated for this small feature request. I think we must use one of simplest ways: new class as in my pull-request or integrate functionality to old class as described before in this thread. -- With regards, Alexander Moskalev irker@irker.net irker@php.net a.moskalev@corp.badoo.com