Browse Source

Fix wrong getHeader refactoring (#2749)

* Fix wrong getHeader refactoring

Fix regression introduced by
https://github.com/FreshRSS/FreshRSS/pull/2373
The refactoring required a call to init() even for static functions,
which is most of the time not done.
Removed premature abstraction of `$_SERVER`, which was the root cause of
the bug.
https://github.com/FreshRSS/FreshRSS/issues/2748#issuecomment-569898931

* Refactoring: Move serverIsPublic to Minz_Request

* Add mitigations for wrong configurations

Due to the regression, we have some existing configurations with a bad
base_url

* Forgot one instance
Alexandre Alapetite 6 years ago
parent
commit
2aff347b2e
6 changed files with 43 additions and 41 deletions
  1. 3 2
      app/Models/Feed.php
  2. 1 1
      app/install.php
  3. 1 1
      cli/do-install.php
  4. 37 6
      lib/Minz/Request.php
  5. 1 1
      lib/Minz/Url.php
  6. 0 30
      lib/lib_rss.php

+ 3 - 2
app/Models/Feed.php

@@ -643,7 +643,8 @@ class FreshRSS_Feed extends Minz_Model {
 
 	public function pubSubHubbubPrepare() {
 		$key = '';
-		if (FreshRSS_Context::$system_conf->base_url && $this->hubUrl && $this->selfUrl && @is_dir(PSHB_PATH)) {
+		if (Minz_Request::serverIsPublic(FreshRSS_Context::$system_conf->base_url) &&
+			$this->hubUrl && $this->selfUrl && @is_dir(PSHB_PATH)) {
 			$path = PSHB_PATH . '/feeds/' . base64url_encode($this->selfUrl);
 			$hubFilename = $path . '/!hub.json';
 			if ($hubFile = @file_get_contents($hubFilename)) {
@@ -690,7 +691,7 @@ class FreshRSS_Feed extends Minz_Model {
 	//Parameter true to subscribe, false to unsubscribe.
 	public function pubSubHubbubSubscribe($state) {
 		$url = $this->selfUrl ? $this->selfUrl : $this->url;
-		if (FreshRSS_Context::$system_conf->base_url && $url) {
+		if ($url && (Minz_Request::serverIsPublic(FreshRSS_Context::$system_conf->base_url) || !$state)) {
 			$hubFilename = PSHB_PATH . '/feeds/' . base64url_encode($url) . '/!hub.json';
 			$hubFile = @file_get_contents($hubFilename);
 			if ($hubFile === false) {

+ 1 - 1
app/install.php

@@ -142,7 +142,7 @@ function saveStep2() {
 				'prefix' => $_SESSION['bd_prefix'],
 				'pdo_options' => [],
 			],
-			'pubsubhubbub_enabled' => server_is_public($base_url),
+			'pubsubhubbub_enabled' => Minz_Request::serverIsPublic($base_url),
 		];
 		if (!empty($_SESSION['title'])) {
 			$config_array['title'] = $_SESSION['title'];

+ 1 - 1
cli/do-install.php

@@ -53,7 +53,7 @@ foreach ($params as $param) {
 	}
 }
 
-if ((!empty($config['base_url'])) && server_is_public($config['base_url'])) {
+if ((!empty($config['base_url'])) && Minz_Request::serverIsPublic($config['base_url'])) {
 	$config['pubsubhubbub_enabled'] = true;
 }
 

+ 37 - 6
lib/Minz/Request.php

@@ -11,7 +11,6 @@ class Minz_Request {
 	private static $controller_name = '';
 	private static $action_name = '';
 	private static $params = array();
-	private static $headers = array();
 
 	private static $default_controller_name = 'index';
 	private static $default_action_name = 'index';
@@ -101,7 +100,6 @@ class Minz_Request {
 	 * Initialise la Request
 	 */
 	public static function init() {
-		static::$headers = $_SERVER;
 		self::initJSON();
 	}
 
@@ -227,6 +225,42 @@ class Minz_Request {
 		return filter_var($url, FILTER_SANITIZE_URL);
 	}
 
+	/**
+	 * Test if a given server address is publicly accessible.
+	 *
+	 * Note: for the moment it tests only if address is corresponding to a
+	 * localhost address.
+	 *
+	 * @param $address the address to test, can be an IP or a URL.
+	 * @return true if server is accessible, false otherwise.
+	 * @todo improve test with a more valid technique (e.g. test with an external server?)
+	 */
+	public static function serverIsPublic($address) {
+		if (strlen($address) < strlen('http://a.bc')) {
+			return false;
+		}
+		$host = parse_url($address, PHP_URL_HOST);
+		if (!$host) {
+			return false;
+		}
+
+		$is_public = !in_array($host, array(
+			'localhost',
+			'localhost.localdomain',
+			'[::1]',
+			'ip6-localhost',
+			'localhost6',
+			'localhost6.localdomain6',
+		));
+
+		if ($is_public) {
+			$is_public &= !preg_match('/^(10|127|172[.]16|192[.]168)[.]/', $host);
+			$is_public &= !preg_match('/^(\[)?(::1$|fc00::|fe80::)/i', $host);
+		}
+
+		return (bool)$is_public;
+	}
+
 	/**
 	 * Relance une requête
 	 * @param $url l'url vers laquelle est relancée la requête
@@ -348,10 +382,7 @@ class Minz_Request {
 	 * @return mixed
 	 */
 	public static function getHeader($header, $default = null) {
-		if (isset(static::$headers[$header])) {
-			return static::$headers[$header];
-		}
-		return $default;
+		return isset($_SERVER[$header]) ? $_SERVER[$header] : $default;
 	}
 
 	/**

+ 1 - 1
lib/Minz/Url.php

@@ -25,7 +25,7 @@ class Minz_Url {
 
 		if ($absolute) {
 			$url_string = Minz_Request::getBaseUrl();
-			if ($url_string == '') {
+			if (strlen($url_string) < strlen('http://a.bc')) {
 				$url_string = Minz_Request::guessBaseUrl();
 				if (PUBLIC_RELATIVE === '..') {
 					//TODO: Implement proper resolver of relative parts such as /test/./../

+ 0 - 30
lib/lib_rss.php

@@ -118,36 +118,6 @@ function escapeToUnicodeAlternative($text, $extended = true) {
 	return trim(str_replace($problem, $replace, $text));
 }
 
-/**
- * Test if a given server address is publicly accessible.
- *
- * Note: for the moment it tests only if address is corresponding to a
- * localhost address.
- *
- * @param $address the address to test, can be an IP or a URL.
- * @return true if server is accessible, false otherwise.
- * @todo improve test with a more valid technique (e.g. test with an external server?)
- */
-function server_is_public($address) {
-	$host = parse_url($address, PHP_URL_HOST);
-
-	$is_public = !in_array($host, array(
-		'localhost',
-		'localhost.localdomain',
-		'[::1]',
-		'ip6-localhost',
-		'localhost6',
-		'localhost6.localdomain6',
-	));
-
-	if ($is_public) {
-		$is_public &= !preg_match('/^(10|127|172[.]16|192[.]168)[.]/', $host);
-		$is_public &= !preg_match('/^(\[)?(::1$|fc00::|fe80::)/i', $host);
-	}
-
-	return (bool)$is_public;
-}
-
 function format_number($n, $precision = 0) {
 	// number_format does not seem to be Unicode-compatible
 	return str_replace(' ', ' ',  //Espace fine insécable