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

Downloads are broken because of semicolon appended to filename #13919

Closed
bbasinski opened this issue Mar 29, 2019 · 2 comments

Comments

@bbasinski
Copy link

@bbasinski bbasinski commented Mar 29, 2019

Changes from #13496 breaks our all downloads. Downloaded file has semicolon appended to the extension which disable file type recognition by system (which is important). Tested both linux and windows, chrome and firefox. Even RFC 6266 Content-Disposition in HTTP omits semicolon at the end of header. Browser does not strip this. Tried to wrap filename in double quotes but with no luck. Only solution is to overwrite Content-Disposition header to remove semicolon from the end.

$this->response->setFileToSend($fileDir, $filename);

//fix
$this->response->setHeader("Content-Disposition", "attachment; filename=$filename");

Details

  • Phalcon version: 3.4.2
  • PHP Version: 7.2.15
  • Operating System: Debian
  • Server: Apache
@niden niden added this to To do in 4.1 Release via automation May 19, 2019
@niden niden added the Bug - Medium label May 19, 2019
@niden niden added 4.0 4.1 and removed 4.1 4.0 labels Jun 21, 2019
@StudioMaX

This comment has been minimized.

Copy link
Contributor

@StudioMaX StudioMaX commented Aug 5, 2019

I always hated the Content-Disposition header. The current implementation in Phalcon is suitable for the simplest cases - when the filename is without any encodings and spaces, like filename.txt. For more complex file names each browser will react differently. Just look at this Test case: http://test.greenbytes.de/tech/tc2231/
Personally I use something like this:

if ($browserInfo['browser_name'] === 'Internet Explorer' && ($browserInfo['browser_version'] === '7.0' || $browserInfo['browser_version'] == '8.0')) {
    $filename = mb_convert_encoding($filename, 'UTF-8', 'Windows-1251');
    $contentDisposition = 'attachment; filename="' . rawurlencode($filename) . '"';
} elseif ($browserInfo['browser_name'] === 'Safari') {
    $filename = mb_convert_encoding($filename, 'UTF-8', 'Windows-1251');
    $contentDisposition = 'attachment; filename="' . $filename . '"';
} else {
    $filename = mb_convert_encoding($filename, 'UTF-8', 'Windows-1251');
    $contentDisposition = 'attachment; filename="' . $filename . '"; filename*=UTF-8\'\'' . rawurlencode($filename);
}

$this->response->setHeader("Content-Disposition", $contentDisposition);

Fortunately, Internet Explorer 7.0 and 8.0 are already dead, but Safari can still be a problem.

And yes, in any of these cases you don't need a semicolon at the end, but the filename must be enclosed in quotes.

sergeyklay added a commit that referenced this issue Aug 30, 2019
Fixes #13919, make response->setFileToSend support non-ASCII filename.
@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Aug 30, 2019

Fixed in the 4.0.x branch. Feel free to open a new issue if the problem appears again. Thank you for the bug report.

@sergeyklay sergeyklay closed this Aug 30, 2019
4.1 Release automation moved this from To do to Done Aug 30, 2019
@ian4hu ian4hu referenced this issue Sep 1, 2019
2 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4.1 Release
  
Done
4 participants
You can’t perform that action at this time.