Skip to content
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

Upload progress + PHP 7.2 #6177

Closed
fesarlis opened this issue Feb 14, 2018 · 11 comments
Closed

Upload progress + PHP 7.2 #6177

fesarlis opened this issue Feb 14, 2018 · 11 comments

Comments

@fesarlis
Copy link

fesarlis commented Feb 14, 2018

Hello.
What is the current way to enable upload progress in RC+PHP 7.2; uploadprogress pecl extension does not compile anymore. Found one that claims it works (https://github.com/Jan-E/uploadprogress), it compiles fine but RC does not attach files, it produces an error.
RC = 1.3.2

Thanks

@alecpl
Copy link
Member

alecpl commented Feb 15, 2018

What error? Roundcube supports also apc.rfc1867 (with apc.rfc1867_name) or session.upload_progress.enabled (with session.upload_progress.name), but I didn't test these recently.

@fesarlis
Copy link
Author

fesarlis commented Feb 16, 2018

Error states that file could not be attached (red text popup).
Did not try apc.rfc1867 since apc is now obsolete. Error appears when trying to compile pecl extension under PHP 7.2. As I said there is one patched version (link supplied above) that compiles but doesn't work. Tried to toggle session.upload_progress.name with no luck (I've never had to use it with PHP 5.6 though). I paste part of the error when trying to compile uploadprogress in PHP 7.2:

cc -I. -I/tmp/pear/temp/uploadprogress -DPHP_ATOM_INC -I/tmp/pear/temp/pear-build-rootddPNtt/uploadprogress-1.0.3.1/include -I/tmp/pear/temp/pear-build-rootddPNtt/uploadprogress-1.0.3.1/main -I/tmp/pear/temp/uploadprogress -I/opt/servers/php/include/php -I/opt/servers/php/include/php/main -I/opt/servers/php/include/php/TSRM -I/opt/servers/php/include/php/Zend -I/opt/servers/php/include/php/ext -I/opt/servers/php/include/php/ext/date/lib -DHAVE_CONFIG_H -g -O2 -c /tmp/pear/temp/uploadprogress/uploadprogress.c -fPIC -DPIC -o .libs/uploadprogress.o /tmp/pear/temp/uploadprogress/uploadprogress.c: In function ‘uploadprogress_php_rfc1867_file’: /tmp/pear/temp/uploadprogress/uploadprogress.c:160:31: error: ‘ENFORCE_SAFE_MODE’ undeclared (first use in this function) int options = ENFORCE_SAFE_MODE; ^ /tmp/pear/temp/uploadprogress/uploadprogress.c:160:31: note: each undeclared identifier is reported only once for each function it appears in /tmp/pear/temp/uploadprogress/uploadprogress.c: In function ‘uploadprogress_file_php_get_info’: /tmp/pear/temp/uploadprogress/uploadprogress.c:420:57: error: macro "add_assoc_string" passed 4 arguments, but takes just 3 add_assoc_string( return_value, k, v, 1 ); ^ /tmp/pear/temp/uploadprogress/uploadprogress.c:420:17: error: ‘add_assoc_string’ undeclared (first use in this function) add_assoc_string( return_value, k, v, 1 ); ^ /tmp/pear/temp/uploadprogress/uploadprogress.c: In function ‘uploadprogress_file_php_get_contents’: /tmp/pear/temp/uploadprogress/uploadprogress.c:437:19: error: ‘ENFORCE_SAFE_MODE’ undeclared (first use in this function) int options = ENFORCE_SAFE_MODE; ^ /tmp/pear/temp/uploadprogress/uploadprogress.c:458:71: error: macro "php_stream_copy_to_mem" passed 4 arguments, but takes just 3 if ((len = php_stream_copy_to_mem(stream, &contents, maxlen, 0)) > 0) { ^ /tmp/pear/temp/uploadprogress/uploadprogress.c:458:20: error: ‘php_stream_copy_to_mem’ undeclared (first use in this function) if ((len = php_stream_copy_to_mem(stream, &contents, maxlen, 0)) > 0) { ^ /tmp/pear/temp/uploadprogress/uploadprogress.c:466:44: error: macro "RETVAL_STRINGL" passed 3 arguments, but takes just 2 RETVAL_STRINGL(contents, len, 0); ^ /tmp/pear/temp/uploadprogress/uploadprogress.c:466:13: error: ‘RETVAL_STRINGL’ undeclared (first use in this function) RETVAL_STRINGL(contents, len, 0); ^ make: *** [uploadprogress.lo] Error 1

Therefore it is my understanding that RC must now find a different way to support upload progress.

@alecpl
Copy link
Member

alecpl commented Feb 16, 2018

I think apc.rfc1867 should work with APCu extension.

@alecpl
Copy link
Member

alecpl commented Feb 16, 2018

Session handler does not work for me. It also looks that APCu 5.x does not support apc.rfc1867. I found some hints on https://www.drupal.org/project/drupal/issues/2718253. There's php-uploadprogress package on ubuntu that is supposed to work with PHP 7.2, but does not work for me for unknown reason.

@alecpl alecpl added this to the later milestone Feb 16, 2018
@alecpl
Copy link
Member

alecpl commented Jan 1, 2019

In https://bugs.php.net/bug.php?id=74131 the problem is confirmed and suggested that the only option is to use HTML5 ajax progress event. This might be not that bad idea. However, it does not work with all browsers and implementation will take time. We probably will need a fallback to old upload method, can be with disabled progress.

@alecpl alecpl modified the milestones: later, 1.5-beta Jan 1, 2019
alecpl added a commit to alecpl/roundcubemail that referenced this issue Jan 10, 2019
Replaced all old upload progress code in favour of ajax upload progress.
Instead of posting a hidden iframe, we now use AJAX (as we did for drag-n-drop).
Removed code for old browsers. Now we support IE >= 10, Firefox > 4.
Upload progress may not work in some more, but support is quite good.
@alecpl
Copy link
Member

alecpl commented Jan 10, 2019

Proposed solution in #6583. @thomascube what do you think? how about merging it for 1.4?

@thomascube
Copy link
Member

@alecpl Looks good. The new approach includes apc.rfc1867 and thus should also work on PHP 5.x versions, right? 1.4 RC packages are almost ready but we could still include this.

@alecpl
Copy link
Member

alecpl commented Jan 10, 2019

The new approach removes all apc/session/uploadprogress related code. We'll be HTML5-only from now on.

@alecpl
Copy link
Member

alecpl commented Jan 10, 2019

To clarify, by HTML5-only I mean upload progress is javascript-client-side thing. Also much better for performance.

@ivnish
Copy link

ivnish commented Jan 24, 2019

I use debian repo https://packages.sury.org/php/ it has php 7.1, 7.2, 7.3 and package php-uploadprogress for all these versions

I think it's not roundcube problem

alecpl added a commit that referenced this issue Feb 3, 2019
Replaced all old upload progress code in favour of ajax upload progress.
Instead of posting a hidden iframe, we now use AJAX (as we did for drag-n-drop).
Removed code for old browsers. Now we support IE >= 10, Firefox > 4.
Upload progress may not work in some more, but support is quite good.
@alecpl alecpl modified the milestones: 1.5-beta, 1.4-rc Feb 3, 2019
@alecpl
Copy link
Member

alecpl commented Feb 3, 2019

HTML5 solution merged into master.

@alecpl alecpl closed this as completed Feb 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants