Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 72 additions & 19 deletions php/class-delivery.php
Original file line number Diff line number Diff line change
Expand Up @@ -1583,9 +1583,10 @@ public function parse_element( $element, $content = '' ) {
// Track back the found URL.
if ( $this->media->is_cloudinary_url( $raw_url ) ) {
$public_id = $this->media->get_public_id_from_url( $raw_url );
if ( isset( $this->known[ $public_id ] ) ) {

$url = $this->known[ $public_id ]['sized_url'];
// A URL with transformations of its own (e.g. typed into content by hand) is left as is, since rebuilding it would drop them.
if ( isset( $this->known[ $public_id ] ) && ! $this->has_custom_transformations( $raw_url, $this->known[ $public_id ]['post_id'] ) ) {
// The relation's sized_url is relative to the content dir, while the URL keys of $this->known are full URLs.
$url = $this->get_searchable_url( $this->known[ $public_id ] );

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Non-blocking question: This fix also makes the lookup match Cloudinary URLs that are typed into post content by hand. Before, a hard-coded Cloudinary <img> with a synced public ID and no wp-image-{id} class never matched here, so the tag stayed as it was. Now it matches, and rebuild_tag() replaces its src with cloudinary_url().

parse_element() only keeps transformations from the attachment, from cld_params, and from cld_overwrite. Transformations in the URL path (for example e_grayscale or a manual crop) are not read, so the rebuilt URL can lose them.

This is how the 2021 code was meant to work, and tags with the wp-image-{id} class already behave this way, so I don't think this blocks the PR. Can you confirm that your QA of hard-coded Cloudinary URLs included one with path transformations and no wp-image-{id} class? If they are lost, we can handle it in a follow-up, because it also affects tags with the class.

$attributes['src'] = $url;
}
}
Expand Down Expand Up @@ -1837,25 +1838,54 @@ public function validate_url( $url ) {
}

/**
* Set url usability.
* Check if a Cloudinary URL has transformations other than the ones the plugin applies to the attachment itself:
* its size, and the attachment's own and global transformations.
*
* @param array $item The item result.
* @param null|bool $auto_sync If auto_sync is on.
* @param string $url The Cloudinary URL.
* @param int $attachment_id The attachment ID.
*
* @return bool
*/
protected function set_usability( $item, $auto_sync = null ) {
protected function has_custom_transformations( $url, $attachment_id ) {
$type = $this->media->get_resource_type( $attachment_id );
$expected = $this->media->get_transformations_from_string( (string) $this->media->cloudinary_url( $attachment_id ), $type );

foreach ( $this->media->get_transformations_from_string( $url, $type ) as $transformation ) {
// Loose comparison, so the order of the parameters doesn't matter.
if ( ! $this->is_size_transformation( $transformation ) && ! in_array( $transformation, $expected ) ) { // phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict
return true;
}
}

/**
* Filter the found item to allow usability to be altered.
*
* @hook cloudinary_set_usable_asset
* @since 3.0.2
*
* @param array $item The found asset.
*
* @return array
*/
$item = apply_filters( 'cloudinary_set_usable_asset', $item );
return false;
}

/**
* Check if a transformation is one the plugin uses to size an image, e.g. `w_300,h_200,c_scale` or `w_150,h_150,c_thumb,g_auto`.
*
* @param array $transformation The transformation, as parsed by Media::get_transformations_from_string().
*
* @return bool
*/
protected function is_size_transformation( $transformation ) {
if ( array_diff_key( $transformation, array_flip( array( 'width', 'height', 'crop', 'gravity' ) ) ) ) {
return false;
}
if ( isset( $transformation['crop'] ) && ! in_array( $transformation['crop'], array( 'scale', 'fill', 'thumb' ), true ) ) {
return false;
}

return ! isset( $transformation['gravity'] ) || 'auto' === $transformation['gravity'];
}

/**
* Get the URL an item is known by, as used for the keys of $this->known.
*
* @param array $item The item result.
*
* @return string
*/
protected function get_searchable_url( $item ) {
/**
* The URL to be searched for and prepared to be delivered by Cloudinary.
*
Expand Down Expand Up @@ -1887,7 +1917,30 @@ protected function set_usability( $item, $auto_sync = null ) {
*
* @return string
*/
$url = apply_filters( 'cloudinary_delivery_searchable_url', $url, $item, $content_url );
return apply_filters( 'cloudinary_delivery_searchable_url', $url, $item, $content_url );
}

/**
* Set url usability.
*
* @param array $item The item result.
* @param null|bool $auto_sync If auto_sync is on.
*/
protected function set_usability( $item, $auto_sync = null ) {

/**
* Filter the found item to allow usability to be altered.
*
* @hook cloudinary_set_usable_asset
* @since 3.0.2
*
* @param array $item The found asset.
*
* @return array
*/
$item = apply_filters( 'cloudinary_set_usable_asset', $item );

$url = $this->get_searchable_url( $item );

$found = array();

Expand Down
220 changes: 220 additions & 0 deletions tests/phpunit/tests/test-delivery-parse-element.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
<?php
/**
* Tests for Cloudinary\Delivery::parse_element().
*
* Since WPP-1183, image_downsize() and wp_get_attachment_image_src() return Cloudinary URLs on
* the front end too, so tags built by theme code (wp_get_attachment_image() in a template, a
* shortcode, a widget) reach Delivery with a Cloudinary src instead of a local one. Delivery
* tracks such a src back to its attachment through the public ID; this covers that the track-back
* resolves to the same full URL the known items are keyed by, rather than the relation's
* content-relative sized_url, which never matched and left the tag unprocessed (no lazy loading,
* responsive breakpoints or SEO attributes). A src with transformations of its own (e.g. typed
* into content by hand) is left alone, as rebuilding it from the attachment would drop them.
*
* The known items are seeded directly rather than driven through prepare_delivery() and the
* relationship table, so no Cloudinary account is needed.
*
* @package Cloudinary
*/

/**
* Covers Delivery::parse_element().
*/
class Test_Delivery_Parse_Element extends WP_UnitTestCase {

/**
* The Cloudinary base URL used for the test media component.
*
* @var string
*/
const BASE_URL = 'https://res.cloudinary.com/test-cloud';

/**
* A real attachment, so the tracked-back item points at an existing post.
*
* @var int
*/
protected static $attachment_id;

/**
* Create a real attachment once for all tests.
*
* @param WP_UnitTest_Factory $factory The test factory.
*
* @return void
*/
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
self::$attachment_id = $factory->attachment->create_upload_object( DIR_TESTDATA . '/images/canola.jpg' );
}

/**
* Build a Delivery instance using a media component pointed at the test Cloudinary domain.
*
* @return Test_Delivery_Parse_Element_Delivery
*/
protected function get_delivery() {
$plugin = \Cloudinary\get_plugin_instance();
$media = new Test_Delivery_Parse_Element_Media( $plugin );
$media->base_url = self::BASE_URL;
$media->filter = new \Cloudinary\Media\Filter( $media );
$media->stub_cloudinary_url = self::BASE_URL . '/images/f_auto,q_auto/v1/canola/canola.jpg';

$delivery = new Test_Delivery_Parse_Element_Delivery( $plugin );
$delivery->set_media( $media );

return $delivery;
}

/**
* Seed the known items the same way Delivery::set_usability() keys them: by public ID, and
* by the full URL built from the content URL and the relation's content-relative sized_url.
*
* @param Test_Delivery_Parse_Element_Delivery $delivery The delivery instance.
*
* @return string The full URL the item is known by.
*/
protected function seed_known( $delivery ) {
$file = get_post_meta( self::$attachment_id, '_wp_attached_file', true );
$item = array(
'post_id' => self::$attachment_id,
'public_id' => 'canola',
'sized_url' => '/uploads/' . $file,
'sync_type' => 'media',
'format' => 'jpg',
'width' => 640,
'height' => 480,
'post_state' => 'enable',
);
$url = \Cloudinary\Utils::clean_url( content_url() ) . $item['sized_url'];

$delivery->known['canola'] = $item;
$delivery->known[ $url ] = $item;

return $url;
}

/**
* An img tag with a Cloudinary src for a known public ID is tracked back to its attachment.
*
* @return void
*/
public function test_cloudinary_src_is_tracked_back_to_its_attachment() {
$delivery = $this->get_delivery();
$url = $this->seed_known( $delivery );

$element = $delivery->parse_element( '<img width="300" height="225" src="' . self::BASE_URL . '/images/w_300,h_225,c_scale/f_auto,q_auto/v1/canola/canola.jpg?_i=AA" class="attachment-medium size-medium" alt="" />' );

$this->assertSame( self::$attachment_id, $element['id'] );
$this->assertSame( $url, $element['atts']['src'] );
$this->assertSame( 'canola.jpg', $element['atts']['data-public-id'] );
}

/**
* A Cloudinary src with the plugin's own thumbnail crop is still tracked back.
*
* @return void
*/
public function test_cloudinary_src_with_thumbnail_crop_is_tracked_back() {
$delivery = $this->get_delivery();
$this->seed_known( $delivery );

$element = $delivery->parse_element( '<img width="150" height="150" src="' . self::BASE_URL . '/images/w_150,h_150,c_thumb,g_auto/f_auto,q_auto/v1/canola/canola.jpg" alt="" />' );

$this->assertSame( self::$attachment_id, $element['id'] );
}

/**
* A Cloudinary src with transformations of its own, e.g. typed into content by hand, is left
* alone, since rebuilding it from the attachment would drop them.
*
* @dataProvider data_custom_transformations
*
* @param string $transformations The transformations in the src.
*
* @return void
*/
public function test_cloudinary_src_with_custom_transformations_is_not_tracked_back( $transformations ) {
$delivery = $this->get_delivery();
$this->seed_known( $delivery );

$src = self::BASE_URL . '/images/' . $transformations . '/v1/canola/canola.jpg';
$element = $delivery->parse_element( '<img src="' . $src . '" alt="" />' );

$this->assertSame( 0, $element['id'] );
$this->assertSame( $src, $element['atts']['src'] );
}

/**
* Transformations the plugin doesn't apply itself.
*
* @return array
*/
public function data_custom_transformations() {
return array(
'effect' => array( 'w_300,h_225,c_scale/e_sepia/f_auto,q_auto' ),
'manual crop' => array( 'c_crop,w_400,h_400,x_100,y_100/e_grayscale' ),
'gravity' => array( 'w_300,h_300,c_fill,g_face' ),
);
}

/**
* A Cloudinary src for a public ID Delivery doesn't know is left alone.
*
* @return void
*/
public function test_unknown_cloudinary_src_is_not_tracked_back() {
$delivery = $this->get_delivery();
$this->seed_known( $delivery );

$src = self::BASE_URL . '/images/w_300,h_225,c_scale/v1/other/other.jpg';
$element = $delivery->parse_element( '<img width="300" height="225" src="' . $src . '" alt="" />' );

$this->assertSame( 0, $element['id'] );
$this->assertSame( $src, $element['atts']['src'] );
}
}

/**
* Media with cloudinary_url() stubbed, to avoid the real sync/signature pipeline.
*/
class Test_Delivery_Parse_Element_Media extends \Cloudinary\Media {

/**
* Canned cloudinary_url() return value.
*
* @var string|false
*/
public $stub_cloudinary_url = false;

/**
* Stubbed to avoid the real sync/signature pipeline.
*
* @param int $attachment_id The attachment ID.
* @param array|string $size The requested size.
* @param array $transformations Transformations to apply.
* @param string|null $cloudinary_id A forced Cloudinary ID.
* @param bool $overwrite_transformations Whether to overwrite transformations.
*
* @return string|false
*/
public function cloudinary_url( $attachment_id, $size = array(), $transformations = array(), $cloudinary_id = null, $overwrite_transformations = false ) {
return $this->stub_cloudinary_url;
}
}

/**
* Delivery with a settable media component, since init() only wires the real one once connected.
*/
class Test_Delivery_Parse_Element_Delivery extends \Cloudinary\Delivery {

/**
* Set the media component.
*
* @param \Cloudinary\Media $media The media component.
*
* @return void
*/
public function set_media( $media ) {
$this->media = $media;
}
}
Loading