Skip to content

M2.1.2 : Impossible to create an after interceptor when files are being downloaded by controllers #7356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kanduvisla opened this issue Nov 8, 2016 · 5 comments
Labels
bug report Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed

Comments

@kanduvisla
Copy link
Contributor

kanduvisla commented Nov 8, 2016

Preconditions

Magento - develop branch
Take the \Magento\Sales\Controller\Adminhtml\Shipment\PrintAction controller for example. This has an execute()-method which triggers a downloading to the browser.

Now if you want to do something after this event with a plugin, you're out of luck. Since this controller uses \Magento\Framework\App\Response\Http\FileFactory::create() to print the PDF.

And if you look closer in this code, you'll notice the following:

if ($content !== null) {
    $this->_response->sendHeaders();
    if ($isFile) {
        $stream = $dir->openFile($file, 'r');
        while (!$stream->eof()) {
            echo $stream->read(1024);
        }
    } else {
        $dir->writeFile($fileName, $content);
        $stream = $dir->openFile($fileName, 'r');
        while (!$stream->eof()) {
            echo $stream->read(1024);
        }
    }
    $stream->close();
    flush();
    if (!empty($content['rm'])) {
        $dir->delete($file);
    }
    $this->callExit();
}

This piece if code (especially the $this->callExit()) makes it impossible to use a plugin with after (or around partially), since it hard sends an exit code. That's right: $this->callExit() is equivalent for exit(0).

I suggest that this get handles differently because exiting the code in the middle of the Magento lifecycle doesn't seems right to me. Especially if it breaks functionality like this.

Steps to reproduce

Create a plugin that executes code after \Magento\Sales\Controller\Adminhtml\Shipment\AbstractShipment\PrintAction::execute(). For example:

<type name="Magento\Sales\Controller\Adminhtml\Shipment\PrintAction">
    <plugin name="after_print_shipment"
            type="VendorModule\Plugin\Magento\Sales\Controller\Adminhtml\Shipment\PrintAction"/>
</type>

code:

function afterExecute(\Magento\Sales\Controller\Adminhtml\Shipment\PrintAction $subject, $result) {
    echo 'x';
    die;
}

Expected result

See the 'x' in the browser

Actual result

It just starts downloading; Magento ignores the plugin.

@KrystynaKabannyk
Copy link

Hello @kanduvisla, thank you for your submission. We recently made some changes to the way we process GitHub submissions to more quickly identify and respond to core code issues. Your issue is seems Feature Request or Improvement, and they should now be submitted to the new Magento 2 Feature Requests and Improvements forum (see details here).

@kanduvisla
Copy link
Contributor Author

I don't see this as a feature request, but rather as a bug. The current way of handling downloads by controllers by directly invoking an exit()-method breaks the lifecycle of Magento. In my opinion it's the same as having a block template echo-ing it's HTML and than immediately performing a die() or exit().

@magento-engcom-team magento-engcom-team added 2.1.x bug report Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed Progress: needs update Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed and removed Progress: needs update Issue: Format is not valid Gate 1 Failed. Automatic verification of issue format is failed labels Sep 11, 2017
@miguelbalparda
Copy link
Contributor

There is a proposed fix for this #11029. Can you try it and report back?

@magento-engcom-team
Copy link
Contributor

@kanduvisla, thank you for your report.
We've created internal ticket(s) MAGETWO-77432 to track progress on the issue.

@okorshenko
Copy link
Contributor

@kanduvisla thank you for your report. The issue is already fixed in 2.2-develop branch (PR: #11200 by @osrecio) and will be available in the Magento 2.2.2 release

@okorshenko okorshenko added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Oct 4, 2017
magento-devops-reposync-svc pushed a commit that referenced this issue Apr 12, 2022
[Platform Health] Remove PHP8.0 declaration from composer.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed
Projects
None yet
Development

No branches or pull requests

6 participants