Not a bug Consistency: URL scheme check in class ImageProxy

AlexT

AlexT

Well-known member
Affected version
2.0 RC2
Update: Please ignore. I would have thought for parse_url to normalize the url, i.e. to lower-case everything. But it doesn't. :( So your choice of using a regex was a good one after all.



In fetchImageDataFromUrl(), you have the following code:

PHP: 
125                 $urlParts = parse_url($url);
126
127                 if (!preg_match('#^https?://#i', $url))
128                 {
129                         throw new \InvalidArgumentException("URL must be http or https");
130                 }

Not a bug, but perhaps not the best in terms of consistency/readability. ;) Since you have $url parsed in $urlParts, you can skip the regular expression in line 127 and instead check for the content in $urlParts['scheme'].

PHP: 
if ($urlParts['scheme'] != 'http' && $urlParts['scheme'] != 'https')
 
Last edited:
It would be possible to do a strtolower and use the parsed version, though I do feel that the regex URL check is quite readable and it's something we do in a number of other places, so I'm not going to make a huge change here, though I am going to run the regex before parsing the URL.
 
Top Bottom