Everything

php

Upload Progress Meter Patch

php.internals

Klaus Reimer

21 years ago

Hello,

The website "http://pdoru.from.ro/" states that "hopefully the patch
will be integrated into PHP". This patch adds some hooks to
main/rfc1867.c so PHP modules can implement stuff like reading the
current state of a file upload. Such an external PHP module can write
the state to disk or shared memory or whatever. A HTML popup shown to
the uploader can then view this data as a progress meter bar to the user
while he uploads large files.

This patch is extremely useful and is working very well. I'm already
using it (together with a self-written PHP module) quite successfully in
some projects where users can upload large image files.

What is the state of considering the integration of this patch into PHP?
Or is the statement on the webpage wrong and you have never heard of it?

I have attached a version of the patch for PHP 5.0.2. Even it you don't
like the way it is written it can still be used as an example to show
how it could be done.

--
Bye, K <http://www.ailis.de/~k/> (FidoNet: 2:240/2188.18)
[A735 47EC D87B 1F15 C1E9  53D3 AA03 6173 A723 E391]
(Finger k@ailis.de to get public key)

Marcus Börger

21 years ago

Hello Klaus,

we've haered of it in the past but it was too late to add for 4.3
series and obviously nobody cared to look for it for 5.0. So no the
next version we could add it to would be 5.1. In general the idea
is nice but it should also allow for script callbacks:

function upload_callback()
{
// some SQL or SESSION functions...
}
register_upload_callback('upload_callback');

// upload code

Shouldn't that work too?

Apart from that the callback type and the c-level register functions
must be declared in the corresponding .h file of course. And last but
not least we cannot add this code because it is not thread safe,
meaning the callback pointer must not be a static pointer but a global
module variable instead especially the info for the script callbacks
i asked for.

regards
marcus

Friday, November 5, 2004, 10:34:01 AM, you wrote:

> Hello,

> The website "http://pdoru.from.ro/" states that "hopefully the patch
> will be integrated into PHP". This patch adds some hooks to
> main/rfc1867.c so PHP modules can implement stuff like reading the
> current state of a file upload. Such an external PHP module can write
> the state to disk or shared memory or whatever. A HTML popup shown to
> the uploader can then view this data as a progress meter bar to the user
> while he uploads large files.

> This patch is extremely useful and is working very well. I'm already
> using it (together with a self-written PHP module) quite successfully in
> some projects where users can upload large image files.

> What is the state of considering the integration of this patch into PHP?
> Or is the statement on the webpage wrong and you have never heard of it?

> I have attached a version of the patch for PHP 5.0.2. Even it you don't
> like the way it is written it can still be used as an example to show
> how it could be done.

--
Best regards,
Marcus                            mailto:helly@php.net

Klaus Reimer

21 years ago

Marcus Boerger wrote:

>   we've haered of it in the past but it was too late to add for 4.3
> series and obviously nobody cared to look for it for 5.0. So no the
> next version we could add it to would be 5.1. In general the idea
> is nice but it should also allow for script callbacks:

This is possible? Up to now I thought that the retrieving and processing
of POST data is done BEFORE the PHP script is loaded. If I'm wrong here
it definetly would be a cool thing to have script callbacks for this.
And another neat thing would be to have access to the PHP session inside
such a callback to store the upload progress data in it. But I think
that's difficult because the session id is not known at this early moment.

Derick Rethans

21 years ago

On Fri, 5 Nov 2004, Klaus Reimer wrote:

> Marcus Boerger wrote:
> >   we've haered of it in the past but it was too late to add for 4.3
> > series and obviously nobody cared to look for it for 5.0. So no the
> > next version we could add it to would be 5.1. In general the idea
> > is nice but it should also allow for script callbacks:
>
> This is possible? Up to now I thought that the retrieving and processing
> of POST data is done BEFORE the PHP script is loaded. If I'm wrong here
> it definetly would be a cool thing to have script callbacks for this.

You are not wrong, POST/GET data is parsed before the PHP scripts are
run. I also think that functionality like this should not be part of PHP
itself. It is a user interface problem, so it should be done there.

> And another neat thing would be to have access to the PHP session inside
> such a callback to store the upload progress data in it. But I think
> that's difficult because the session id is not known at this early moment.

I am not sure if the modules are loaded/activated before the request
variables are parsed, I would guess they are though.

Derick

