Please fix the codestyle issues Travis mentions.

For reference: The mentioned changes were done with PR #3182 and #4025.
@illovo and @phproberto may want to have a look at this since it was your PRs originally.

In 2.5 it was #4090 by @wilsonge

Tested this issue and can confirm the bug when the Content-Disposition has multiple entries. After applying the fix, the filename was correctly filtered out of the Content-Disposition string.

Sorry I have no idea how to test this. Without any test instructions its just going to sit here until it is so old that it is just closed next month. @jms2win

@brianteeman Yes this is a old bug that were confirmed and that is still present in J3.5.
This now make more than 1 year that we have to deliver the fix to our customers to allow them continue working correctly.
It would be good that finally Joomla include the fix that were confirmed.
Unfortunately as the bug were not included immediately in joomla, it still affect Joomla 2.5 that should also be fixed and for which we still have to deliver the patch as I suppose that you will no more update it.
We are lucky that Joomla 1.5 is NOT affected with this bug.

The test simply consists in creating a small program to check that the downloadPackage() function returns only a file name without any additional parameters.

Here it is a sample code that you can be use to verify the file name.
This sample calls our website to collect the free "Search/Replace" plugin that is delivered via the DOCMan extension.

In attachement, I join the plugin that contain this code.
pr5238.zip

Be-careful that once the plugin is enabled, its behavior is to DIE Joomla as long as the fix is not applied.

---- Test Code ------
defined('_JEXEC') or die;

class PlgSystemPR5238 extends JPlugin
{
public function onAfterInitialise()
{
$url = 'https://www.jms2win.com/index.php?option=com_docman&task=doc_download&gid=13';

  // Download the file from the URL given
  $p_file = JInstallerHelper::downloadPackage($url, 'sr2win.zip');
  if ( $p_file === false) {
     echo( 'Download ERROR');
  }
  else if ( ($p = strpos( $p_file, ';')) !== FALSE) {
    $validFilename = rtrim( substr( $p_file, 0, $p), '"');
    // https://github.com/joomla/joomla-cms/pull/5238
     die( "FATAL ERROR: Please fix the bug described in Pull Request 5238. Invalid file name returned: [$p_file]. Expected file name : [$validFilename]");
  }
  else {
     echo 'CONGRATULATION Joomla! The bug is fixed';
  }
}

}

---- Extract of DOCMan to send file in application of RFC2183 ------

For your information, DOCMan respect the "The Content-Disposition Header Field" as described in the RFC2183
https://tools.ietf.org/html/rfc2183

Here it is an extract of the code used in DOCMan to send the file.

header("Pragma: public");
header("Cache-Control: must-revalidate, post-check=0, pre-check=0");
header("Expires: 0");

header("Content-Transfer-Encoding: binary");
header('Content-Disposition:' . $cont_dis .';'
. ' filename="' . $this->name . '";'
. ' modification-date="' . $mod_date . '";'
. ' size=' . $fsize .';'
); //RFC2183
header("Content-Type: " . $this->mime ); // MIME type
header("Content-Length: " . $fsize);
<<< The File Content comes here >>>>

It will NOT be fixed in joomla 2.5 - that is EOL and not supported

On 14 April 2016 at 11:28, jms2win notifications@github.com wrote:

@brianteeman https://github.com/brianteeman Yes this is a old bug that
were confirmed and that is still present in J3.5.
This now make more than 1 year that we have to deliver the fix to our
customers to allow them continue working correctly.
It would be good that finally Joomla include the fix that were confirmed.
Unfortunately as the bug were not included immediately in joomla, it still
affect Joomla 2.5 that should also be fixed and for which we still have to
deliver the patch as I suppose that you will no more update it.
We are lucky that Joomla 1.5 is NOT affected with this bug.

The test simply consists in creating a small program to check that the
downloadPackage() function returns only a file name without any additional
parameters.

Here it is a sample code that you can be use to verify the file name.
This sample calls our website to collect the free "Search/Replace" plugin
that is delivered via the DOCMan extension.

In attachement, I join the plugin that contain this code.
pr5238.zip https://github.com/joomla/joomla-cms/files/218870/pr5238.zip

Be-careful that once the plugin is enabled, its behavior is to DIE Joomla
as long as the fix is not applied.

---- Test Code ------
defined('_JEXEC') or die;

class PlgSystemPR5238 extends JPlugin
{
public function onAfterInitialise()
{
$url = '
https://www.jms2win.com/index.php?option=com_docman&task=doc_download&gid=13
';

// Download the file from the URL given
$p_file = JInstallerHelper::downloadPackage($url, 'sr2win.zip');
if ( $p_file === false) {
echo( 'Download ERROR');
}
else if ( ($p = strpos( $p_file, ';')) !== FALSE) {
$validFilename = rtrim( substr( $p_file, 0, $p), '"');
// #5238
die( "FATAL ERROR: Please fix the bug described in Pull Request 5238. Invalid file name returned: [$p_file]. Expected file name : [$validFilename]");
}
else {
echo 'CONGRATULATION Joomla! The bug is fixed';
}
}

}

---- Extract of DOCMan to send file in application of RFC2183 ------

For your information, DOCMan respect the "The Content-Disposition Header
Field" as described in the RFC2183
https://tools.ietf.org/html/rfc2183

Here it is an extract of the code used in DOCMan to send the file.

header("Pragma: public");
header("Cache-Control: must-revalidate, post-check=0, pre-check=0");
header("Expires: 0");

header("Content-Transfer-Encoding: binary");
header('Content-Disposition:' . $cont_dis .';'
. ' filename="' . $this->name . '";'
. ' modification-date="' . $mod_date . '";'
. ' size=' . $fsize .';'
); //RFC2183
header("Content-Type: " . $this->mime ); // MIME type
header("Content-Length: " . $fsize);
<<< The File Content comes here >>>>

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#5238 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

I have tested this item successfully on 2f525b8

Finally this has been tested and can be merged. It is so much easier and faster if you write test instructions

Thank you @jms2win and testers!