Improved invoice PDF generation flow

[rogyar]

Description

Currently, in an attempt to print an invoice from admin panel, the corresponding PDF file is generated directly in var directory of the Magento installation. Very often this approach leads to a mess in the var directory if the invoice printing action takes place frequently during a day.

This PR brings a concept for two improvements:

• Invoice PDFs are being generated in the var/pdf directory so the var directory root is clean.
• The generated PDF does not include the timestamp in its name. There's no reason to keep a separate PDF for the same invoice over and over again. So once the "Print" action is performed on the same invoice more than once, the old PDF file is simply overwritten.

Fixed Issues (if relevant)

1. Print pdf don't delete file in var folder #3535
2. PDF invoices in /var folder #14517

Manual testing scenarios

• Open an existing invoice in the admin panel.

• Click the "Print" button.

• You should have the invoice downloaded.

• You should have a copy of the invoice generated in the var/pdf directory instead of var.

• Click on the "Print" button once again

• The var/pdf directory should not contain an additional copy of the same invoice

The same issue is fare for shipments and credit memos. I'm going to address those parts in a separate PR or within a scope of this one. I just want to make sure that the proposed concept is fine and we are good to move forward.

Thank you

[magento-engcom-team]

Hi @rogyar. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on Pull Request changes
• @magento-engcom-team give me new test instance - deploy NEW test instance based on Pull Request changes
• @magento-engcom-team give me {$VERSION} instance - deploy Vanilla Magento instance for Issue or Pull Request

For more details, please, review the Magento Contributor Assistant documentation

[phoenix128]

Would it better to filter $invoiceId before? I see we are taking it directly from a request.
I am concerned about a potential path traversal attack.
I suppose changing line 51 would be just fine: $invoiceId = (int) $this->getRequest()->getParam('invoice_id');.

[rogyar]

Thanks, Riccardo. That's a very good point. I've added the mentioned change

[phoenix128]

I am wandering if it does worth to move the focus to \Magento\Framework\App\Response\Http\FileFactory::create.
I think the problem is here because there is no way to remove the temporary file after download if this comes from a string.

\Magento\Framework\App\Response\Http\FileFactory::create:99:

            if (!empty($content['rm'])) {
                $dir->delete($file);
            }

[phoenix128]

@rogyar , I am putting this onhold while we are deciding which is the best way.

[rogyar]

Hi @phoenix128. I'm closing this PR since we have a better solution. I've created another PR for fixing the same issue #16401 .

Thanks a lot for the brainstorm! ;)