Skip to content

Commit

Permalink
CRM-20821 - Use smarter logic to adjust img URL
Browse files Browse the repository at this point in the history
This change is the crux of CRM-20821.

Before this change, editing a premium product would automatically
break the image URL. After this change, premium products can be edited
while retaining the image URL. Functionality is preserved which changes
the URL to a local URL when possible.

This commit adds two new Utils functions (along with tests) to handle
the logic of changing a supplied URL to a local URL.
  • Loading branch information
seancolsen authored and monishdeb committed Jul 24, 2017
1 parent aa4ad33 commit 6c094ca
Show file tree
Hide file tree
Showing 3 changed files with 213 additions and 9 deletions.
12 changes: 3 additions & 9 deletions CRM/Contribute/BAO/ManagePremiums.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,9 @@ public static function add(&$params, &$ids) {
);
$params = array_merge($defaults, $params);

// CRM-14283 - strip protocol and domain from image URLs
$image_type = array('image', 'thumbnail');
foreach ($image_type as $key) {
if (isset($params[$key]) && $params[$key]) {
$parsedURL = explode('/', $params[$key]);
$pathComponents = array_slice($parsedURL, 3);
$params[$key] = '/' . implode('/', $pathComponents);
}
}
// Use local URLs for images when possible
$params['image'] = CRM_Utils_String::simplifyURL($params['image'], TRUE);
$params['thumbnail'] = CRM_Utils_String::simplifyURL($params['thumbnail'], TRUE);

// Save and return
$premium = new CRM_Contribute_DAO_Product();
Expand Down
74 changes: 74 additions & 0 deletions CRM/Utils/String.php
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,80 @@ public static function unstupifyUrl($htmlUrl) {
return str_replace('&', '&', $htmlUrl);
}

/**
* When a user supplies a URL (e.g. to an image), we'd like to:
* - Remove the protocol and domain name if the URL points to the current
* site.
* - Keep the domain name for remote URLs.
* - Optionally, force remote URLs to use https instead of http (which is
* useful for images)
*
* @param string $url
* The URL to simplify. Examples:
* "https://example.org/sites/default/files/coffee-mug.jpg"
* "sites/default/files/coffee-mug.jpg"
* "http://i.stack.imgur.com/9jb2ial01b.png"
* @param bool $forceHttps = FALSE
* If TRUE, ensure that remote URLs use https. If a URL with
* http is supplied, then we'll change it to https.
* This is useful for situations like showing a premium product on a
* contribution, because (as reported in CRM-14283) if the user gets a
* browser warning like "page contains insecure elements" on a contribution
* page, that's a very bad thing. Thus, even if changing http to https
* breaks the image, that's better than leaving http content in a
* contribution page.
*
* @return string
* The simplified URL. Examples:
* "/sites/default/files/coffee-mug.jpg"
* "https://i.stack.imgur.com/9jb2ial01b.png"
*/
public static function simplifyURL($url, $forceHttps = FALSE) {
$config = CRM_Core_Config::singleton();
$siteURLParts = self::simpleParseUrl($config->userFrameworkBaseURL);
$urlParts = self::simpleParseUrl($url);

// If the image is locally hosted, then only give the path to the image
$urlIsLocal
= ($urlParts['host+port'] == '')
| ($urlParts['host+port'] == $siteURLParts['host+port']);
if ($urlIsLocal) {
// and make sure it begins with one forward slash
return preg_replace('_^/*(?=.)_', '/', $urlParts['path+query']);
}

// If the URL is external, then keep the full URL as supplied
else {
return $forceHttps ? preg_replace('_^http://_', 'https://', $url) : $url;
}
}

/**
* A simplified version of PHP's parse_url() function.
*
* @param string $url
* e.g. "https://example.com:8000/foo/bar/?id=1#fragment"
*
* @return array
* Will always contain keys 'host+port' and 'path+query', even if they're
* empty strings. Example:
* [
* 'host+port' => "example.com:8000",
* 'path+query' => "/foo/bar/?id=1",
* ]
*/
public static function simpleParseUrl($url) {
$parts = parse_url($url);
$host = isset($parts['host']) ? $parts['host'] : '';
$port = isset($parts['port']) ? ':' . $parts['port'] : '';
$path = isset($parts['path']) ? $parts['path'] : '';
$query = isset($parts['query']) ? '?' . $parts['query'] : '';
return array(
'host+port' => "$host$port",
'path+query' => "$path$query",
);
}

/**
* Formats a string of attributes for insertion in an html tag.
*
Expand Down
136 changes: 136 additions & 0 deletions tests/phpunit/CRM/Utils/StringTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,4 +227,140 @@ public function testFilterByWildCards($patterns, $expectedResults) {
$this->assertEquals(array_merge($expectedResults, array('noise')), $actualResults);
}

/**
* CRM-20821
* CRM-14283
*
* @param string $imageURL
* @param book $forceHttps
* @param string $expected
*
* @dataProvider simplifyURLProvider
*/
public function testSimplifyURL($imageURL, $forceHttps, $expected) {
$this->assertEquals(
$expected,
CRM_Utils_String::simplifyURL($imageURL, $forceHttps)
);
}

/**
* Used for testNormalizeImageURL above
*
* @return array
*/
public function simplifyURLProvider() {

$config = CRM_Core_Config::singleton();
$localDomain = parse_url($config->userFrameworkBaseURL)['host'];
$externalDomain = 'example.org';

// Ensure that $externalDomain really is different from $localDomain
if ($externalDomain == $localDomain) {
$externalDomain = 'example.net';
}

return array(

'prototypical example' =>
array(
"https://$localDomain/sites/default/files/coffee-mug.jpg",
FALSE,
'/sites/default/files/coffee-mug.jpg',
),

'external domain with https' =>
array(
"https://$externalDomain/sites/default/files/coffee-mug.jpg",
FALSE,
"https://$externalDomain/sites/default/files/coffee-mug.jpg",
),

'external domain with http forced to https' =>
array(
"http://$externalDomain/sites/default/files/coffee-mug.jpg",
TRUE,
"https://$externalDomain/sites/default/files/coffee-mug.jpg",
),

'external domain with http not forced' =>
array(
"http://$externalDomain/sites/default/files/coffee-mug.jpg",
FALSE,
"http://$externalDomain/sites/default/files/coffee-mug.jpg",
),

'local URL' =>
array(
"/sites/default/files/coffee-mug.jpg",
FALSE,
"/sites/default/files/coffee-mug.jpg",
),

'local URL without a forward slash' =>
array(
"sites/default/files/coffee-mug.jpg",
FALSE,
"/sites/default/files/coffee-mug.jpg",
),

'empty input' =>
array(
'',
FALSE,
'',
),
);
}

/**
* @param string $url
* @param array $expected
*
* @dataProvider parseURLProvider
*/
public function testSimpleParseUrl($url, $expected) {
$this->assertEquals(
$expected,
CRM_Utils_String::simpleParseUrl($url)
);
}

/**
* Used for testSimpleParseUrl above
*
* @return array
*/
public function parseURLProvider() {
return array(

"prototypical example" =>
array(
"https://example.com:8000/foo/bar/?id=1#fragment",
array(
'host+port' => "example.com:8000",
'path+query' => "/foo/bar/?id=1",
),
),

"empty" =>
array(
"",
array(
'host+port' => "",
'path+query' => "",
),
),

"path only" =>
array(
"/foo/bar/image.png",
array(
'host+port' => "",
'path+query' => "/foo/bar/image.png",
),
),
);
}

}

0 comments on commit 6c094ca

Please sign in to comment.