Automattic / Automattic/jetpack

Photon: strip_image_dimensions_maybe() method prone to fail due to file_exists()

Open
#2,472 2 comments 0 reactions 0 assignees View on GitHub
[Feature] Photon [Pri] Low Enhancement
Dominant language
PHP
Stars
1.8k
Forks
898
Avg merge
1d 18h
Merged PRs (30d)
774

Description

`Jetpack_Photon::strip_image_dimensions_maybe()` calls `file_exists()` without checking if the image is local and should be accessible on the filesystem: https://github.com/Automattic/jetpack/blob/5cd6e4452f23213f36475cf7429b449e1c64e6ae/class.photon.php#L523.

If the `file_exists()` check is necessary, the method should first check that the image was uploaded to the current WP installation. Even then, it's problematic because it assumes uploads are locally accessible, but that isn't necessarily the case. For sites hosted on distributed hosting platforms that don't store the images locally, sites using a plugin like [WP Offload S3](https://wordpress.org/plugins/amazon-s3-and-cloudfront/) to store media centrally/on a CDN, or sites using Core filters to change upload locations, the check will always fail.

Ideally, the method shouldn't use `file_exists()` at all, but if it must, there should be more care given to whether the image can be accessed locally, and the check should be gated by a filter so that users can disable it.

Is there a way to rework the `strip_image_dimensions_maybe()` method to work regardless of where the image is hosted?

Introduced to fix #73.

Contributor guide

Open the contributing guide

Research direction

Start in class.photon.php around strip_image_dimensions_maybe() and its file_exists() call. Trace how the method handles images from the current WordPress installation, distributed hosting, offloaded storage, and filtered upload locations. Done means the method no longer fails solely because an image is not locally accessible, with the intended behavior covered by appropriate tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.