Return 200 OK for all OPTIONS requests (#7534)

5 files changed, 85 insertions, 11 deletions

diff --git a/changelog.d/7534.misc b/changelog.d/7534.misc
new file mode 100644
index 0000000000..9088fb65b8
--- /dev/null
+++ b/changelog.d/7534.misc
@@ -0,0 +1 @@
+All endpoints now respond with a 200 OK for `OPTIONS` requests.
\ No newline at end of file
diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py
index a45c876213..a37520000a 100644
--- a/synapse/app/generic_worker.py
+++ b/synapse/app/generic_worker.py
@@ -22,7 +22,6 @@ from typing import Dict, Iterable
 from typing_extensions import ContextManager
 
 from twisted.internet import defer, reactor
-from twisted.web.resource import NoResource
 
 import synapse
 import synapse.events
@@ -41,7 +40,7 @@ from synapse.config.logger import setup_logging
 from synapse.federation import send_queue
 from synapse.federation.transport.server import TransportLayerServer
 from synapse.handlers.presence import BasePresenceHandler, get_interested_parties
-from synapse.http.server import JsonResource
+from synapse.http.server import JsonResource, OptionsResource
 from synapse.http.servlet import RestServlet, parse_json_object_from_request
 from synapse.http.site import SynapseSite
 from synapse.logging.context import LoggingContext
@@ -566,7 +565,7 @@ class GenericWorkerServer(HomeServer):
                 if name == "replication":
                     resources[REPLICATION_PREFIX] = ReplicationRestResource(self)
 
-        root_resource = create_resource_tree(resources, NoResource())
+        root_resource = create_resource_tree(resources, OptionsResource())
 
         _base.listen_tcp(
             bind_addresses,
diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py
index d7f337e586..93a5ba2100 100644
--- a/synapse/app/homeserver.py
+++ b/synapse/app/homeserver.py
@@ -31,7 +31,7 @@ from prometheus_client import Gauge
 from twisted.application import service
 from twisted.internet import defer, reactor
 from twisted.python.failure import Failure
-from twisted.web.resource import EncodingResourceWrapper, IResource, NoResource
+from twisted.web.resource import EncodingResourceWrapper, IResource
 from twisted.web.server import GzipEncoderFactory
 from twisted.web.static import File
 
@@ -52,7 +52,11 @@ from synapse.config._base import ConfigError
 from synapse.config.homeserver import HomeServerConfig
 from synapse.federation.transport.server import TransportLayerServer
 from synapse.http.additional_resource import AdditionalResource
-from synapse.http.server import RootRedirect
+from synapse.http.server import (
+    OptionsResource,
+    RootOptionsRedirectResource,
+    RootRedirect,
+)
 from synapse.http.site import SynapseSite
 from synapse.logging.context import LoggingContext
 from synapse.metrics import METRICS_PREFIX, MetricsResource, RegistryProxy
@@ -121,11 +125,11 @@ class SynapseHomeServer(HomeServer):
 
         # try to find something useful to redirect '/' to
         if WEB_CLIENT_PREFIX in resources:
-            root_resource = RootRedirect(WEB_CLIENT_PREFIX)
+            root_resource = RootOptionsRedirectResource(WEB_CLIENT_PREFIX)
         elif STATIC_PREFIX in resources:
-            root_resource = RootRedirect(STATIC_PREFIX)
+            root_resource = RootOptionsRedirectResource(STATIC_PREFIX)
         else:
-            root_resource = NoResource()
+            root_resource = OptionsResource()
 
         root_resource = create_resource_tree(resources, root_resource)
 
diff --git a/synapse/http/server.py b/synapse/http/server.py
index 042a605198..33fcfbea6e 100644
--- a/synapse/http/server.py
+++ b/synapse/http/server.py
@@ -350,9 +350,6 @@ class JsonResource(HttpServer, resource.Resource):
                 register_paths, so will return (possibly via Deferred) either
                 None, or a tuple of (http code, response body).
         """
-        if request.method == b"OPTIONS":
-            return _options_handler, "options_request_handler", {}
-
         request_path = request.path.decode("ascii")
 
         # Loop through all the registered callbacks to check if the method
@@ -448,6 +445,26 @@ class RootRedirect(resource.Resource):
         return resource.Resource.getChild(self, name, request)
 
 
+class OptionsResource(resource.Resource):
+    """Responds to OPTION requests for itself and all children."""
+
+    def render_OPTIONS(self, request):
+        code, response_json_object = _options_handler(request)
+
+        return respond_with_json(
+            request, code, response_json_object, send_cors=False, canonical_json=False,
+        )
+
+    def getChildWithDefault(self, path, request):
+        if request.method == b"OPTIONS":
+            return self  # select ourselves as the child to render
+        return resource.Resource.getChildWithDefault(self, path, request)
+
+
+class RootOptionsRedirectResource(OptionsResource, RootRedirect):
+    pass
+
+
 def respond_with_json(
     request,
     code,
diff --git a/tests/test_server.py b/tests/test_server.py
index 0d57eed268..437f925bf9 100644
--- a/tests/test_server.py
+++ b/tests/test_server.py
@@ -27,6 +27,7 @@ from synapse.api.errors import Codes, RedirectException, SynapseError
 from synapse.http.server import (
     DirectServeResource,
     JsonResource,
+    OptionsResource,
     wrap_html_request_handler,
 )
 from synapse.http.site import SynapseSite, logger
@@ -168,6 +169,58 @@ class JsonResourceTests(unittest.TestCase):
         self.assertEqual(channel.json_body["errcode"], "M_UNRECOGNIZED")
 
 
+class OptionsResourceTests(unittest.TestCase):
+    def setUp(self):
+        self.reactor = ThreadedMemoryReactorClock()
+
+        class DummyResource(Resource):
+            isLeaf = True
+
+            def render(self, request):
+                return request.path
+
+        # Setup a resource with some children.
+        self.resource = OptionsResource()
+        self.resource.putChild(b"res", DummyResource())
+
+    def _make_request(self, method, path):
+        """Create a request from the method/path and return a channel with the response."""
+        request, channel = make_request(self.reactor, method, path, shorthand=False)
+        request.prepath = []  # This doesn't get set properly by make_request.
+
+        # Create a site and query for the resource.
+        site = SynapseSite("test", "site_tag", {}, self.resource, "1.0")
+        request.site = site
+        resource = site.getResourceFor(request)
+
+        # Finally, render the resource and return the channel.
+        render(request, resource, self.reactor)
+        return channel
+
+    def test_unknown_options_request(self):
+        """An OPTIONS requests to an unknown URL still returns 200 OK."""
+        channel = self._make_request(b"OPTIONS", b"/foo/")
+        self.assertEqual(channel.result["code"], b"200")
+        self.assertEqual(channel.result["body"], b"{}")
+
+    def test_known_options_request(self):
+        """An OPTIONS requests to an known URL still returns 200 OK."""
+        channel = self._make_request(b"OPTIONS", b"/res/")
+        self.assertEqual(channel.result["code"], b"200")
+        self.assertEqual(channel.result["body"], b"{}")
+
+    def test_unknown_request(self):
+        """A non-OPTIONS request to an unknown URL should 404."""
+        channel = self._make_request(b"GET", b"/foo/")
+        self.assertEqual(channel.result["code"], b"404")
+
+    def test_known_request(self):
+        """A non-OPTIONS request to an known URL should query the proper resource."""
+        channel = self._make_request(b"GET", b"/res/")
+        self.assertEqual(channel.result["code"], b"200")
+        self.assertEqual(channel.result["body"], b"/res/")
+
+
 class WrapHtmlRequestHandlerTests(unittest.TestCase):
     class TestResource(DirectServeResource):
         callback = None