Sitemap fix error in case of image with null Title or Caption #37462

RPrudon · 2023-05-04T12:18:06Z

Description (*)

This patch fixes Patch to fix Argument 1 passed to Magento\Sitemap\Model\Sitemap::escapeXmlText() must be of the type string, null given when the given title or caption are null.

Manual testing scenarios (*)

Add a sitemap image with a title or caption with the value null.
The Sitemap model should no longer return an error.

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

Resolved issues:

resolves [Issue] Sitemap fix error in case of image with null Title or Caption #39410: Sitemap fix error in case of image with null Title or Caption

m2-assistant · 2023-05-04T12:18:09Z

Hi @RPrudon. Thank you for your contribution!
Here are some useful tips on 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 give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

RPrudon · 2023-05-04T12:18:26Z

@magento run all tests

magento-automated-testing · 2023-05-04T12:18:33Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

RPrudon · 2023-05-25T09:41:02Z

@magento run all tests

magento-automated-testing · 2023-05-25T09:41:11Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Hotel · 2023-06-06T10:11:51Z

@magento run all tests

magento-automated-testing · 2023-06-06T10:11:59Z

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

engcom-Hotel

Hello @RPrudon,

Thanks for the contribution!

The changes look good to us. Just one thing, please cover this change with some automated tests like unit test.

Thanks

engcom-Dash · 2024-09-26T12:57:17Z

@magento run all tests

engcom-Dash · 2024-10-22T08:54:02Z

@magento run all tests

engcom-Dash · 2024-10-23T09:09:51Z

@magento run all tests

engcom-Hotel · 2024-11-26T07:28:08Z

@magento run all tests

engcom-Hotel

Hello @RPrudon @engcom-Dash,

Please refer to my review comments and also please cover the changes with some automated tests.

Thanks

engcom-Hotel · 2024-11-26T08:35:47Z

app/code/Magento/Sitemap/Model/Sitemap.php

@@ -576,9 +576,9 @@ protected function _getSitemapRow($url, $lastmod = null, $changefreq = null, $pr
            foreach ($images->getCollection() as $image) {
                $row .= '<image:image>';
                $row .= '<image:loc>' . $this->_escaper->escapeUrl($image->getUrl()) . '</image:loc>';
-                $row .= '<image:title>' . $this->escapeXmlText($images->getTitle()) . '</image:title>';
+                $row .= '<image:title>' . $this->escapeXmlText((string)$images->getTitle()) . '</image:title>';


I think it should be $image instead of $images

Suggested change

$row .= '<image:title>' . $this->escapeXmlText((string)$images->getTitle()) . '</image:title>';

$row .= '<image:title>' . $this->escapeXmlText((string)$image->getTitle()) . '</image:title>';

@engcom-Hotel, I have analyzed and debugged the code and found that the current implementation is correct. The reason is that $image does not contain any title; the title is associated only with $images. This is why, in the current codebase, it is $images instead of $image. Please have a look at below screenshots:

Additionally, for the caption, we are already checking if it is null, so IMO, we don't need the typecasting, as the error mentioned is related to null values only.

Since the changes is related to typecasting only and we already have automated tests for this class, could you please review the implementation?

Thanks!

cc: @RPrudon

engcom-Dash · 2024-11-29T06:37:52Z

@magento create issue

engcom-Dash · 2024-11-29T12:52:01Z

Hello @RPrudon,

Thanks for the contributions!

The changes look good to me, but could you please provide a scenario where we would encounter the mentioned error in the description? I tried generating the sitemap multiple times, but I didn't encounter any errors. Additionally, you mentioned that this error occurs when we have a null caption, but this scenario is already handled on line 580. If you could provide more details or a specific scenario where these errors occur, it would help us understand the issue more clearly.

Thanks again!

Fix error in case of image with null Title or Caption

b09d03f

m2-community-project bot added the Progress: pending review label May 4, 2023

engcom-Bravo added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label May 4, 2023

Merge branch '2.4-develop' into patch-4

cabaaaf

engcom-Hotel self-requested a review June 6, 2023 10:11

Merge branch '2.4-develop' into patch-4

a367075

m2-community-project bot added Progress: review and removed Progress: pending review labels Jun 6, 2023

engcom-Hotel requested changes Jun 6, 2023

View reviewed changes

m2-community-project bot assigned engcom-Hotel Jun 6, 2023

m2-community-project bot added Progress: needs update and removed Progress: review labels Jun 6, 2023

engcom-Dash self-assigned this Sep 26, 2024

engcom-Dash and others added 3 commits October 22, 2024 14:22

37462: Fix PHPCS warnings

5c14d7a

37462: remove extra line

06547ed

Merge branch '2.4-develop' into patch-4

1d5fd30

37462: Fix static failure related to copyright tag

dc8e367

Merge branch '2.4-develop' into patch-4

329e7c1

engcom-Hotel requested changes Nov 26, 2024

View reviewed changes

m2-community-project bot added Progress: review and removed Progress: needs update labels Nov 29, 2024

m2-assistant bot mentioned this pull request Nov 29, 2024

[Issue] Sitemap fix error in case of image with null Title or Caption #39410

Open

5 tasks

engcom-Hotel approved these changes Nov 29, 2024

View reviewed changes

m2-community-project bot added Progress: ready for testing and removed Progress: review labels Nov 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sitemap fix error in case of image with null Title or Caption #37462

Sitemap fix error in case of image with null Title or Caption #37462

RPrudon commented May 4, 2023 •

edited by m2-assistant bot

Loading

m2-assistant bot commented May 4, 2023

RPrudon commented May 4, 2023

magento-automated-testing bot commented May 4, 2023

RPrudon commented May 25, 2023

magento-automated-testing bot commented May 25, 2023

engcom-Hotel commented Jun 6, 2023

magento-automated-testing bot commented Jun 6, 2023

engcom-Hotel left a comment

engcom-Dash commented Sep 26, 2024

engcom-Dash commented Oct 22, 2024

engcom-Dash commented Oct 23, 2024

engcom-Hotel commented Nov 26, 2024

engcom-Hotel left a comment

engcom-Hotel Nov 26, 2024

engcom-Dash Nov 29, 2024

engcom-Dash commented Nov 29, 2024

engcom-Dash commented Nov 29, 2024

	$row .= '<image:title>' . $this->escapeXmlText((string)$images->getTitle()) . '</image:title>';
	$row .= '<image:title>' . $this->escapeXmlText((string)$image->getTitle()) . '</image:title>';

Sitemap fix error in case of image with null Title or Caption #37462

Are you sure you want to change the base?

Sitemap fix error in case of image with null Title or Caption #37462

Conversation

RPrudon commented May 4, 2023 • edited by m2-assistant bot Loading

Description (*)

Manual testing scenarios (*)

Contribution checklist (*)

Resolved issues:

m2-assistant bot commented May 4, 2023

RPrudon commented May 4, 2023

magento-automated-testing bot commented May 4, 2023

RPrudon commented May 25, 2023

magento-automated-testing bot commented May 25, 2023

engcom-Hotel commented Jun 6, 2023

magento-automated-testing bot commented Jun 6, 2023

engcom-Hotel left a comment

Choose a reason for hiding this comment

engcom-Dash commented Sep 26, 2024

engcom-Dash commented Oct 22, 2024

engcom-Dash commented Oct 23, 2024

engcom-Hotel commented Nov 26, 2024

engcom-Hotel left a comment

Choose a reason for hiding this comment

engcom-Hotel Nov 26, 2024

Choose a reason for hiding this comment

engcom-Dash Nov 29, 2024

Choose a reason for hiding this comment

engcom-Dash commented Nov 29, 2024

engcom-Dash commented Nov 29, 2024

RPrudon commented May 4, 2023 •

edited by m2-assistant bot

Loading