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:
-
Can we still integrate this functionality to exists class CURLFile?
-
If no, what do you think about class name?
a) CURLBufferFile
b) CURLBuffer
c) CURLFileBuffer
d) ... other -
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
Hi!
I have few questions for my pull request:
https://github.com/php/php-src/pull/1217Short 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:
- 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.
- 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
So, get directly to your questions:
- 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“).
- 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 smalyshev@gmail.com:
Hi!
I have few questions for my pull request:
https://github.com/php/php-src/pull/1217Short 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:
- 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.
- What do you think about unserialization for this class?
a) Do not accept it (becouse object can contain very big file)
b) AcceptI 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
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:
- 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. - 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.
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:
- 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.- 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
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;
}
}
А.
Because currently CURLFile have this constructor:
public __construct http://php.net/manual/en/curlfile.construct.php (
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 mike@php.net:
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:
- 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.- 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
--
With regards, Alexander Moskalev
irker@irker.net
irker@php.net
a.moskalev@corp.badoo.com
Because currently CURLFile have this constructor:
public __construct http://php.net/manual/en/curlfile.construct.php (
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.
А.
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 написал:
On 23 Apr 2015, at 14:26, Alexander Moskalev <irker@irker.net
javascript:;> wrote:Because currently CURLFile have this constructor:
public __construct http://php.net/manual/en/curlfile.construct.php (
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.А.
--
With regards, Alexander Moskalev
irker@irker.net
irker@php.net
a.moskalev@corp.badoo.com
(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
2015-04-24 11:10 GMT+03:00 Alexander Moskalev irker@irker.net:
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