[locationinfo] Put upgrade/cleanup code in distinct function and extend

3 files changed, 39 insertions, 15 deletions

diff --git a/modules-available/locationinfo/inc/locationinfo.inc.php b/modules-available/locationinfo/inc/locationinfo.inc.php
index 1165164d..a46208c6 100644
--- a/modules-available/locationinfo/inc/locationinfo.inc.php
+++ b/modules-available/locationinfo/inc/locationinfo.inc.php
@@ -237,4 +237,41 @@ class LocationInfo
 		return '';
 	}
 
+
+	/**
+	 * Transform legacy url filter config if found, remove no-op filter setups.
+	 * @param array $config The configuration array to be cleaned up. By reference.
+	 */
+	public static function cleanupUrlFilter(array &$config): void
+	{
+		// First, simple trimming of white-space around the list, and making sure the keys exist
+		$config['blacklist'] = trim($config['blacklist'] ?? '');
+		$config['whitelist'] = trim($config['whitelist'] ?? '');
+		if (empty($config['blacklist']) && empty($config['whitelist'])) {
+			// Mangle non-upgraded configurations: They only have one list and a bool specifying if it's a black- or whitelist
+			if (!empty($config['urllist'])) {
+				if ($config['iswhitelist'] ?? false) {
+					$config['whitelist'] = str_replace(' ', "\n", $config['urllist']);
+				} else {
+					$config['blacklist'] = str_replace(' ', "\n", $config['urllist']);
+				}
+				unset($config['urllist'], $config['iswhitelist']);
+			}
+		} elseif ((empty($config['blacklist']) || self::isAnyUrlMatch($config['blacklist']))
+				&& self::isAnyUrlMatch($config['whitelist'])) {
+			// Blocking everything/nothing and allowing everything is a no-op for all three browsers, so clear both lists
+			$config['blacklist'] = '';
+			$config['whitelist'] = '';
+		}
+	}
+
+	/**
+	 * Check if the given pattern would be interpreted as matching any URL.
+	 * @param string $url The URL to check
+	 */
+	private static function isAnyUrlMatch(string $url): bool
+	{
+		return $url === '*' || $url === '*://*' || $url === '*://*/' || $url === '*://*/*';
+	}
+
 }
diff --git a/modules-available/locationinfo/inc/locationinfohooks.inc.php b/modules-available/locationinfo/inc/locationinfohooks.inc.php
index 175afbd2..ac8beab9 100644
--- a/modules-available/locationinfo/inc/locationinfohooks.inc.php
+++ b/modules-available/locationinfo/inc/locationinfohooks.inc.php
@@ -29,14 +29,7 @@ class LocationInfoHooks
 				ConfigHolder::add('SLX_BROWSER_RELOAD_SECS', $data['reload-minutes'] * 60);
 			}
 			ConfigHolder::add('SLX_BROWSER_URL', $data['url']);
-			// Mangle non-upgraded panels
-			if (empty($data['blacklist']) && $data['whitelist'] === '*' && !empty($data['urllist'])) {
-				if ($data['iswhitelist']) {
-					$data['whitelist'] = str_replace(' ', "\n", $data['urllist']);
-				} else {
-					$data['blacklist'] = str_replace(' ', "\n", $data['urllist']);
-				}
-			}
+			LocationInfo::cleanupUrlFilter($data);
 			ConfigHolder::add('SLX_BROWSER_WHITELIST', self::mangleList($data['whitelist']));
 			ConfigHolder::add('SLX_BROWSER_BLACKLIST', self::mangleList($data['blacklist']));
 			// Additionally, update runmode "isclient" flag depending on whether split-login is allowed or not
diff --git a/modules-available/locationinfo/page.inc.php b/modules-available/locationinfo/page.inc.php
index 7e19bfbc..1a58131f 100644
--- a/modules-available/locationinfo/page.inc.php
+++ b/modules-available/locationinfo/page.inc.php
@@ -999,13 +999,7 @@ class Page_LocationInfo extends Page
 				}
 			}
 
-			if (empty($config['blacklist']) && $config['whitelist'] === '*' && !empty($config['urllist'])) {
-				if ($config['iswhitelist']) {
-					$config['whitelist'] = str_replace(' ', "\n", $config['urllist']);
-				} else {
-					$config['blacklist'] = str_replace(' ', "\n", $config['urllist']);
-				}
-			}
+			LocationInfo::cleanupUrlFilter($config);
 
 			Render::addTemplate('page-config-panel-url', array(
 				'new' => $id === 'new',