magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.56k stars 9.32k forks source link

Logos in Invoice PDF stopped showing/working with 2.4.6-p2 #37897

Closed C4rter closed 1 year ago

C4rter commented 1 year ago

Preconditions and environment

Attached an png image to reproduce platewrite_logo_1

Attached an jpg image to reproduce platewrite_logo_1

Steps to reproduce

  1. Configure the "Logo for PDF Print-outs" in Sales->Sales->Invoice and Packing Slip Design with the given PNG.

  2. Try to print a PDF invoice

  3. You receive the following error: {"0":"Warning: Trying to access array offset on value of type null in \/vendor\/magento\/zend-pdf\/library\/Zend\/Pdf\/Resource\/Image\/Png.php on line 291","1":"#1 Zend_Pdf_Resource_Image_Png->__construct() called at [vendor\/magento\/zend-pdf\/library\/Zend\/Pdf\/Resource\/ImageFactory.php:54]\n#2 Zend_Pdf_Resource_ImageFactory::factory() called at [vendor\/magento\/zend-pdf\/library\/Zend\/Pdf\/Image.php:124]\n#3 Zend_Pdf_Image::imageWithPath() called at [vendor\/magento\/module-sales\/Model\/Order\/Pdf\/AbstractPdf.php:282]\n#4 Magento\\Sales\\Model\\Order\\Pdf\\AbstractPdf->insertLogo() called at [vendor\/magento\/module-sales\/Model\/Order\/Pdf\/Invoice.php:140]\n#5 Magento\\Sales\\Model\\Order\\Pdf\\Invoice->getPdf() called at [vendor\/magento\/module-sales\/Controller\/Adminhtml\/Invoice\/AbstractInvoice\/PrintAction.php:58]\n#6 Magento\\Sales\\Controller\\Adminhtml\\Invoice\\AbstractInvoice\\PrintAction->execute() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:58]\n#7 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->___callParent() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:138]\n#8 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->Magento\\Framework\\Interception\\{closure}() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:153]\n#9 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->___callPlugins() called at [generated\/code\/Magento\/Sales\/Controller\/Adminhtml\/Order\/Invoice\/PrintAction\/Interceptor.php:23]\n#10 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->execute() called at [vendor\/magento\/framework\/App\/Action\/Action.php:111]\n#11 Magento\\Framework\\App\\Action\\Action->dispatch() called at [vendor\/magento\/module-backend\/App\/AbstractAction.php:151]\n#12 Magento\\Backend\\App\\AbstractAction->dispatch() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:58]\n#13 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->___callParent() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:138]\n#14 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->Magento\\Framework\\Interception\\{closure}() called at [vendor\/magento\/module-backend\/App\/Action\/Plugin\/Authentication.php:145]\n#15 Magento\\Backend\\App\\Action\\Plugin\\Authentication->aroundDispatch() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:135]\n#16 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->Magento\\Framework\\Interception\\{closure}() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:153]\n#17 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->___callPlugins() called at [generated\/code\/Magento\/Sales\/Controller\/Adminhtml\/Order\/Invoice\/PrintAction\/Interceptor.php:32]\n#18 Magento\\Sales\\Controller\\Adminhtml\\Order\\Invoice\\PrintAction\\Interceptor->dispatch() called at [vendor\/magento\/framework\/App\/FrontController.php:245]\n#19 Magento\\Framework\\App\\FrontController->getActionResponse() called at [vendor\/magento\/framework\/App\/FrontController.php:212]\n#20 Magento\\Framework\\App\\FrontController->processRequest() called at [vendor\/magento\/framework\/App\/FrontController.php:147]\n#21 Magento\\Framework\\App\\FrontController->dispatch() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:58]\n#22 Magento\\Framework\\App\\FrontController\\Interceptor->___callParent() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:138]\n#23 Magento\\Framework\\App\\FrontController\\Interceptor->Magento\\Framework\\Interception\\{closure}() called at [vendor\/magento\/framework\/Interception\/Interceptor.php:153]\n#24 Magento\\Framework\\App\\FrontController\\Interceptor->___callPlugins() called at [generated\/code\/Magento\/Framework\/App\/FrontController\/Interceptor.php:23]\n#25 Magento\\Framework\\App\\FrontController\\Interceptor->dispatch() called at [vendor\/magento\/framework\/App\/Http.php:116]\n#26 Magento\\Framework\\App\\Http->launch() called at [vendor\/magento\/framework\/App\/Bootstrap.php:264]\n#27 Magento\\Framework\\App\\Bootstrap->run() called at [pub\/index.php:30]\n","url":"\/admin\/sales\/order_invoice\/print\/invoice_id\/13083\/key\/74ba62670c05c57820e0765f8efd774afffaa898a5ea2f97ccdb9bf53da94241\/","script_name":"\/index.php","report_id":"2e0b1428c1bd6c499aed5f0128ab15cf066dc2b14bfded767f92cb6f13da231e"}

  4. When you switch the logo to a JPG or another PNG you won't receive that error but no image is printed to the PDF, it's just blank where the logo is supposed to be.

Expected result

It's expected that the logo can still be printed on the PDF invoice.

Actual result

Either an error is thrown or no logo is printed.

Additional information

No response

Release note

No response

Triage and priority

m2-assistant[bot] commented 1 year ago

Hi @C4rter. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

C4rter commented 1 year ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter. Thank you for your request. I'm working on Magento instance for you.

m2-assistant[bot] commented 1 year ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter, here is your Magento Instance: https://b7c3986e611fdc9794e56d663f269212.instances-prod.magento-community.engineering Admin access: https://b7c3986e611fdc9794e56d663f269212.instances-prod.magento-community.engineering/admin_202e Login: 5a5e8e7d Password: bc5fc5c93417

C4rter commented 1 year ago

@magento give me 2.46-p2 instance

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter, unfortunately there is no ability to deploy Magento instance at the moment. Please try again later.

C4rter commented 1 year ago

@magento give me 2.4.6-p2 instance

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter, unfortunately there is no ability to deploy Magento instance at the moment. Please try again later.

C4rter commented 1 year ago

Cannot reproduce on 2.4-develop. Unable to boot up a 2.4.6-p2 version.

C4rter commented 1 year ago

A major change seem to be that now in 2.4.6 "magento/zend-pdf": "^1.16" is used. I come from 2.4.5 and that wasn't the case there. zend-pdf was no Magento module. That seems to be causing the issue.

C4rter commented 1 year ago

When I copy/paste the old folder vendor/magento/zendframework1/library/Zend/Pdf from the earlier version into the new one from magento vendor/magento/zend-pdf/library/Zend/Pdf (and overwrite/replace the files of course) the logo works again.

C4rter commented 1 year ago

Found the reason why it stopped working now.

The issue is here: vendor/magento/zend-pdf/library/Zend/Pdf/Element.php

Compared to the original file you've added this:

    /**
     * Object value
     *
     * @var string
     */
    public $value;

And that causes serious issue in the stream handling here: vendor/magento/zend-pdf/library/Zend/Pdf/Resource/Image/Png.php

$pngDataRawDecoded = $decodingStream->value;

Accessing ->value there just returns "null" all the time with your change instead of actually loading/streaming the file.

C4rter commented 1 year ago

Here's a patch for that:

