News:

Want to get involved in developing SMF, then why not lend a hand on our github!

Main Menu

SMF 2.0.15 proxy.php

Started by butch2k, January 18, 2018, 12:59:25 PM

Previous topic - Next topic

butch2k

In function serve()


$response = $this->checkRequest();
if (!$response) {
// Throw a 404
header('HTTP/1.0 404 Not Found');
exit;
}
// Right, image not cached? Simply redirect, then.
if (!$response) {
header('Location: ' . $request, false, 301);
}


How the second if(!$response) could ever be evaluated if $response is false since we exit the script in the test just above ?

butch2k

#1
The whole caching logic in 2.0.15 seems very very fishy.

Let's have a look at cacheImage()

return file_put_contents($dest, json_encode(array(
'content_type' => $headers['content-type'],
'size' => $response['size'],
'time' => time(),
'body' => base64_encode($response['body']),
))) === false ? 1 : null;;


file_put_content evaluate to false when there is an error writing file, else it returns bytes written.

So cacheImage() returns
- false if there are any errors in url or
- return 1 if there is an error writing file or
- null if everything goes fine.

Now let's have a look to checkRequest()

// Attempt to cache the request if it doesn't exist
if (!$this->isCached($request)) {
return $this->cacheImage($request);
}


So if we get up to the point where we attempt to cache image checkrequest() return either false or 1 (failure to write) or in case everything goes fine up to the writing of file to disk it returns null

up one level to serve();

$response = $this->checkRequest();
if ($response) {
// Throw a 404
header('HTTP/1.0 404 Not Found');
exit;
}
// Right, image not cached? Simply redirect, then.
if (!$response) {
header('Location: ' . $request, false, 301);
}

