dev/core#3034 Ensure that filename contains the file extension for PDFs

[seamuslee001]

Overview

This ensures that the file extension is within the content-disposition header when downloading a pdf file

Before

File extension may not be in the header

After

File extension is in the header and part of the file name

ping @eileenmcnaughton @demeritcowboy @colemanw

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

Thanks @seamuslee001 just I'm not sure this is the correct solution, since it might double-append .pdf in some cases.

I'm also not sure the reported reproduction steps are, or should be, a supported operation:

Click 'edit' on a Mail merge activity
Click preview or download.

Maybe, just wondering if that ever was supported or is a weird artifact of the changes in 5.43+

[seamuslee001]

@demeritcowboy I am summarising that $filename doesn't have the extension in it because of https://github.com/civicrm/civicrm-core/pull/22532/files#diff-02764055ac009532ffaa74e9add094b93fda105527f2bf8d3a8e7defe824aa8eR273

[demeritcowboy]

Except it does when you're initially creating the file. And I'm trying to even reproduce the issue and can't. Can you reproduce the reported issue?

Also noting that oddly the buttons only appear on edit if you have popups enabled but that's only relevant to reproducing. Update: The buttons appearing seems to depend which version of a link you follow, e.g. from the recent items list or not.

[demeritcowboy]

I tried again to reproduce and can't. Have made comment in ticket: https://lab.civicrm.org/dev/core/-/issues/3034#note_70503

[masetto]

@demeritcowboy I think @seamuslee001 modification is right for these reasons:

1. On line 36 of CRM/Utils/PDF/Utils.php it is implicitly written that the $fileName must contain the file extension ($fileName = 'civicrm.pdf', "Ex: "HelloWorld.pdf")

2. If you use DOMPDF, on line 191 the file extension is removed:

    // CRM-19183 remove .pdf extension from filename
    $fileName = basename($fileName, ".pdf");

3. The problem occurs when you use "wkhtmltopdf" instead of DOMPDF: here the file extension is not added!

[demeritcowboy]

jenkins retest this please

[demeritcowboy]

Agreed.

I might do a followup with at least a code comment because I think this happened because it's not obvious what gets set/removed where and the differences. And it turns out it's also different between firefox and chrome.