diff --git a/library/Zend/Pdf/Element.php b/library/Zend/Pdf/Element.php
index 1f423e801087..0e4d79aac933 100644
--- a/library/Zend/Pdf/Element.php
+++ b/library/Zend/Pdf/Element.php
@@ -46,13 +46,6 @@
     private $_parentObject = null;

     /**
-     * Object value
-     *
-     * @var string
-     */
-    public $value;
-
-    /**
      * Return type of the element.
      * See ZPdfPDFConst for possible values
      *
C4rter commented 1 year ago

@magento give me 2.4.6-p2 instance

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @C4rter, unfortunately there is no ability to deploy Magento instance at the moment. Please try again later.

engcom-Bravo commented 1 year ago

@magento give me 2.4-develop instance

magento-deployment-service[bot] commented 1 year ago

Hi @engcom-Bravo. Thank you for your request. I'm working on Magento instance for you.

magento-deployment-service[bot] commented 1 year ago

Hi @engcom-Bravo, here is your Magento Instance: https://b7c3986e611fdc9794e56d663f269212.instances-prod.magento-community.engineering Admin access: https://b7c3986e611fdc9794e56d663f269212.instances-prod.magento-community.engineering/admin_af16 Login: 4a209e5a Password: 5e2ee67ecd67

engcom-Bravo commented 1 year ago

Hi @C4rter,

Thank you for reporting and collaboration.

Verified the issue on Magento 2.4-develop instance and the issue is not reproducible.Kindly refer the screenshots.

Steps to reproduce

We are able to get the Logo for a given png image in PDF invoice.

Screenshot 2023-08-22 at 12 13 48 PM

It seems to be issue got fixed in Magento 2.4-develop branch.

it is recommended to verify the issue on Magento 2.4-develop instance as it is an upcoming 2.4.x release and have latest code base.

Thanks.

hostep commented 1 year ago

@C4rter: thanks for reporting this

Can you confirm that your issue is caused by the changes in https://github.com/magento/magento-zend-pdf/pull/1? (your comments mention code that was introduced there) I haven't tested all the changes very carefully but if you can confirm that changes from the PR causes the problem, I can try to test it a bit better to see what's going on.

Also can you tell me what version of magento/zend-pdf you are using (and if you apply any custom patches to it or not)? Because that PR I just mentioned wasn't released in a new version yet, it's still pending to be released as far as I'm aware...

C4rter commented 1 year ago

@hostep Thank you for your reply.

Can you confirm that your issue is caused by the changes in magento/magento-zend-pdf#1? (your comments mention code that was introduced there)

Yeah, the change that causes the issue for me is in that PR. It's that one: https://github.com/magento/magento-zend-pdf/pull/1/files#diff-d58c4dd9f204c5959a3000df54e3b1c786efa9cf005463b9a961cb5ebb0a6c51

Because $value is pre-initialized in that class the image file streaming doesn't work anymore.

I can try to test it a bit better to see what's going on.

Yes, it would be great if you are able to test the use case that I mentioned here. Feel free to use my images. The png triggers the exception. Other images just don't appear but without an exception. The cause is the same for all, that the stream stopped working because it's not read anymore, instead it's prefilled with "null" and that's returned instead of pulling the file. Which leads to an exception for my png (because of specific png handling with the Stream value) and just missing images with other image files.

Also can you tell me what version of magento/zend-pdf you are using

The composer.lock bit for magento/zend-pdf points to "dev-main" and commit 243d76feb68ef85aa594ebb1c3e44b78497e54c9 for magento/zend-pdf. That's the latest on the main branch of that repo: https://github.com/magento/magento-zend-pdf/commit/243d76feb68ef85aa594ebb1c3e44b78497e54c9 And it contains the change.

and if you apply any custom patches to it or not

Aside from the one now that I needed to fix the issue there are no other custom patches applied to it.

Because that PR I just mentioned wasn't released in a new version yet, it's still pending to be released as far as I'm aware...

That's correct it seems. It's merged into main but it's not tagged in a new version yet. With 1.16.2 everything would still be fine I guess (until your change is released).

magento/framework and magento/page-builder both have a requirement "magento/zend-pdf": "^1.16" in my composer.lock. But it seems that they pull in the main branch instead of a specific version. That's strange.

Here's the version part for magento/zend-pdf in my current composer.lock

            "name": "magento/zend-pdf",
            "version": "dev-main",

But when I do a composer create-project --repository-url=https://repo.magento.com/ magento/project-community-edition:2.4.6-p2 into a new folder, that part in the composer.lock looks like this:

            "name": "magento/zend-pdf",
            "version": "1.16.2",

So that's the current issue for me, that version. It should be "version": "1.16.2" but for me it's "version": "dev-main" No idea how that could happen. You have any idea?

When I manually add it to the require section like this, it pulls the old version: "magento/zend-pdf": "1.16.2", But when I add it like this (like Magento requires it) "magento/zend-pdf": "^1.16", It pulls the dev-main version: Upgrading magento/zend-pdf (1.16.2 => dev-main 243d76f): Extracting archive Edit: Ah it's because of "minimum-stability": "dev" instead of "minimum-stability": "stable" in my composer.json

But still, once your change is tagged and relased, I would assume it would cause more issues. I was just the first one to get it now.

hostep commented 1 year ago

Thanks again for report @C4rter, I'm able to reproduce the issue.

Will investigate later today or later this week (depending on my time, I'm not an Adobe employee, so I'm not being paid for this) to see what we can do to fix this.

C4rter commented 1 year ago

I'm able to reproduce the issue.

That's good to hear.

Will investigate later today or later this week (depending on my time, I'm not an Adobe employee, so I'm not being paid for this) to see what we can do to fix this.

Sure, that's good enough for me. With the change to the minimum-stability from dev to stable I'm running the working version now again and don't need my patch anymore. That's fine for now. Thank you @hostep

hostep commented 1 year ago

@C4rter: this works for me: https://github.com/magento/magento-zend-pdf/pull/2, can you maybe also test to be sure?

C4rter commented 1 year ago

@C4rter: this works for me: magento/magento-zend-pdf#2, can you maybe also test to be sure?

Thank you, I think I can try it tomorrow or Friday. Will give you feedback here.

C4rter commented 1 year ago

@hostep Just tested it and it works. Stream is being pulled again when accessing the $value property.

hostep commented 1 year ago

Version 1.16.3 got released yesterday with these fixes included. I think we can close this issue here now? 🙂

engcom-Bravo commented 1 year ago

Hi @hostep,

Thanks for your contribution & collaboration over here.

@C4rter We are Closing this issue Please feel to raise a fresh ticket or reopen this ticket if you need more assistance on this.

Thanks.