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

Fix #79584: Segmentation fault in uploadprogress 1.1.0 and up #9

Merged
merged 3 commits into from
Sep 28, 2021

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Apr 30, 2021

This patch has been provided by @oerdnj and attached to the respective bug almost a year ago. I'm merely forwarding to hopefully increase visibility.

@cmb69
Copy link
Member Author

cmb69 commented Apr 30, 2021

The test failures are caused by Xdebug 3 now being used; should probably be disabled for Travis CI.

@williamdes
Copy link

Would it fix https://bugs.php.net/bug.php?id=80979 ?

@cmb69
Copy link
Member Author

cmb69 commented Apr 30, 2021

Would it fix https://bugs.php.net/bug.php?id=80979 ?

Possible, but I don't think so, since that bug apparently doesn't affect PHP 8 (and this issue should affect any PHP version).

@williamdes
Copy link

Hello @ramsey
Does this PR look good?

@cmb69 do you think you could add a php test using my test script from the php bug please or should I open a PR to add it and see it fail before the fix?

@cmb69
Copy link
Member Author

cmb69 commented May 7, 2021

@williamdes, I think it is a good idea to have the test as separate PR first.

@williamdes
Copy link

@williamdes, I think it is a good idea to have the test as separate PR first.

I just tried but the extension seems not to give me information

--TEST--
Upload with enctype="multipart/form-data" (bug #80979)

--SKIPIF--
<?php
if (!extension_loaded('uploadprogress')) exit('skip extension not loaded');

--POST_RAW--
Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryoJPNsLDEoFMAuRiZ
------WebKitFormBoundaryoJPNsLDEoFMAuRiZ
Content-Disposition: form-data; name="uf"; filename="williamdes-optimized.svg"
Content-Type: image/svg+xml

<?xml version="1.0" encoding="UTF-8"?>
<svg width="64" height="64" version="1.1" viewBox="0 0 64 64" xmlns="http://www.w3.org/2000/svg">
<g transform="translate(.0021289 -.0039062)">
<path d="m0 32v-32h64v64h-64v-32zm16 8v-8h-8v16h8v-8zm40 0v-8h-8v16h8v-8z" fill="#f3c089"/>
<path d="m0 24v-24h64v48h-8v-16h-8v16h-8v-16h-16v16h-8v-16h-8v16h-8v-24z" fill="#00538b"/>
<path d="m0 24v-24h64v48h-8v-16h-48v16h-8v-24z"/>
</g>
</svg>

------WebKitFormBoundaryoJPNsLDEoFMAuRiZ
Content-Disposition: form-data; name="UPLOAD_IDENTIFIER"

0c6c62b0c2cbc01b6599151de4584bff
------WebKitFormBoundaryoJPNsLDEoFMAuRiZ
Content-Disposition: form-data; name="Submit"

Envoyer
------WebKitFormBoundaryoJPNsLDEoFMAuRiZ--

--FILE--
<?php
var_dump($_POST['UPLOAD_IDENTIFIER']);
var_dump($_FILES['uf']['size']);
var_dump($_FILES['uf']['name']);
$info = uploadprogress_get_info('0c6c62b0c2cbc01b6599151de4584bff');
echo $info['filename'];
?>
<html>
    <form id="import_file_form" method="post" enctype="multipart/form-data" name="import" class="ajax" >
        <input type="file" name="uf">
        <input type="hidden" name="UPLOAD_IDENTIFIER" value="0c6c62b0c2cbc01b6599151de4584bff">
        <input type="submit" name="Submit">
    </form>
</html>
--EXPECTF--
string(32) "0c6c62b0c2cbc01b6599151de4584bff"
int(428)
string(24) "williamdes-optimized.svg"

<html>
    <form id="import_file_form" method="post" enctype="multipart/form-data" name="import" class="ajax" >
        <input type="file" name="uf">
        <input type="hidden" name="UPLOAD_IDENTIFIER" value="0c6c62b0c2cbc01b6599151de4584bff">
        <input type="submit" name="Submit">
    </form>
</html>
========DIFF========
     string(32) "0c6c62b0c2cbc01b6599151de4584bff"
     int(428)
     string(24) "williamdes-optimized.svg"
005+ Notice: Trying to access array offset on value of type null in /mnt/Dev/@php/uploadprogress/tests/bug80979.php on line 6
     <html>
         <form id="import_file_form" method="post" enctype="multipart/form-data" name="import" class="ajax" >
             <input type="file" name="uf">
--
========DONE========
FAIL Upload with enctype="multipart/form-data" (bug #80979) [tests/bug80979.phpt] 

If you can help me find out if I reproduced the issue or not that would help. My code seems valid but uploadprogress_get_inforeturns null.

@kid4git
Copy link

kid4git commented May 27, 2021

I can confirm this bug and I'm happy that the patch fixes the issue for me. This is on PHP 7.4.19 and Drupal 7.80 with uploadprogress 1.1.3.

@andypost
Copy link
Contributor

It will need to copy the fix to #5

@xTimeFor
Copy link

Thank you so so much for this patch. I had been tearing my hair out for hours on this segmentation issue tonight. Working on a new Drupal 9 install trying to get upload progress working. Everything said it should be working but wasn't. I was convinced it was something to do with mod_php not running correctly as I had previously been using php-fpm. Then I found this post. This is way outside my skillset but gave it try.

Downloaded 1.1.3 to my user's home directory, replaced uploadprogress.c with the patched version, and ran:

$ phpize
$ ./configure
$ make
$ make test
$ sudo make install

and... holy smokes it worked. Not just compiled but uploadprogress fully works. For someone who's never compiled any extensions before, whole new world. Very exciting. Really hope this can get merged and released soon. Uploadprogress was completely broken for me without this running Drupal 9.2, PHP 7.4, Amazon Linux AMI 2

@duhvir
Copy link

duhvir commented Sep 27, 2021

Helped for me. Can't understand why still not commited

@ramsey ramsey merged commit 26987fa into php:master Sep 28, 2021
@ramsey
Copy link
Member

ramsey commented Sep 28, 2021

Can't understand why still not commited

Time constraints. Sorry. I'll have a release out very soon. 😄

@cmb69 cmb69 deleted the cmb/79584 branch September 28, 2021 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants