Skip to content
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

SKU allows characters that are invalid for Varnish ESI loading #7513

Closed
maximbaibakov opened this issue Nov 22, 2016 · 4 comments
Closed

SKU allows characters that are invalid for Varnish ESI loading #7513

maximbaibakov opened this issue Nov 22, 2016 · 4 comments
Labels
bug report Component: Framework/Cache Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@maximbaibakov
Copy link

Preconditions

  1. Magento version 2.1.2 CE / EE
  2. PHP 5.6
  3. MySQL 5.7

Steps to reproduce

  1. Configure Magento to use Varnish for the full page cache
  2. Create a product with an apostrophe in the SKU
  3. Browse to the product on the front-end

Expected result

  1. Page loads without any problems

Actual result

  1. Result will depend on your level of error reporting. (Either there will be blank areas on the page where specific pieces of content being loaded via ESI should appear, with the PHP warning written to the error log, or the PHP warning will be output directly on the page:

Warning: SimpleXMLElement::xpath(): Invalid predicate in /server/sites/strancommerce.dev/vendor/magento/framework/View/Model/Layout/Merge.php on line 523

Cause

The apostrophe in the SKU is also included in the resulting layout update handle. When using Varnish, blocks being loaded via ESI result in requests like the following being issued from Varnish to the web server:

http://www.mysite.com/page_cache/block/esi/blocks/[%22catalog.topnav%22]/handles/[%22default%22,%22catalog_product_view%22,%22catalog_product_view_id_231%22,%22catalog_product_view_sku_Men%27s%20Shirt,%22catalog_product_view_type_configurable%22]/

In \Magento\Framework\View\Model\Layout\Merge::_fetchPackageLayoutUpdates, the update handle containing the SKU is passed straight to an xpath expression, where the special character results in the error.

If special characters like apostrophes are considered valid in a SKU, the ESI/layout processing should be modified accordingly to avoid the error. If they are not considered valid, sanitation should be done on SKUs to remove them when saving a product.

The same issue described here, but ticket has been closed due to missing information: #5032

The same issue has been resolved in the Pull Request #874 but seems like it has never been merged into Core.

Hope you can include the fix into upcoming release (2.1.3)

@veloraven
Copy link
Contributor

@maximbaibakov is this issue still actual?
We were not able to reproduce it.

@JerVoo
Copy link

JerVoo commented Jul 10, 2017

@veloraven I managed to reproduce this in Magento v2.1.7 with a product with an apostrophe in the SKU.

The SKU is Polo's van Jack & Jones, containing both an apostrophe and an ampersand, which should both be considered invalid in an SKU. The SKU had been auto generated based on the product name (making it exactly the same as the product's name).

Removing the apostrophe fixes the issue, the ampersand does not seem to be a problem for ESI.

@ArthurTransoftgroup
Copy link

@maximbaibakov @JerVoo We've created internal ticket MAGETWO-71005 to address this issue.

@veloraven veloraven added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Jul 26, 2017
@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Framework/Cache Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed labels Sep 11, 2017
@magento-engcom-team
Copy link
Contributor

@maximbaibakov, thank you for your report.
We were not able to reproduce this issue by following the steps you provided. If you'd like to update it, please reopen the issue.
We tested the issue on 2.3.0-dev, 2.2.0, 2.1.9

@magento-engcom-team magento-engcom-team added the Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch label Oct 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Framework/Cache Issue: Cannot Reproduce Cannot reproduce the issue on the latest `2.4-develop` branch Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

6 participants