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

Image CDN: URL encode path parts #39560

Merged
merged 7 commits into from
Oct 8, 2024
+41 −2
Merged

Image CDN: URL encode path parts #39560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
56f26f3
rawurlencode each path part in the image URL
haqadn Sep 27, 2024
7c3c441
Fix a typo
haqadn Sep 27, 2024
a48ad17
Add a test
haqadn Sep 27, 2024
1e7f606
changelog
haqadn Sep 27, 2024
f38137b
Merge branch 'trunk' into fix/photon-url-encoding
dilirity Oct 7, 2024
98d10b4
Add comment to clarify space usage
dilirity Oct 7, 2024
e9ce2c4
Merge branch 'trunk' into fix/photon-url-encoding
dilirity Oct 8, 2024
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
4 changes: 4 additions & 0 deletions projects/packages/image-cdn/changelog/fix-photon-url-encoding
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

URL encode path parts of an image
19 changes: 17 additions & 2 deletions projects/packages/image-cdn/src/class-image-cdn-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,7 @@ public static function cdn_url( $image_url, $args = array(), $scheme = null ) {
}
}

$image_host_path = $image_url_parts['host'] . $image_url_parts['path'];

$image_host_path = $image_url_parts['host'] . static::escape_path( $image_url_parts['path'] );
/**
* Filters the domain used by the Photon module.
*
Expand Down Expand Up @@ -262,6 +261,22 @@ public static function is_cdn_url( $url ) {
|| wp_parse_url( $custom_photon_url, PHP_URL_HOST ) === $parsed_url['host'];
}

/**
* URL-encodes each path component.
*
* Example:
* Input: "foo/bar baz/baz"
* Output: "foo/bar%20baz/baz"
*
* @param string $path The path to escape.
* @return string The escaped path.
*/
private static function escape_path( $path ) {
$parts = explode( '/', $path );
$parts = array_map( 'rawurlencode', $parts );
return implode( '/', $parts );
Comment on lines +275 to +277
Copy link
Member

Choose a reason for hiding this comment

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

Could we rely on urlencode_deep() here maybe, to simplify things?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

urlencode converts spaces to + which doesn't work with path parts in photon https://i0.wp.com/boost.in.ngrok.io/wp-content/uploads/2024/09/Screenshot+2024-09-16-at-12.46.59%E2%80%AFPM.png

The purpose is served by rawurlencode https://i0.wp.com/boost.in.ngrok.io/wp-content/uploads/2024/09/Screenshot%202024-09-16-at-12.46.59%E2%80%AFPM.png

}

/**
* Parses WP.com-hosted image args to replicate the crop.
*
Expand Down
12 changes: 12 additions & 0 deletions projects/packages/image-cdn/tests/php/test_class.image_cdn_core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,18 @@ public function test_is_cdn_url_method() {
$this->assertFalse( Image_CDN_Core::is_cdn_url( '//example.com/img.jpg' ) );
}

/**
* @covers ::Image_CDN_Core::cdn_url
* @since $$next-version$$
* @group jetpack_photon_filter_url_encoding
*/
public function test_photon_url_filter_url_encodes_path_parts() {
// The first two spaces are not standard spaces - https://www.compart.com/en/unicode/U+202F
$url = Image_CDN_Core::cdn_url( '//example.com/narrow no-break space/name with spaces.jpg', array(), 'https' );

$this->assertEquals( 'https://i0.wp.com/example.com/narrow%E2%80%AFno-break%E2%80%AFspace/name%20with%20spaces.jpg', $url );
}

/**
* @author aduth
* @covers ::Image_CDN_Core::cdn_url_scheme
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/boost/changelog/fix-photon-url-encoding
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Image CDN: URL encode image path parts for RSS feed compatibility
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/fix-photon-url-encoding
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: compat

Image CDN: URL encode image path parts for RSS feed compatibility
Loading