\Magento\Framework\App\Response\Http\FileFactory::create generates output causing headers already sent error

[kirkmadera]

\Magento\Framework\App\Response\Http\FileFactory::create generates output causing headers already sent error. It looks like the intent of this is to output the file at the same time as writing to it when creating a new file from string content. This, however, breaks the contract of controllers (or any code, really) that use this method. Specifically, for controllers, the contract is to return a \Magento\Framework\App\ResponseInterface object and allow the app to do the actual rendering.

M2 2.3.5 code

See $this->_response->sendHeaders();, echo, and flush()

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

public function create(
    $fileName,
    $content,
    $baseDir = DirectoryList::ROOT,
    $contentType = 'application/octet-stream',
    $contentLength = null
) {
    $dir = $this->_filesystem->getDirectoryWrite($baseDir);
    $isFile = false;
    $file = null;
    $fileContent = $this->getFileContent($content);
    if (is_array($content)) {
        if (!isset($content['type']) || !isset($content['value'])) {
            throw new \InvalidArgumentException("Invalid arguments. Keys 'type' and 'value' are required.");
        }
        if ($content['type'] == 'filename') {
            $isFile = true;
            $file = $content['value'];
            if (!$dir->isFile($file)) {
                // phpcs:ignore Magento2.Exceptions.DirectThrow
                throw new \Exception((string)new \Magento\Framework\Phrase('File not found'));
            }
            $contentLength = $dir->stat($file)['size'];
        }
    }
    $this->_response->setHttpResponseCode(200)
        ->setHeader('Pragma', 'public', true)
        ->setHeader('Cache-Control', 'must-revalidate, post-check=0, pre-check=0', true)
        ->setHeader('Content-type', $contentType, true)
        ->setHeader('Content-Length', $contentLength === null ? strlen($fileContent) : $contentLength, true)
        ->setHeader('Content-Disposition', 'attachment; filename="' . $fileName . '"', true)
        ->setHeader('Last-Modified', date('r'), true);

    if ($content !== null) {
        $this->_response->sendHeaders();
        if ($isFile) {
            $stream = $dir->openFile($file, 'r');
            while (!$stream->eof()) {
                // phpcs:ignore Magento2.Security.LanguageConstruct.DirectOutput
                echo $stream->read(1024);
            }
        } else {
            $dir->writeFile($fileName, $fileContent);
            $file = $fileName;
            $stream = $dir->openFile($fileName, 'r');
            while (!$stream->eof()) {
                // phpcs:ignore Magento2.Security.LanguageConstruct.DirectOutput
                echo $stream->read(1024);
            }
        }
        $stream->close();
        flush();
        if (!empty($content['rm'])) {
            $dir->delete($file);
        }
    }
    return $this->_response;
}

RMA use case where this is used incorrectly

This method returns the response object. When the create() method also outputs directly, this result in a "headers already sent" PHP error.

Oddly, the behavior is inconsistent. It only seems to fail on the second attempt to download a file on a given frontend page.

\Magento\Rma\Controller\Adminhtml\Rma\PrintLabel::execute See return $this->_fileFactory->create

public function execute()
{
    try {
        $model = $this->_initModel();
        /** @var $shippingModel \Magento\Rma\Model\Shipping */
        $shippingModel = $this->_objectManager->create(\Magento\Rma\Model\Shipping::class);
        $labelContent = $shippingModel->getShippingLabelByRma($model)->getShippingLabel();
        if ($labelContent) {
            $pdfContent = null;
            if (stripos($labelContent, '%PDF-') !== false) {
                $pdfContent = $labelContent;
            } else {
                $pdf = new \Zend_Pdf();
                $page = $this->labelService->createPdfPageFromImageString($labelContent);
                if (!$page) {
                    $this->messageManager->addError(
                        __(
                            "We don't recognize or support the file extension in shipment %1.",
                            $model->getIncrementId()
                        )
                    );
                }
                $pdf->pages[] = $page;
                $pdfContent = $pdf->render();
            }

            return $this->_fileFactory->create(
                'ShippingLabel(' . $model->getIncrementId() . ').pdf',
                $pdfContent,
                DirectoryList::MEDIA,
                'application/pdf'
            );
        }
    } catch (\Magento\Framework\Exception\LocalizedException $e) {
        $this->messageManager->addError($e->getMessage());
    } catch (\Exception $e) {
        $this->_objectManager->get(\Psr\Log\LoggerInterface::class)->critical($e);
        $this->messageManager->addError(__('Something went wrong creating a shipping label.'));
    }

    /** @var \Magento\Backend\Model\View\Result\Redirect $resultRedirect */
    $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT);
    return $resultRedirect->setPath('adminhtml/*/edit', ['id' => $this->getRequest()->getParam('id')]);
}

Preconditions (*)

1. Magento 2.3.5-p1
2. No frontend RMA customizations

Steps to reproduce (*)

Our test case is in downloading the shipping label or packing slip on the frontend for RMA. But it seems this would be true for any file download on the frontend. I am actually not sure why this works in the admin to be honest, because the admin controller suffers from the same issue of both outputting and returning a response object.

1. Place an order on the frontend
2. Invoice the order in the admin
3. Ship the order in the admin
4. Initiate a return on the frontend
5. Authorize the return in the admin
6. Create a shipping label in the admin
7. Print the label in the admin to ensure it worked
8. Attempt to print the shipping label on the frontend
9. Oddly, this seems to work on the first try sometimes. Try printing the label multiple times to see the error.
10. Watch var/log/exception.log to see the "headers already sent" PHP error
11. You can also try to print the packing slip on the frontend "Show Packages" > "Print". You will see the same error.

Expected result (*)

1. PDF downloads every single time you request to print the shipping label

Actual result (*)

1. PDF sometimes downloads on the first request
2. The PDF then fails to download an every subsequent request.
3. A "headers already sent" error is logged in var/log/exception.log

---

Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.

• [ ] Severity: S0 - Affects critical data or functionality and leaves users without workaround.
• [ x] Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
• [ ] Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
• [ ] Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
• [ ] Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.

Customers are able to potentially work around this if they refresh the full page a few times and try the download until it works. There are times when it doesn't work at all and the customer may have no workaround.

In this use case, this would prevent a customer from being able to print his shipping label and return his item. This would result in a customer service call and a work around on the customer service end of printing the label manually and emailing it to the customer.

[m2-assistant[bot]]

Hi @kirkmadera. Thank you for your report. To help us process this issue please make sure that you provided the following information:

• Summary of the issue
• Information on your environment
• Steps to reproduce
• Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.4-develop instance - upcoming 2.4.x release

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

Please, add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

  :warning: According to the Magento Contribution requirements, all issues must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

:clock10: You can find the schedule on the Magento Community Calendar page.

:telephone_receiver: The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

:movie_camera: You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

:pencil2: Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[kirkmadera]

This can't be fixed by removing the output as many other classes assume this functionality to exist. I think you should maybe add a flag like $outputDirectly defaulted to false and call this out in BC breaks. Then update existing core code to either use this with the $response object correctly or set this flag to true.

Our current workaround is add output buffering around this. This feels really dirty, but makes this a non-issue for downloads on the frontend. I think it must technically be sending the headers multiple times, but it does it in one shot which makes PHP not complain. I am guessing that Nginx cleans up the duplicate headers? Not sure of the downstream here, but it does work.

class FileFactoryPlugin
{
    /**
     * @param \Magento\Framework\App\Response\Http\FileFactory $subject
     * @param string $fileName
     * @param string|array $content
     * @param string $baseDir
     * @param string $contentType
     * @param int $contentLength
     * @return array
     */
    public function beforeCreate(
        \Magento\Framework\App\Response\Http\FileFactory $subject,
        $fileName,
        $content,
        $baseDir = DirectoryList::ROOT,
        $contentType = 'application/octet-stream',
        $contentLength = null
    ) {
        ob_start();
        return
            [
                $fileName,
                $content,
                $baseDir,
                $contentType,
                $contentLength
            ];
    }
}

[df2k2]

I wanted to point out that there was a post in Magento Community Engineering Slack -- General channel -- That appeared to validate this issue.

Warning: Cannot modify header information - headers already sent by (output started at /home/jetrails/a.com/html/vendor/magento/framework/App/Response/Http/FileFactory.php:102) in /home/jetrails/a.com/html/vendor/magento/framework/Stdlib/Cookie/PhpCookieManager.php on line 148

#1 setcookie() called at [vendor/magento/framework/Stdlib/Cookie/PhpCookieManager.php:148]
#2 Magento\Framework\Stdlib\Cookie\PhpCookieManager->setCookie() called at [vendor/magento/framework/Stdlib/Cookie/PhpCookieManager.php:101]
#3 Magento\Framework\Stdlib\Cookie\PhpCookieManager->setSensitiveCookie() called at [vendor/magento/framework/App/Response/Http.php:114]
#4 Magento\Framework\App\Response\Http->sendVary() called at [generated/code/Magento/Framework/App/Response/Http/Interceptor.php:37]
#5 Magento\Framework\App\Response\Http\Interceptor->sendVary() called at [vendor/magento/module-page-cache/Model/App/Response/HttpPlugin.php:25]
#6 Magento\PageCache\Model\App\Response\HttpPlugin->beforeSendResponse() called at [vendor/magento/framework/Interception/Interceptor.php:121]
#7 Magento\Framework\App\Response\Http\Interceptor->Magento\Framework\Interception\{closure}() called at [vendor/magento/framework/Interception/Interceptor.php:153]
#8 Magento\Framework\App\Response\Http\Interceptor->___callPlugins() called at [generated/code/Magento/Framework/App/Response/Http/Interceptor.php:117]
#9 Magento\Framework\App\Response\Http\Interceptor->sendResponse() called at [vendor/magento/framework/App/Bootstrap.php:262]
#10 Magento\Framework\App\Bootstrap->run() called at [pub/index.php:40]

/returns/attachment/download/uid/1b2805dced1f9b4d0e1921f67bf426f8f6cf413f87a521813f69c43c5ee02a32/index.php

Here is additional details with the versioning:

I'm having this error after I upgrade 2.3.3 to 2.3.5p2. Any input is appreciated.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 14 days if no further activity occurs. Is this issue still relevant? If so, what is blocking it? Is there anything you can do to help move it forward? Thank you for your contributions!

[Djohn12]

Hi,

still facing this issue on magento enterprise 2.3.6 when trying to download order invoice from my customer account in storefront.

Any idea if this still happens on magento 2.4.x ?