Fixes #13919, make response->setFileToSend support non-ASCII filename.

[ian4hu]

• see https://bugs.php.net/bug.php?id=37738
• see https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21File%21FileSystem.php/function/FileSystem%3A%3Abasename/8.2.x
• see https://tools.ietf.org/html/rfc2231#section-5

Hello!

• Type: bug fix
• Link to issue: Downloads are broken because of semicolon appended to filename #13919

In raising this pull request, I confirm the following:

• I have read and understood the Contributing Guidelines
• I have checked that another pull request for this purpose does not exist
• I wrote some tests for this PR
• I have updated the relevant CHANGELOG
• I have created a PR for the documentation about this change

Small description of change:
Add non-ASCII support basename() by Phalcon\Helper\Fs and fixed the #13919

Thanks

[ruudboon]

Thank you for the pull. We're not accepting pulls on the master. Can you sent into the 4.0.x branch and would it be possible to add some unit tests?

[ian4hu]

Thank you for the pull. We're not accepting pulls on the master. Can you sent into the 4.0.x branch and would it be possible to add some unit tests?

I am not sure about how to make a right unit test, because the it only affect the response header.
The Phalcon\Helper\Fs->basename could be unit tested, and I will try to make the unit tests.

[codecov]

Codecov Report

Merging #14333 into 4.0.x will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##            4.0.x   #14333      +/-   ##
==========================================
- Coverage   66.31%   66.31%   -0.01%     
==========================================
  Files         488      488              
  Lines      115910   115927      +17     
==========================================
+ Hits        76862    76873      +11     
- Misses      39048    39054       +6

[ruudboon]

Can you run phpcs to fix the code style? See failing test
https://travis-ci.org/phalcon/cphalcon/jobs/577361625

[ian4hu]

Can you run phpcs to fix the code style? See failing test
https://travis-ci.org/phalcon/cphalcon/jobs/577361625

sorry for the code style issue. I don't have a fullly configured develop environment and missed the code style check.

[StudioMaX]

Take note that the semicolon at the end of the header is still not allowed according to http://test.greenbytes.de/tech/tc2231/#attwithasciifilenamenqs.

The trailing semicolon makes the header field value syntactically incorrect, as no other parameter follows. Thus the header field should be ignored.

[StudioMaX]

@ian4hu could you test these changes with the all common browsers with ASCII/non-ASCII filenames?

I may be wrong, but the file name should still be in double quotes, as the file name may contain spaces (see http://test.greenbytes.de/tech/tc2231/#attwithasciifilenamenqws).

Also, filename in filename= shouldn't be urlencoded, but filename*=UTF-8'' should (see http://test.greenbytes.de/tech/tc2231/#attwithfnrawpctenca).

Maybe something like this:

if basePathEncoding != "ASCII" {
    this->setRawHeader("Content-Disposition: attachment; filename=\"" . basePath . "\"; filename*=". strtolower(basePathEncoding) . "''" . basePathEncoded);
} else {
    this->setRawHeader("Content-Disposition: attachment; filename=\"" . basePath . "\"");
}

[sergeyklay]

Thank you

[ian4hu]

@ian4hu could you test these changes with the all common browsers with ASCII/non-ASCII filenames?

I may be wrong, but the file name should still be in double quotes, as the file name may contain spaces (see http://test.greenbytes.de/tech/tc2231/#attwithasciifilenamenqws).

Also, filename in filename= shouldn't be urlencoded, but filename*=UTF-8'' should (see http://test.greenbytes.de/tech/tc2231/#attwithfnrawpctenca).

Maybe something like this:

if basePathEncoding != "ASCII" {
    this->setRawHeader("Content-Disposition: attachment; filename=\"" . basePath . "\"; filename*=". strtolower(basePathEncoding) . "''" . basePathEncoded);
} else {
    this->setRawHeader("Content-Disposition: attachment; filename=\"" . basePath . "\"");
}

I could not make a complete tests in http://test.greenbytes.de/tech/tc2231/#attwithasciifilenamenqws, I only have the main stream browser sets to test, which including latest Chrome, Firefox, Safari, IE or Edge, and because I am busy at work recently, I don't have time to test so I just make the header to follow RFC2231#Section 5.

IMO, the RFC is the standard specific between browser and server. if I make the framework with browser specific behavior, then if the browser changed its behavior your framework may not work with it again.

[ian4hu]

@ian4hu could you test these changes with the all common browsers with ASCII/non-ASCII filenames?

I may be wrong, but the file name should still be in double quotes, as the file name may contain spaces (see http://test.greenbytes.de/tech/tc2231/#attwithasciifilenamenqws).

Also, filename in filename= shouldn't be urlencoded, but filename*=UTF-8'' should (see http://test.greenbytes.de/tech/tc2231/#attwithfnrawpctenca).

Maybe something like this:

if basePathEncoding != "ASCII" {
    this->setRawHeader("Content-Disposition: attachment; filename=\"" . basePath . "\"; filename*=". strtolower(basePathEncoding) . "''" . basePathEncoded);
} else {
    this->setRawHeader("Content-Disposition: attachment; filename=\"" . basePath . "\"");
}

Your suggesstion about quoting is right. as the RFC mentioned:

I will re-read the RFC and make some changes later. thanks.