From eb79110beb79c639d75b26f6c5832a8192776a8f Mon Sep 17 00:00:00 2001
From: Mark Haines <mark.haines@matrix.org>
Date: Mon, 16 May 2016 13:03:59 +0100
Subject: [PATCH] Clean up the blacklist/whitelist handling.

Always set the config key with an empty list, even if a list isn't specified.
This means that the codepaths are the same for both the empty list and
for a missing key. Since the behaviour is the same for both cases this
makes the code somewhat easier to reason about.
---
 synapse/config/repository.py                  | 12 ++--
 synapse/http/client.py                        |  3 +-
 synapse/rest/media/v1/preview_url_resource.py | 61 +++++++++----------
 3 files changed, 36 insertions(+), 40 deletions(-)

diff --git a/synapse/config/repository.py b/synapse/config/repository.py
index 44224424f..881007984 100644
--- a/synapse/config/repository.py
+++ b/synapse/config/repository.py
@@ -100,13 +100,13 @@ class ContentRepositoryConfig(Config):
                     "to work"
                 )
 
-            if "url_preview_ip_range_whitelist" in config:
-                self.url_preview_ip_range_whitelist = IPSet(
-                    config["url_preview_ip_range_whitelist"]
-                )
+            self.url_preview_ip_range_whitelist = IPSet(
+                config.get("url_preview_ip_range_whitelist", ())
+            )
 
-            if "url_preview_url_blacklist" in config:
-                self.url_preview_url_blacklist = config["url_preview_url_blacklist"]
+            self.url_preview_url_blacklist = config.get(
+                "url_preview_url_blacklist", ()
+            )
 
     def default_config(self, **kwargs):
         media_store = self.default_path("media_store")
diff --git a/synapse/http/client.py b/synapse/http/client.py
index a8e2d8e80..c7fa69243 100644
--- a/synapse/http/client.py
+++ b/synapse/http/client.py
@@ -380,8 +380,7 @@ class CaptchaServerHttpClient(SimpleHttpClient):
 class SpiderEndpointFactory(object):
     def __init__(self, hs):
         self.blacklist = hs.config.url_preview_ip_range_blacklist
-        if hasattr(hs.config, "url_preview_ip_range_whitelist"):
-            self.whitelist = hs.config.url_preview_ip_range_whitelist
+        self.whitelist = hs.config.url_preview_ip_range_whitelist
         self.policyForHTTPS = hs.get_http_client_context_factory()
 
     def endpointForURI(self, uri):
diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py
index dc1e5fbdb..37dd1de89 100644
--- a/synapse/rest/media/v1/preview_url_resource.py
+++ b/synapse/rest/media/v1/preview_url_resource.py
@@ -56,8 +56,7 @@ class PreviewUrlResource(Resource):
         self.client = SpiderHttpClient(hs)
         self.media_repo = media_repo
 
-        if hasattr(hs.config, "url_preview_url_blacklist"):
-            self.url_preview_url_blacklist = hs.config.url_preview_url_blacklist
+        self.url_preview_url_blacklist = hs.config.url_preview_url_blacklist
 
         # simple memory cache mapping urls to OG metadata
         self.cache = ExpiringCache(
@@ -86,39 +85,37 @@ class PreviewUrlResource(Resource):
         else:
             ts = self.clock.time_msec()
 
-        # impose the URL pattern blacklist
-        if hasattr(self, "url_preview_url_blacklist"):
-            url_tuple = urlparse.urlsplit(url)
-            for entry in self.url_preview_url_blacklist:
-                match = True
-                for attrib in entry:
-                    pattern = entry[attrib]
-                    value = getattr(url_tuple, attrib)
-                    logger.debug((
-                        "Matching attrib '%s' with value '%s' against"
-                        " pattern '%s'"
-                    ) % (attrib, value, pattern))
+        url_tuple = urlparse.urlsplit(url)
+        for entry in self.url_preview_url_blacklist:
+            match = True
+            for attrib in entry:
+                pattern = entry[attrib]
+                value = getattr(url_tuple, attrib)
+                logger.debug((
+                    "Matching attrib '%s' with value '%s' against"
+                    " pattern '%s'"
+                ) % (attrib, value, pattern))
 
-                    if value is None:
+                if value is None:
+                    match = False
+                    continue
+
+                if pattern.startswith('^'):
+                    if not re.match(pattern, getattr(url_tuple, attrib)):
                         match = False
                         continue
-
-                    if pattern.startswith('^'):
-                        if not re.match(pattern, getattr(url_tuple, attrib)):
-                            match = False
-                            continue
-                    else:
-                        if not fnmatch.fnmatch(getattr(url_tuple, attrib), pattern):
-                            match = False
-                            continue
-                if match:
-                    logger.warn(
-                        "URL %s blocked by url_blacklist entry %s", url, entry
-                    )
-                    raise SynapseError(
-                        403, "URL blocked by url pattern blacklist entry",
-                        Codes.UNKNOWN
-                    )
+                else:
+                    if not fnmatch.fnmatch(getattr(url_tuple, attrib), pattern):
+                        match = False
+                        continue
+            if match:
+                logger.warn(
+                    "URL %s blocked by url_blacklist entry %s", url, entry
+                )
+                raise SynapseError(
+                    403, "URL blocked by url pattern blacklist entry",
+                    Codes.UNKNOWN
+                )
 
         # first check the memory cache - good to handle all the clients on this
         # HS thundering away to preview the same URL at the same time.