--
Derick Rethans
http://derickrethans.nl | http://ez.no | http://xdebug.org

Joe Orton

21 years ago

On Fri, Nov 05, 2004 at 11:07:53AM +0100, Derick Rethans wrote:

> On Fri, 5 Nov 2004, Klaus Reimer wrote:
>
> > Marcus Boerger wrote:
> > >   we've haered of it in the past but it was too late to add for 4.3
> > > series and obviously nobody cared to look for it for 5.0. So no the
> > > next version we could add it to would be 5.1. In general the idea
> > > is nice but it should also allow for script callbacks:
> >
> > This is possible? Up to now I thought that the retrieving and processing
> > of POST data is done BEFORE the PHP script is loaded. If I'm wrong here
> > it definetly would be a cool thing to have script callbacks for this.
>
> You are not wrong, POST/GET data is parsed before the PHP scripts are
> run. I also think that functionality like this should not be part of PHP
> itself. It is a user interface problem, so it should be done there.

It's really the only place it can be done safely: sending an HTTP
response before the request body has been entirely read is fundamentally
risky, too: HTTP clients are not required to read response data
concurrently to sending request bodies, so sending the upload meter
stuff can fill the TCP send buffer server-side and deadlock the
connection for a compliant HTTP client.

joe

Curt Zirzow

21 years ago

* Thus wrote Joe Orton:

> On Fri, Nov 05, 2004 at 11:07:53AM +0100, Derick Rethans wrote:
> > On Fri, 5 Nov 2004, Klaus Reimer wrote:
> >
> > > Marcus Boerger wrote:
> > >
> > > This is possible? Up to now I thought that the retrieving and processing
> > > of POST data is done BEFORE the PHP script is loaded. If I'm wrong here
> > > it definetly would be a cool thing to have script callbacks for this.
> >
> > You are not wrong, POST/GET data is parsed before the PHP scripts are
> > run. I also think that functionality like this should not be part of PHP
> > itself. It is a user interface problem, so it should be done there.
>
> It's really the only place it can be done safely: sending an HTTP
> response before the request body has been entirely read is fundamentally
> risky, too: HTTP clients are not required to read response data
> concurrently to sending request bodies, so sending the upload meter
> stuff can fill the TCP send buffer server-side and deadlock the
> connection for a compliant HTTP client.

I've been struggling with this issue, I havn't found any solid
standard on HTTP communication.  I've tested sending data without
reading all of the input with apache and IE/firefox/opera, with
success, but it still worries me that nothing is set as a standard.


Curt

--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Klaus Reimer

21 years ago

Joe Orton wrote:

>> It is a user interface problem, so it should be done there.
> It's really the only place it can be done safely

Web browsers don't offer any API to allow a client-side progress bar.
The only client-side solution I know of is ActiveX and Java and these
solutions are not available to all users.

> sending an HTTP
> response before the request body has been entirely read is fundamentally
> risky, too: HTTP clients are not required to read response data
> concurrently to sending request bodies, so sending the upload meter
> stuff can fill the TCP send buffer server-side and deadlock the
> connection for a compliant HTTP client.

Sending the upload meter stuff to the client while the client still
uploads? In the same request? I have the bad feeling that the idea
behind the progress meter bar was not fully understood. It's working
like this:

1. Client uploads files. PHP (with the patch I posted) calls callback
functions of a PHP extension to inform the PHP extension about the
status of the upload.

2. The PHP extension writes the upload status to a file or to a database
or to shared memory or whatever.

