Skip to content

Commit

Permalink
fix: compatibility with images which contains query arguments, causin…
Browse files Browse the repository at this point in the history
…g broken image urls
  • Loading branch information
selul committed Mar 4, 2019
1 parent b33dbca commit 56108be
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
4 changes: 2 additions & 2 deletions inc/manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ public function process_urls_from_json( $json ) {
* @return array array of urls.
*/
public function extract_urls_from_json( $content ) {
$regex = '/(?<!(=|\\\\)(?:"|\'|"))(?:http(?:s?):)(?:[\/\\\\|.|\w|\s|-])*\.(?:' . implode( '|', array_keys( Optml_Config::$extensions ) ) . ')/';
$regex = '/(?<!(=|\\\\)(?:"|\'|"))(?:http(?:s?):)(?:[\/\\\\|.|\w|\s|-])*\.(?:' . implode( '|', array_keys( Optml_Config::$extensions ) ) . ')(?:\??[\w|=|&|\-|\.|:]*)/';
preg_match_all(
$regex,
$content,
Expand Down Expand Up @@ -392,7 +392,7 @@ public function process_urls_from_content( $html ) {
* @return array
*/
public function extract_image_urls_from_content( $content ) {
$regex = '/(?:http(?:s?):)(?:[\/\\\\|.|\w|\s|-])*\.(?:' . implode( '|', array_keys( Optml_Config::$extensions ) ) . ')/';
$regex = '/(?:http(?:s?):)(?:[\/\\\\|.|\w|\s|-])*\.(?:' . implode( '|', array_keys( Optml_Config::$extensions ) ) . ')(?:\??[\w|=|&|\-|\.|:]*)/';
preg_match_all(
$regex,
$content,
Expand Down
3 changes: 3 additions & 0 deletions inc/url_replacer.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ public function build_image_url(
if ( ! $this->can_replace_url( $url ) ) {
return $url;
}
// Remove any query strings that might affect conversion.
$url = strtok( $url, '?' );

if ( ! $this->is_valid_mimetype_from_url( $url ) ) {
return $url;
}
Expand Down
13 changes: 13 additions & 0 deletions tests/test-replacer.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,19 @@ public function test_replacement_non_whitelisted_urls() {
$this->assertContains( 'https://www.codeinwp.org', $replaced_content );
}

public function test_replacement_remove_query_arg() {
$content = '<div class="before-footer">
<div class="codeinwp-container">
<p class="featuredon">Featured On</p>
<img src="https://www.example.org/wp-content/uploads/2018/05/brands.png?param=123">
</div>
</div>';
$replaced_content = Optml_Manager::instance()->replace_content( $content );

$this->assertContains( 'i.optimole.com', $replaced_content );
$this->assertNotContains( '?param=123', $replaced_content );
}

// TODO We need to extend this to single url replacement. If we make the url extractor regex with option scheme, the parsing will take huge amount of time. We need to think alternatives.
public function test_replacement_without_scheme() {
$content = '<div class="before-footer">
Expand Down

0 comments on commit 56108be

Please sign in to comment.