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.

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

Magento should catch getimagesize() issues #943

Open
loekvangool opened this issue May 1, 2020 · 1 comment
Open

Magento should catch getimagesize() issues #943

loekvangool opened this issue May 1, 2020 · 1 comment
Labels
needs investigation waiting for feedback

Comments

@loekvangool
Copy link
Contributor

loekvangool commented May 1, 2020 •
edited
Loading

If getimagesize() runs into trouble, a NOTICE is thrown and code execution continues, no biggie. But, Magento assumes the operation went fine. But we all know that images can go missing or get corrupted.

There are 14 usages of getimagesize() at this time: https://github.com/OpenMage/magento-lts/search?q=getimagesize&unscoped_q=getimagesize. Some are handled, some are surpressed (should @ be abolished in the codebase?), some are unhandled.

Magento should handle these situations more gracefully IMO.
@colinmollenhour
Copy link
Member

Agreed, ideally each instance should be checking the return value to ensure it is not FALSE and then throwing an error early if so, like "Could not get image size for provided file.". In developer mode these errors are thrown as exceptions so it would only have an effect in production mode.

colinmollenhour pushed a commit that referenced this issue Jun 26, 2020
@rvelhote
Cache all attribute options for display in layered navigation #943 (#942
19648fd 
)

* Cache all attribute options for display in layered navigation
This will improve performance by reducing the amount of SQL Queries when fetching all possible attributes to display in the layered navigation

* Use the attribute's own cache tags

Co-authored-by: Ricardo Velhote <ricardo@caretobeauty.com>
edannenberg pushed a commit to edannenberg/magento-lts that referenced this issue Aug 20, 2020
@rvelhote @edannenberg
[perf-OpenMage#943] Cache attribute options used in layered navigation
5b90a66 
This will improve performance by reducing the amount of SQL Queries when
fetching all possible attributes to display in the layered navigation
edannenberg pushed a commit to edannenberg/magento-lts that referenced this issue Aug 24, 2020
@rvelhote @edannenberg
[perf-OpenMage#943] Cache attribute options used in layered navigation
3b3e5aa 
This will improve performance by reducing the amount of SQL Queries when
fetching all possible attributes to display in the layered navigation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs investigation waiting for feedback
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@colinmollenhour @loekvangool @addison74