3. When the client starts uploading the files it also opens a javascript
popup window which loads a second page (which is totally separated from
the upload). It passes an upload identifier to the PHP script so the PHP
script can ask the PHP extension about the status of this specific
upload. The PHP script then displays this status in the javascript popup
which is reloaded regularly. Instead of reloading the page it can also
be done with a permanent stream of Javascript commands (like Sascha
Schumann's chat module IRCG) but how it is done does not matter.

4. When the upload is complete then the javascript popup ist closed.


So ne need to send a response while the request is still read. The whole
scenario can also work without Javascript if frames/iframes are used to
separate the upload from the progress bar. But even this does not
matter. It's all up to the user how it's implemented. All the dirty work
can be done in an external PHP Extension and in PHP code. Only this
little patch I sent a few days ago is needed in PHP itself.

Curt Zirzow

21 years ago

* Thus wrote Klaus Reimer:

> Joe Orton wrote:
> >>It is a user interface problem, so it should be done there.
> >It's really the only place it can be done safely
>
> Web browsers don't offer any API to allow a client-side progress bar.
> The only client-side solution I know of is ActiveX and Java and these
> solutions are not available to all users.

Actually, the little blue bar in the lower right corner is a
standard place for the progress bar. Unfortantly It isn't as
'friendly'.

> >sending an HTTP
> >response before the request body has been entirely read is fundamentally
> >risky, too: HTTP clients are not required to read response data
> >concurrently to sending request bodies, so sending the upload meter
> >stuff can fill the TCP send buffer server-side and deadlock the
> >connection for a compliant HTTP client.
>
> Sending the upload meter stuff to the client while the client still
> uploads? In the same request? I have the bad feeling that the idea
> behind the progress meter bar was not fully understood. It's working
> like this:

I'm aware of how the patch you originally proposed works. The
issue I see is if we're making changes to how uploads work within
php, It would be nice to be able to, easily, do other things like:

- authenticate a user before accepting a download
- stream the download through an encryption algorithm.

>
> So ne need to send a response while the request is still read. The whole
> scenario can also work without Javascript if frames/iframes are used to
> separate the upload from the progress bar. But even this does not
> matter. It's all up to the user how it's implemented. All the dirty work
> can be done in an external PHP Extension and in PHP code. Only this
> little patch I sent a few days ago is needed in PHP itself.

You can provide a progress bar, as php stands right now, without any
patch.


Curt

--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Klaus Reimer

21 years ago

Curt Zirzow wrote:

> You can provide a progress bar, as php stands right now, without any
> patch.

Ok, then tell me how, please. Is it documented somewhere?

ilya77@gmail.com

21 years ago

Actually, it's displaying a progress bar during upload is only one
part of the problem.

The major problems with HTTP uploads are introduced with large files
(which the progress bar is needed for, anyway, otherwise there's not
much sense to it) and bad or loosy connections (which most of the home
connections are).
The major problem is resuming an interrupted upload back where it
started, and pure PHP solution won't help here...

On Wed, 10 Nov 2004 15:54:26 +0000, Curt Zirzow <curt@php.net> wrote:

Marcus Börger

21 years ago

Hello Klaus,

Friday, November 5, 2004, 11:03:34 AM, you wrote:

> Marcus Boerger wrote:
>>   we've haered of it in the past but it was too late to add for 4.3
>> series and obviously nobody cared to look for it for 5.0. So no the
>> next version we could add it to would be 5.1. In general the idea
>> is nice but it should also allow for script callbacks:

> This is possible? Up to now I thought that the retrieving and processing
> of POST data is done BEFORE the PHP script is loaded. If I'm wrong here
> it definetly would be a cool thing to have script callbacks for this.
> And another neat thing would be to have access to the PHP session inside
> such a callback to store the upload progress data in it. But I think
> that's difficult because the session id is not known at this early moment.

Thinking about it another time you're right. So what the patch offers is
all we can do. However having the session id available somehow would make
the the patch usefull. Perhaps it is enough to pass the url which should
be available at that time so an extension can find out the session id
throug it. Maybe it would be best to simply pass the sapi_globals_struct.

Best regards,
Marcus                            mailto:helly@php.net

Klaus Reimer

21 years ago

Marcus Boerger wrote:

> Thinking about it another time you're right. So what the patch offers is
> all we can do. However having the session id available somehow would make
> the the patch usefull.

I have some ideas for this. It may even be possible to remove the
UPLOAD_IDENTIFIER-stuff from the patch and move it to the PHP extension.
This would make the patch more independant. The author of the PHP
extension can decide how this variable is named and how it is transmitted.

If nobody has already a working solution I can give it a try this weekend.

But one question stays: Let's say that the PHP extension now knows the
PHP session ID during progress tracking (because it was passed like the
UPLOAD_IDENTIFIER via a POST variable BEFORE the uploaded files). What
can the extension do with it? Is it possible for this PHP extension to
open the session, update variables in it, and close it again? Or is the
stage in which the callback is called to early for doing so?

Marcus Börger

21 years ago

Hello Klaus,

Friday, November 5, 2004, 11:42:49 AM, you wrote:

> Marcus Boerger wrote:
>> Thinking about it another time you're right. So what the patch offers is
>> all we can do. However having the session id available somehow would make
>> the the patch usefull.

> I have some ideas for this. It may even be possible to remove the
> UPLOAD_IDENTIFIER-stuff from the patch and move it to the PHP extension.
> This would make the patch more independant. The author of the PHP
> extension can decide how this variable is named and how it is transmitted.

> If nobody has already a working solution I can give it a try this weekend.

> But one question stays: Let's say that the PHP extension now knows the
> PHP session ID during progress tracking (because it was passed like the
> UPLOAD_IDENTIFIER via a POST variable BEFORE the uploaded files). What
> can the extension do with it? Is it possible for this PHP extension to
> open the session, update variables in it, and close it again? Or is the
> stage in which the callback is called to early for doing so?

Doesn't matter. If it is to late you simply do it with some SQL.

--
Best regards,
Marcus                            mailto:helly@php.net

Curt Zirzow

21 years ago

* Thus wrote Klaus Reimer:

> Marcus Boerger wrote:
> >Thinking about it another time you're right. So what the patch offers is
> >all we can do. However having the session id available somehow would make
> >the the patch usefull.
>
> I have some ideas for this. It may even be possible to remove the
> UPLOAD_IDENTIFIER-stuff from the patch and move it to the PHP extension.
> This would make the patch more independant. The author of the PHP
> extension can decide how this variable is named and how it is transmitted.
>
> If nobody has already a working solution I can give it a try this weekend.

I have a pecl extenstion started that provides this functionality.
It needs some more testing and a patch so php doesn't parse the
data.

I'll put it up into pecl's cvs later today.


Curt

--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Klaus Reimer

21 years ago

Curt Zirzow wrote:

> I have a pecl extenstion started that provides this functionality.
> It needs some more testing and a patch so php doesn't parse the
> data.

And this extension is working WITHOUT callbacks in main/rfc1867.c?

Curt Zirzow

21 years ago

* Thus wrote Klaus Reimer:

> Curt Zirzow wrote:
> >I have a pecl extenstion started that provides this functionality.
> >It needs some more testing and a patch so php doesn't parse the
> >data.
>
> And this extension is working WITHOUT callbacks in main/rfc1867.c?

correct. The only thing that needs to go into the core of php is a
new php.ini option to disable parsing the input.

When this is the case the user can either parse the data via
php://stdin or by using an extension.


Curt

--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Klaus Reimer

21 years ago

Curt Zirzow wrote:

>>And this extension is working WITHOUT callbacks in main/rfc1867.c?
> correct. The only thing that needs to go into the core of php is a
> new php.ini option to disable parsing the input.

Ok, then you can do with PHP the same as I have done with a CGI program
before I found the mentioned patch. But then you have to do all the
multipart/form-data parsing yourself. And you have to put all the
uploaded data into the $_FILES array to stay compatible with PHP's file
receiver. So you have to rewrite all the stuff that PHP is already doing
very good in your PECL extension.

Implementing some callbacks in rfc1867.c sounds better to me. Then a
PECL extension can focus on the progress informations and let PHP handle
the data.

Curt Zirzow

21 years ago

* Thus wrote Klaus Reimer:

> Curt Zirzow wrote:
> >>And this extension is working WITHOUT callbacks in main/rfc1867.c?
> >correct. The only thing that needs to go into the core of php is a
> >new php.ini option to disable parsing the input.

One thing to add is that I surely dont think its php's
responsibilty to provide a way to show progress of an uploaded
file.  The client that is uploading the file knows better and
should be the one with the responsibilty.

The main benefit I see having php, either using callbacks or some
extension, is the ability to authenticate a user (or some logical
decision) before actually downloading a file.

>
> Ok, then you can do with PHP the same as I have done with a CGI program
> before I found the mentioned patch. But then you have to do all the
> multipart/form-data parsing yourself. And you have to put all the
> uploaded data into the $_FILES array to stay compatible with PHP's file
> receiver. So you have to rewrite all the stuff that PHP is already doing
> very good in your PECL extension.

The extensions' main responsibilty is to parse the
multipart/form-data.  The way my extension works now is something
like:

$fp = fopen('php://stdin', 'r');

$parser = postparser_init($fp);

while ($token = postparser_token($parser) ) {
switch ($token['type']) {

case 1: // POST VAR
//...
$_POST[$token['name']] = stream_get_contents($parser);
break;

case 2: // FILE
while (! feof($parser)) {
$buf = fread($parser, $size);
}
}
}

>
> Implementing some callbacks in rfc1867.c sounds better to me. Then a
> PECL extension can focus on the progress informations and let PHP handle
>  the data.

After some thought, it might be even better just to implement a
filter. So  postparser_token could be replaced with
stream_get_meta_data(). The extension works much like a filter
already, anyway.

If we add a ini option for php not to read the stdin on a multipart
post, and rework some of  rfc1867.c functions to be accessable from
outside of rfc1867.c's namespace, a custom filter could easily be
written and not have to duplicate the current code.


Curt

--
First, let me assure you that this is not one of those shady pyramid schemes
you've been hearing about.  No, sir.  Our model is the trapezoid!

Tom Rogers

21 years ago

Hi,

Friday, November 5, 2004, 8:42:49 PM, you wrote:
KR> Marcus Boerger wrote:

>> Thinking about it another time you're right. So what the patch offers is
>> all we can do. However having the session id available somehow would make
>> the the patch usefull.

KR> I have some ideas for this. It may even be possible to remove the
KR> UPLOAD_IDENTIFIER-stuff from the patch and move it to the PHP extension.
KR> This would make the patch more independant. The author of the PHP
KR> extension can decide how this variable is named and how it is transmitted.

KR> If nobody has already a working solution I can give it a try this weekend.

KR> But one question stays: Let's say that the PHP extension now knows the
KR> PHP session ID during progress tracking (because it was passed like the
KR> UPLOAD_IDENTIFIER via a POST variable BEFORE the uploaded files). What
KR> can the extension do with it? Is it possible for this PHP extension to
KR> open the session, update variables in it, and close it again? Or is the
KR> stage in which the callback is called to early for doing so?

here are some changes I made to make a thread safe file upload. The diffs are
against 4.3.8 but should be close enough. I am not an expert by any means but it
may help

--
regards,
Tom

Marcus Börger

21 years ago

Hello Tom,

can you please provide a unified diff (diff -u) which everyone here
understands? Also it would help to see a diff against HEAD.

regards
marcus

Saturday, November 6, 2004, 1:16:33 AM, you wrote:

> Hi,

> Friday, November 5, 2004, 8:42:49 PM, you wrote:

KR>> Marcus Boerger wrote:

>>> Thinking about it another time you're right. So what the patch offers is
>>> all we can do. However having the session id available somehow would make
>>> the the patch usefull.

KR>> I have some ideas for this. It may even be possible to remove the
KR>> UPLOAD_IDENTIFIER-stuff from the patch and move it to the PHP extension.
KR>> This would make the patch more independant. The author of the PHP
KR>> extension can decide how this variable is named and how it is transmitted.

KR>> If nobody has already a working solution I can give it a try this weekend.

KR>> But one question stays: Let's say that the PHP extension now knows the
KR>> PHP session ID during progress tracking (because it was passed like the
KR>> UPLOAD_IDENTIFIER via a POST variable BEFORE the uploaded files). What
KR>> can the extension do with it? Is it possible for this PHP extension to
KR>> open the session, update variables in it, and close it again? Or is the
KR>> stage in which the callback is called to early for doing so?

> here are some changes I made to make a thread safe file upload. The diffs are
> against 4.3.8 but should be close enough. I am not an expert by any means but it
> may help

--
Best regards,
Marcus                            mailto:helly@php.net

Tom Rogers

21 years ago

Hi,

Saturday, November 6, 2004, 10:25:57 AM, you wrote:
MB> Hello Tom,

MB>   can you please provide a unified diff (diff -u) which everyone here
MB>   understands? Also it would help to see a diff against HEAD.

MB> regards
MB> marcus

This is against the latest cvs 4.3

--
regards,
Tom

Wez Furlong

21 years ago

It needs to be made against HEAD (eg: 5.1 CVS) as it won't be going
into any release branch.

--Wez.

On Sat, 6 Nov 2004 13:55:21 +1000, Tom Rogers <trogers@kwikin.com> wrote:

Andi Gutmans

21 years ago

Without commenting on the main part of the patch (I still haven't come to a
conlusion either way), I don't see why you had to make any changes to the
Zend Engine. Shouldn't those globals be declared outside the engine? (i.e.
php5/main?).

Andi

At 01:55 PM 11/6/2004 +1000, Tom Rogers wrote: