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

Restricted Assets Download via request-data mechanism not working #94

Closed
galum opened this issue Apr 30, 2019 · 7 comments
Closed

Restricted Assets Download via request-data mechanism not working #94

galum opened this issue Apr 30, 2019 · 7 comments
Labels
Milestone

Comments

@galum
Copy link

galum commented Apr 30, 2019

Q A
Bug report? yes

Asset Download via members/request-data/(token) mechanism seems not working any more (Pimcore 5.7.1, Members Bundle v2.3.0, PHP 7.2, Apache). Only the first few bytes of the requested files are download. The file is thus downloaded broken. The problem is most probably in how the response is created in RequestController / serveFile function:

        $response->setCallback(function () use ($asset) {
            flush();
            ob_flush();
            $handle = fopen(rawurldecode(PIMCORE_ASSET_DIRECTORY . $asset->getFullPath()), 'rb');
            while (!feof($handle)) {
                print(fread($handle, self::BUFFER_SIZE));
                flush();
                ob_flush();
            }
        });

Maybe the use of flush() is causing problems or maybe an other problem. In a quick test, I replaced the Response for download with Symfony's BinaryFileResponse and then it worked:

            $response = new BinaryFileResponse($asset->getFileSystemPath(), 200, [], false, null, true, true);
            $response->headers->set('Content-Type', $asset->getMimetype());
            $response->headers->set('Content-Length', $asset->getFileSize());
            $response->headers->set('Cache-Control', 'private');

The function serveZip also uses a similar mechanism and may also be effected.

EDIT: I just saw that in the current version in the repository the print function in this part has been replaced with the echo function. So maybe this problem has already been solved in the latest version.

EDIT2: In a similar situation, we used StreamedResponse in the way it is documented here:
https://medium.com/@matriphe/streaming-csv-using-php-46c717e33d87
Maybe writing to $handle=fopen('php://output', 'w'); would be better than using "print" or "echo". Flush ist not used there at all in the example.

@solverat
Copy link
Member

solverat commented May 8, 2019

Hey @galum,

i just tested the zip generation by using a simple twig call and it worked perfectly fine:

 {{ dump(members_generate_asset_package_url([11,12], true)) }} 

11 and 12 are asset ids (which must be available in the /restricted-assets assets folder.

Did you also tried the latest dev-master? If not, please do so. We also need some more information, if the problem still exists: total file size, file type, file amount.

Thanks!

@galum
Copy link
Author

galum commented May 9, 2019

@solverat

By trial and error, I figured out that the problem seems to be this:

$response->headers->set('Content-Length', $asset->getFileSize('noformatting'));

A generated string there is for example 78 KB but the browser will then only download the first 78 bytes!

I replaced it with this:

$response->headers->set('Content-Length', $asset->getFileSize(false));

Output here: 78001

And then it is working.

I assume Pimcore changed something in the getFileSize behaviour. The first argument is a boolean(!):

/**
* Get filesize
*
* @param bool $formatted
* @param int $precision
*
* @return string
*/
public function getFileSize($formatted = false, $precision = 2)

Edit 1:
The problem was not with the ZIP download but with the single file download (serveFile)!

@solverat
Copy link
Member

solverat commented May 9, 2019

@galum you're right... they really changed that. jeez...
I'm going to change this - big thanks for your investigation!

(for reference: pimcore/pimcore#3885)
(for reference II: implement tests #73)

@solverat solverat added the bug label May 9, 2019
@solverat solverat added this to the 2.4.0 milestone May 9, 2019
solverat added a commit that referenced this issue May 9, 2019
* do not use formatted file size in content-length header, see #94

* dont use pimcore file size check to keep BC
@solverat
Copy link
Member

solverat commented May 9, 2019

fixed

@solverat solverat closed this as completed May 9, 2019
@galum
Copy link
Author

galum commented May 9, 2019

@solverat

Thanks for your efforts!

@Sonja-Schubert
Copy link

@solverat and @galum Thank you very much for solving this!
@solverat Is there already a time schedule when 2.4.0 will be released? We use these restricted asset downloads a lot also in production env so it would be great to have a release to not use the master branch

@solverat
Copy link
Member

Hey @Sonja-Schubert, just released version 2.4.0! Enjoy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants