From 334e075dd85a21e859eb89948b606c2710b932ad Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 2 Oct 2018 11:49:39 +0100
Subject: [PATCH 1/2] Fix error when logging incomplete requests

If a connection is lost before a request is read from Request, Twisted
sets `method` (and `uri`) attributes to dummy values. These dummy values
have incorrect types (i.e. they're not bytes), and so things like
`__repr__` would raise an exception.

To fix this we had a helper method to return the method with a
consistent type.
---
 synapse/http/site.py | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/synapse/http/site.py b/synapse/http/site.py
index 50be2de3bb..e508c0bd4f 100644
--- a/synapse/http/site.py
+++ b/synapse/http/site.py
@@ -75,14 +75,14 @@ class SynapseRequest(Request):
         return '<%s at 0x%x method=%r uri=%r clientproto=%r site=%r>' % (
             self.__class__.__name__,
             id(self),
-            self.method.decode('ascii', errors='replace'),
+            self.get_method(),
             self.get_redacted_uri(),
             self.clientproto.decode('ascii', errors='replace'),
             self.site.site_tag,
         )
 
     def get_request_id(self):
-        return "%s-%i" % (self.method.decode('ascii'), self.request_seq)
+        return "%s-%i" % (self.get_method(), self.request_seq)
 
     def get_redacted_uri(self):
         uri = self.uri
@@ -90,6 +90,21 @@ class SynapseRequest(Request):
             uri = self.uri.decode('ascii')
         return redact_uri(uri)
 
+    def get_method(self):
+        """Gets the method associated with the request (or placeholder if not
+        method has yet been received).
+
+        Note: This is necessary as the placeholder value in twisted is str
+        rather than bytes, so we need to sanitise `self.method`.
+
+        Returns:
+            str
+        """
+        method = self.method
+        if isinstance(method, bytes):
+            method = self.method.decode('ascii')
+        return method
+
     def get_user_agent(self):
         return self.requestHeaders.getRawHeaders(b"User-Agent", [None])[-1]
 
@@ -119,7 +134,7 @@ class SynapseRequest(Request):
             # dispatching to the handler, so that the handler
             # can update the servlet name in the request
             # metrics
-            requests_counter.labels(self.method.decode('ascii'),
+            requests_counter.labels(self.get_method(),
                                     self.request_metrics.name).inc()
 
     @contextlib.contextmanager
@@ -207,14 +222,14 @@ class SynapseRequest(Request):
         self.start_time = time.time()
         self.request_metrics = RequestMetrics()
         self.request_metrics.start(
-            self.start_time, name=servlet_name, method=self.method.decode('ascii'),
+            self.start_time, name=servlet_name, method=self.get_method(),
         )
 
         self.site.access_logger.info(
             "%s - %s - Received request: %s %s",
             self.getClientIP(),
             self.site.site_tag,
-            self.method.decode('ascii'),
+            self.get_method(),
             self.get_redacted_uri()
         )
 
@@ -280,7 +295,7 @@ class SynapseRequest(Request):
             int(usage.db_txn_count),
             self.sentLength,
             code,
-            self.method.decode('ascii'),
+            self.get_method(),
             self.get_redacted_uri(),
             self.clientproto.decode('ascii', errors='replace'),
             user_agent,

From eeb755d17b5a38a3fb9f4648c6c56962efd58500 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Tue, 2 Oct 2018 12:01:51 +0100
Subject: [PATCH 2/2] Newsfile

---
 changelog.d/3990.bugfix | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/3990.bugfix

diff --git a/changelog.d/3990.bugfix b/changelog.d/3990.bugfix
new file mode 100644
index 0000000000..51ea105a64
--- /dev/null
+++ b/changelog.d/3990.bugfix
@@ -0,0 +1 @@
+Fix error when logging incomplete HTTP requests