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

Allow limited HTML when rendering failure messages (CSV import). #32000

Merged
merged 1 commit into from
Mar 11, 2022

Conversation

barryhughes
Copy link
Member

Changes proposed in this Pull Request:

When the product CSV importer is used, any errors (an example might be where a downloadable file has a disallowed mimetype) will be presented to the user when the task completes.

These error messages are nicely formatted with HTML, however when exposed in this particular context the level of escaping is too high, and exposes the HTML tags:

csv-importer-exhibit-1

One solution would be to strip the tags completely, but the error messages would not be very clear (and any links would be useless). Much better if we allow a decent range of HTML tags to give us something like the following:

csv-importer-exhibit-2

How to test the changes in this Pull Request:

  1. Go to Products ▸ Import and upload a file containing the CSV I shared below.
  2. Once the import completes it should indicate it failed to import 1 product. Click the provided link to view the import log.
  3. Without this change, you will see the escaped HTML tags. With this change, you will see the HTML properly rendered.

Sample CSV:

Type,Name,Published,"Is featured?","Visibility in catalog","Short description",Description,"Date sale price starts","Date sale price ends","Tax status","Tax class","In stock?",Stock,"Backorders allowed?","Sold individually?","Weight (lbs)","Length (in)","Width (in)","Height (in)","Allow customer reviews?","Purchase note","Sale price","Regular price",Categories,Tags,"Shipping class",Images,"Download limit","Download expiry days",Parent,"Grouped products",Upsells,Cross-sells,"External URL","Button text",Position,"Attribute 1 name","Attribute 1 value(s)","Attribute 1 visible","Attribute 1 global","Attribute 2 name","Attribute 2 value(s)","Attribute 2 visible","Attribute 2 global","Meta: _wpcom_is_markdown","Download 1 name","Download 1 URL","Download 2 name","Download 2 URL"
"simple, downloadable, virtual",Album,1,0,visible,"This is a simple, virtual product.","Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vestibulum sagittis orci ac odio dictum tincidunt. Donec ut metus leo. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Sed luctus, dui eu sagittis sodales, nulla nibh sagittis augue, vel porttitor diam enim non metus. Vestibulum aliquam augue neque. Phasellus tincidunt odio eget ullamcorper efficitur. Cras placerat ut turpis pellentesque vulputate. Nam sed consequat tortor. Curabitur finibus sapien dolor. Ut eleifend tellus nec erat pulvinar dignissim. Nam non arcu purus. Vivamus et massa massa.",,,taxable,,1,,0,0,,,,,1,,,15,Music,,,https://woocommercecore.mystagingwebsite.com/wp-content/uploads/2017/12/album-1.jpg,1,1,,,,,,,0,,,,,,,,,1,"Single 1",https://woocommerce.lab/disallowed.zzz,"Single 2",https://woocommerce.lab/bad.zzz

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Tweak - Change level of escaping used to render the CSV import error log.

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@barryhughes barryhughes requested review from a team and jorgeatorres and removed request for a team March 1, 2022 03:44
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 1, 2022
@jorgeatorres jorgeatorres self-assigned this Mar 1, 2022
Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @barryhughes! LGTM 👍.

@jorgeatorres jorgeatorres merged commit 8af58a5 into trunk Mar 11, 2022
@jorgeatorres jorgeatorres deleted the fix/csv-import-failure-formatting branch March 11, 2022 19:51
@github-actions github-actions bot added this to the 6.4.0 milestone Mar 11, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 11, 2022

Hi @jorgeatorres, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add changelog label
  • Add the release: add testing instructions label

@jorgeatorres jorgeatorres added the release: add changelog Mark all PRs that have not had their changelog entries added. [auto] label Mar 11, 2022
@ObliviousHarmony ObliviousHarmony removed the release: add changelog Mark all PRs that have not had their changelog entries added. [auto] label Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants