Skip to content

Suppressing warnings for malformed metadata in product template downloads #32971

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.

Sign up for GitHub

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

Jump to bottom
Merged
joelclimbsthings merged 4 commits into from
May 24, 2022
Merged

Suppressing warnings for malformed metadata in product template downloads #32971

joelclimbsthings merged 4 commits into from
May 24, 2022

Conversation

@joelclimbsthings
Copy link
Contributor

@joelclimbsthings joelclimbsthings commented May 11, 2022 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

This resolves a warning that is being triggered when a user creates a product via the digital download template. The warning results from corrupt meta-data on an image file that is downloaded (from this source) and then used within the product template as the product image. This doesn't cause any functional issue other than the warning when WP_DEBUG mode is set to true.

A couple options to resolve this:

  1. Fix the EXIF data on the image itself, although this is sourced at https://woocommercecore.mystagingwebsite.com/wp-content/uploads/2017/12/album-1.jpg, and I'm unsure how this is edited or accessed.
  2. We could consider this just too noisy as it would be triggered from any image with questionable metadata, with the solution being to suppress warnings in this instance.

I've taken option 2 for this PR.

Closes #32137 .

How to test the changes in this Pull Request:

  1. Checkout branch
  2. Ensure WP_DEBUG is set to true in wp-config.php
  3. Go to the Home screen
  4. Press Add my products on task list
  5. And then press Start with a template
  6. Select Digital product
  7. Ensure no warning is displayed here or within the debug.log

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?
  • Have you created a changelog file for each project being changed, ie pnpm nx changelog <project>?

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.

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 11, 2022
@joelclimbsthings joelclimbsthings marked this pull request as ready for review May 11, 2022 23:12
@joelclimbsthings joelclimbsthings requested a review from a team May 11, 2022 23:12
@joelclimbsthings joelclimbsthings self-assigned this May 11, 2022
@joelclimbsthings joelclimbsthings changed the title Supressing warnings originating from metadata functions Suppressing warnings originating from metadata functions May 11, 2022
@joelclimbsthings joelclimbsthings changed the title Suppressing warnings originating from metadata functions Suppressing warnings for malformed metadata in product template images May 11, 2022
@joelclimbsthings joelclimbsthings changed the title Suppressing warnings for malformed metadata in product template images Suppressing warnings for malformed metadata in product template downloads May 11, 2022
joelclimbsthings
joelclimbsthings commented May 11, 2022
View reviewed changes
plugins/woocommerce/includes/wc-rest-functions.php
}

$image_meta = wp_read_image_metadata( $upload['file'] );
$image_meta = @wp_read_image_metadata( $upload['file'] );
Copy link
Contributor Author

@joelclimbsthings joelclimbsthings May 11, 2022

Choose a reason for hiding this comment

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

Adding the @ here will suppress any warnings from calling this function. I noticed other calls to wp_read_image_metadata within WooCommerce are already doing this.

joshuatf
joshuatf reviewed May 13, 2022
View reviewed changes
Copy link
Contributor

@joshuatf joshuatf left a comment

Choose a reason for hiding this comment

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

This looks like a good call, but would like to get this addressed at its root as well. I'll ping you with details on getting admin access to the site.

@mattsherman
Copy link
Contributor

mattsherman commented May 19, 2022

@joelclimbsthings What's the status of this PR?

@joelclimbsthings
Copy link
Contributor Author

joelclimbsthings commented May 19, 2022

What's the status of this PR?

@mattsherman I had been prioritizing the task chaining side panel PR, but I was also able to gain access to the https://woocommercecore.mystagingwebsite.com to replace this image at the source. Looking at it now.

@github-actions github-actions bot added the package: @woocommerce/e2e-core-tests Issues related to @woocommerce/e2e-core-tests package. label May 19, 2022
@joelclimbsthings
Copy link
Contributor Author

joelclimbsthings commented May 19, 2022 •
edited
Loading

@joshuatf Alright, I've updated the image on the staging site, and updated all references to the new image. I also tested on a fresh site an fresh clone of this branch, and the correct image was downloaded without issue. Have at it!

Edit: The diff is a bit hard to read, but the only change(s) regarding the image is the date in the path.

@joelclimbsthings joelclimbsthings requested a review from joshuatf May 19, 2022 23:28
@botwoo
Copy link
Collaborator

botwoo commented May 19, 2022

📊 Test reports for this pull request have been published and are accessible through the following links:

Latest commit referenced in the reports: Updating references to old album-1 image 9e62fb5
This comment will automatically be updated with the latest referenced commit when you push new changes to this pull request.

Visit the WooCommerce Test Reports homepage to view all published reports. See the FAQs page if you're having problems accessing them.

joshuatf
joshuatf approved these changes May 24, 2022
View reviewed changes
Copy link
Contributor

@joshuatf joshuatf left a comment

Choose a reason for hiding this comment

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

Testing well, thanks for the fix, @joelclimbsthings!

@joelclimbsthings joelclimbsthings merged commit 5f56ecb into trunk May 24, 2022
@joelclimbsthings joelclimbsthings deleted the fix/32137 branch May 24, 2022 18:12
@github-actions github-actions bot added this to the 6.7.0 milestone May 24, 2022
@github-actions
Copy link
Contributor

github-actions bot commented May 24, 2022

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

  • Add the release: add testing instructions label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@joshuatf joshuatf joshuatf approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@joelclimbsthings joelclimbsthings

Labels

package: @woocommerce/e2e-core-tests Issues related to @woocommerce/e2e-core-tests package. plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Milestone

6.7.0

Development

Successfully merging this pull request may close these issues.

PHP Warning when adding a Digital product from template

5 participants

@joelclimbsthings @mattsherman @botwoo @joshuatf