Serve rescaled jpg images on demand with myphoto!500.jpg syntax (here 500px-width rescaled)

Question

On web pages, obviously large photos (e.g. 4000x4000px) cannot be served with a reliance on CSS to do the resizing: it would be too slow for end users and too bandwidth-consuming for the server.

So it's important to rescale photos to the desired size that will be actually used on the website. For this reason I usually resized the image with a tool like Photoshop to, say 800px width. But then later you always have the case "I would finally have preferred 10000px instead! and then ... you have to export the JPG again, re-upload, etc. Annoying!

Instead here is an automated solution in which you upload the top-quality version (never served to end users because it would be too big), and the tool automatically resizes on-demand.

Here is code that:

• serves myphoto.jpg if present in current folder
• serves myphoto!500.jpg if present in current folder
• creates myphoto!500.jpg by rescaling myphoto.jpg with a width of 500px if it is not done yet. So as it will be saved/cached to a file, next requests will be fast because the server will just serve the pre-cooked file

.htaccess

RewriteEngine On
RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ index.php [QSA,L]

index.php

<?php
$req = urldecode(basename($_SERVER['REQUEST_URI']));
list($f, $size) = explode('!', $req);
list($size, $ext) = explode('.', $size);
$size = intval($size);
if (($size > 2000) || ($size % 50 != 0))
    die();
$f .= '.' . $ext;
list($width, $height) = getimagesize($f);
$src = imagecreatefromstring(file_get_contents($f));
$dst = imagescale($src, $size);
imagejpeg($dst, $req);
?>

Question for code review: is there something to improve in terms of security?

Note: To avoid the scenario that someone requests myimage!1.jpg, myimage!2.jpg, ..., myimage!1999.jpg, myimage!2000.jpg in loop, filling the disk for nothing, the code only accepts size as integer multiples of 50. At most 50 100 150 ... 1900 1950 2000 will be written, i.e. 40 versions.

Answer 1

Analysis

The code looks acceptable; it is not difficult to read. The description does not state which image formats should be supported however the code appears to output resized images as JPEG images. It may be wise to maintain the image format - e.g. GIF, PNG, JPG, etc.

Suggestions

Consider using pattern matching to validate the request URI

A regular expression could be used to both ensure the request URI matches the expected format and also parse out the components. While it may be slightly slower the difference should be negligible. One could use preg_match() or if Unicode string support is needed then mb_eregi() could be used. A pattern could be crafted to ensure:

• there is not more than one exclamation mark
• The number that follows the exclamation mark is positive and has a number of digits within a certain range (e.g. 1-5)
• The extension is within the acceptable list of extensions

Doing this should allow removal of the multiple calls to split the string using explode(). For example:

preg_match('#^([^!\./]+)!(\d+)\.(png|jpg|gif)#i', $req, $matches);
if (!$matches) {
    die();
}
[$uri, $name, $size, $ext] = $matches;

• ^ asserts position at start of the string
• 1st Capturing Group ([^!\./]+) Match a single character not present in the list below [^!\./] + matches the previous token between one and unlimited times, as many times as possible, giving back as needed (greedy)
  • Could be changed to ([a-z0-9]+) to only allow alphanumeric characters or (\w+) to allow [A-Za-z0-9_]
• ! matches the character ! with index 3310 (2116 or 418) literally (case insensitive)
• 2nd Capturing Group (\d+)
  • \d matches a digit (equivalent to [0-9])
  • + matches the previous token between one and unlimited times, as many times as possible, giving back as needed (greedy)
• \. matches the character . with index 4610 (2E16 or 568) literally (case insensitive)
• 3rd Capturing Group (png|jpg|gif)
  • 1st Alternative png png matches the characters png literally (case insensitive)
  • 2nd Alternative jpg jpg matches the characters jpg literally (case insensitive)
  • 3rd Alternative gif gif matches the characters gif literally (case insensitive)
• Global pattern flags
  • i modifier: insensitive. Case insensitive match (ignores case of [a-zA-Z])

See the regular expression pattern explained here.

Named sub-patterns can be named to allow the array of matches to have keys that are easier to work with.

For example:

preg_match('#^(?P<name>[^!\./]+)!(?P<size>\d+)\.(?P<ext>png|jpg|gif)#i', $req, $matches);

Here are the parts of the pattern explained:

• opening delimiter: #

• Start of line anchor: ^

• match one or more chars for file name- anything that isn’t an exclamation mark, dot or slash: (?P<name>[^!\./]+)

  Could be changed to (?P<name>[a-z0-9]+) to only allow those characters

• the exclamation mark: !

• The size - digits only (?P<size>\d+)

• Match a literal dot \.

• The extension (?P<ext>png|jpg|gif)

• closing delimiter: #

• Case insensitive modifier flag i

If the pattern is matched then the array $matches would look like this:

array ( 
    0 => 'myphoto!500.jpg', 
    'name' => 'myphoto', 
    1 => 'myphoto', 
    'size' => '500', 
    2 => '500', 
    'extension' => 'jpg', 
    3 => 'jpg', 
)

Simplify variable assignment using Array Destructuring syntax

Like was used in the previous section, As of PHP 7.1 array assignment can be used to destructure arrays¹. Instead of using list() when exploding strings like the request into two parts, a simple array can be used. This may not save much processing time since list() is just a language construct² but it is simpler to type.

So instead of these lines:

list($f, $size) = explode('!', $req);
list($size, $ext) = explode('.', $size);

Those could be simplified to:

[$f, $size] = explode('!', $req);
[$size, $ext] = explode('.', $size);

Remove unused variables

The variables declared here:

list($width, $height) = getimagesize($f);

appear to be unused. While the memory allocation for those may not be significant, those can be eliminated to save resources and make the code simpler.

Updated code

I created a repository with a docker file and container file so I could test it on http://localhost:8082. I also updated the code to utilize the suggestions above and output the image to the browser.

<?php
$req = urldecode(basename($_SERVER['REQUEST_URI']));
preg_match('#^(\w+)!(\d+)\.(png|jpg|gif)#i', $req, $matches);
if (!$matches) {
    die();
}
[, $name, $size, $ext] = $matches;
$size = intval($size);
if (($size > 2000) || ($size % 50 != 0))
    die();
$src = imagecreatefromstring(file_get_contents($name . '.' . $ext));
$dst = imagescale($src, $size);
$function = 'image' . ($ext == 'jpg' ? 'jpeg' : strtolower($ext));
header('Content-Type: image/' . strtolower($ext));
$function($dst, $req);
$function($dst);    //send the image to the browser

Answer 2

If this were my code I'd be totally distrustful of the URI presented by my user. In other words I would assume that the results of the explode operations $f, $size, and $ext contained malicious garbage.

I would wrap the whole thing in a try{} block. I would check the values of $size and $ext against explicit allowlists. I would examine the path in $f to rigorously reject stuff like ../ and other file system relative navigation operations. I would restrict the character set allowed in each level of the path to, probably to numbers and ASCII letters, excluding anything else.

I would check files before opening them, for existence, for having a byte size less than some arbitrary threshold (no 70MiB jpg files, thank you very much). I'd also check whether the dimensions were reasonable (no 10k x 50k pngs, please).

If any of these checks failed, I would respond "sorry" to the (untrusted) user without giving the slightest detail about which check failed.

You're not paranoid when you believe highly motivated cybercriminals are plotting against you. They are plotting against you.