!$response evaluate to true if checkRequest() is either false (that's ok) and NULL !!! (that is not ok at all as this will 404 on the first image load)

Shouldn't it read


if ($response === false) {//content-type is no image or not HTTP 200... so 404
// Throw a 404
header('HTTP/1.0 404 Not Found');
exit;
}
// Right, image not cached? Simply redirect, then.
if ($response === 1) {// ok it's an image original server sent a 200... but we did not manage to cache it so redirect to original server
header('Location: ' . $request, false, 301);
}



Am i out of my mind or is there a serious logic issue in the current proxy ?

[edit]
My logic does not cover up the other cases in checkrequest() where it returns false because it's not activated for instance. It should not 404 in that case but 301 to the original file.

butch2k

Other issue...
I would add some Cache-control, Expires and Last-Modified headers (based on the original ones maybe ?) else file will get reloaded each time a user goes to the page.

butch2k

Another one.
I've just noticed that the proxy.php file in 2.0.15 while being different from the one in the 2.0.14 package is still branded as 2.0.14.

butch2k

Here is my take which distinguish hard failures (404, not image, etc...) from soft ones (file too large, no cache enabled, etc...)
Moreover it add some cache headers (to be modified for less agressive caching)



<?php

/**
 * This is a lightweight proxy for serving images, generally meant to be used alongside SSL
 *
 * Simple Machines Forum (SMF)
 *
 * @package SMF
 * @author Simple Machines http://www.simplemachines.org
 * @copyright 2016 Simple Machines and individual contributors
 * @license http://www.simplemachines.org/about/smf/license.php BSD
 *
 * @version 2.0.15b
 */

define('SMF''proxy');

/**
 * Class ProxyServer
 */
class ProxyServer
{
/** @var bool $enabled Whether or not this is enabled */
protected $enabled;

/** @var int $maxSize The maximum size for files to cache */
protected $maxSize;

/** @var string $secret A secret code used for hashing */
protected $secret;

/** @var string The cache directory */
protected $cache;


/**
 * Constructor, loads up the Settings for the proxy
 *
 * @access public
 */
public function __construct()
{
global $image_proxy_enabled$image_proxy_maxsize$image_proxy_secret$cachedir$sourcedir;

require_once(dirname(__FILE__) . '/Settings.php');
require_once($sourcedir '/Class-CurlFetchWeb.php');

// Turn off all error reporting; any extra junk makes for an invalid image.
error_reporting(0);

$this->enabled = (bool) $image_proxy_enabled;
$this->maxSize = (int) $image_proxy_maxsize;
$this->secret = (string) $image_proxy_secret;
$this->cache $cachedir '/images';
}

/**
 * Checks whether the request is valid or not
 *
 * @access public
 * @return bool Whether the request is valid
 */
public function checkRequest()
{
if (!$this->enabled)
return false;

// Try to create the image cache directory if it doesn't exist
if (!file_exists($this->cache))
if (!mkdir($this->cache) || !copy(dirname($this->cache) . '/index.php'$this->cache '/index.php'))
return false;

if (empty($_GET['hash']) || empty($_GET['request']) || ($_GET['request'] === "http:") || ($_GET['request'] === "https:"))
return false;

$hash $_GET['hash'];
$request $_GET['request'];

if (md5($request $this->secret) != $hash)
return false;

// Attempt to cache the request if it doesn't exist
if (!$this->isCached($request)) {
return $this->cacheImage($request);
}

return true;
}


/**
 * Serves the request
 *
 * @access public
 * @return void
 */
public function serve()
{
$request $_GET['request'];
$cached_file $this->getCachedPath($request);
$cached json_decode(file_get_contents($cached_file), true);

// Did we get an error when trying to fetch the image
$response $this->checkRequest();
if ($response === -1) {//No image on the other side, just 404
// Throw a 404
header('HTTP/1.0 404 Not Found');
exit;
}
// Right, image not cached? Simply redirect, then.
if ($response === false) {// not able to cache for some reason just redirect then
header('Location: ' $requestfalse301);
}

// Is the cache expired?
if (!$cached || time() - $cached['time'] > (86400))
{
@unlink($cached_file);
if ($this->checkRequest())
$this->serve();
exit;
}

// Make sure we're serving an image
$contentParts explode('/', !empty($cached['content_type']) ? $cached['content_type'] : '');
if ($contentParts[0] != 'image')
exit;

header('Content-type: ' $cached['content_type']);
header('Content-length: ' $cached['size']);
// adding some caching 
header('Cache-control: public'); // can be made private
header('Expires: ' gmdate('D, d M Y H:i:s'time() + 525600 60) . ' GMT');// in a time far far away
header('Last-Modified: ' gmdate('D, d M Y H:i:s'filemtime($cached_file)) . ' GMT');//since when have you been touched ?
echo base64_decode($cached['body']);
}

/**
 * Returns the request's hashed filepath
 *
 * @access public
 * @param string $request The request to get the path for
 * @return string The hashed filepath for the specified request
 */
protected function getCachedPath($request)
{
return $this->cache '/' sha1($request $this->secret);
}

/**
 * Check whether the image exists in local cache or not
 *
 * @access protected
 * @param string $request The image to check for in the cache
 * @return bool Whether or not the requested image is cached
 */
protected function isCached($request)
{
return file_exists($this->getCachedPath($request));
}

/**
 * Attempts to cache the image while validating it
 *
 * @access protected
 * @param string $request The image to cache/validate
 * @return bool|int Whether the specified image was cached or error code when accessing
 */
protected function cacheImage($request)
{
$dest $this->getCachedPath($request);
$curl = new curl_fetch_web_data(array(CURLOPT_BINARYTRANSFER => 1));
$request $curl->get_url_data($request);
$responseCode $request->result('code');
$response $request->result();

if (empty($response)) {
return -1;//got nothing ughhh !
}

if ($responseCode != 200) {
return -1;//no good mate
}

$headers $response['headers'];

// Make sure the url is returning an image
$contentParts explode('/', !empty($headers['content-type']) ? $headers['content-type'] : '');
if ($contentParts[0] != 'image')
return -1;//no image in there

// Validate the filesize
if ($response['size'] > ($this->maxSize 1024))
return false;//too big for caching

return file_put_contents($destjson_encode(array(
'content_type' => $headers['content-type'],
'size' => $response['size'],
'time' => time(),
'body' => base64_encode($response['body']),
))); //false if write failed else number of bytes written
}
}

$proxy = new ProxyServer();
$proxy->serve();




bjornforum

This seems to work! Only bug/limitation in this code is that disabling the cache results in no padlock at the first load. After refreshing the page the padlock appears. However, for the first time for my site, caching seems to work with this version of the script, so I simply enabled the cache :)

Thanks!

albertlast

i try to transfer you idea to smf2.1 code base,
result your find here: https://github.com/SimpleMachines/SMF2.1/pull/4550
One the question on your code i got.
Why the check of request http and https?

if (empty($_GET['hash']) || empty($_GET['request']) || ($_GET['request'] === "http:") || ($_GET['request'] === "https:"))

Btw the code should work also in smf 2.0.x

butch2k

IIRC it was to prevent issues with things like {img]http:[/img}, i don't remember exactly why i needed it but i remember encountering an edge case in my very modified forum which required this extra check.

epikurieu

Hi, I have the same problem of images not showing on my forum after installing an SSL certificate.
I read quite extensively all threads on the subject and tried all solutions and patches that were submitted, but none of them worked.
So I switched back to the file included in 2.0.15 upgrade pack, and disabled image proxy until I find a solution.

The forum is here : https://www.neuronesconnection.fr/

Here is an example of an url that is called when the image proxy is enabled : https://www.neuronesconnection.fr/proxy.php?request=http%3A%2F%2Fwww.neuronesconnection.fr%2Fimages%2Fjeux%2Fcine%2F060.jpg&hash=30312a1598c442c739750f80b1d43464
It seems to be an encoding issue, am I wrong ?

Illori


shawnb61

We believe these proxy issues (not due to host config) were all fixed in 2.0.16.
Address the process rather than the outcome.  Then, the outcome becomes more likely.   - Fripp

Advertisement: