[tor-commits] [bridgedb/master] Implement moat error handling when no bridges are available.

isis at torproject.org isis at torproject.org
Wed Dec 20 23:10:05 UTC 2017


commit 4c799735b5b86beea486ee3b1cd0ea8a4f5cb4b5
Author: Isis Lovecruft <isis at torproject.org>
Date:   Wed Dec 20 22:36:06 2017 +0000

    Implement moat error handling when no bridges are available.
    
     * FIXES #24637: https://bugs.torproject.org/24637
---
 bridgedb/distributors/moat/server.py           | 73 +++++++++++++++++++++-----
 bridgedb/test/test_distributors_moat_server.py | 55 +++++++++++++++++--
 2 files changed, 112 insertions(+), 16 deletions(-)

diff --git a/bridgedb/distributors/moat/server.py b/bridgedb/distributors/moat/server.py
index eb356fe..a669ee5 100644
--- a/bridgedb/distributors/moat/server.py
+++ b/bridgedb/distributors/moat/server.py
@@ -209,6 +209,7 @@ class JsonAPIErrorResource(JsonAPIResource):
 
 
 resource403 = JsonAPIErrorResource(code=403, status="Forbidden")
+resource404 = JsonAPIErrorResource(code=404, status="Not Found")
 resource406 = JsonAPIErrorResource(code=406, status="Not Acceptable")
 resource415 = JsonAPIErrorResource(code=415, status="Unsupported Media Type")
 resource419 = JsonAPIErrorResource(code=419, status="No You're A Teapot")
@@ -497,21 +498,21 @@ class CaptchaCheckResource(CaptchaResource):
         self.nBridgesToGive = N
         self.useForwardedHeader = useForwardedHeader
 
-    def getBridgeLines(self, ip, data):
-        """Get bridge lines for a client's HTTP request.
+    def createBridgeRequest(self, ip, data):
+        """Create an appropriate :class:`MoatBridgeRequest` from the ``data``
+        of a client's request.
 
         :param str ip: The client's IP address.
         :param dict data: The decoded JSON API data from the client's request.
-        :rtype: list or None
-        :returns: A list of bridge lines.
+        :rtype: :class:`MoatBridgeRequest`
+        :returns: An object which specifies the filters for retreiving
+            the type of bridges that the client requested.
         """
-        bridgeLines = None
-        interval = self.schedule.intervalStart(time.time())
+        logging.debug("Creating moat bridge request object for %s." % ip)
 
-        logging.debug("Replying to JSON API request from %s." % ip)
+        bridgeRequest = MoatBridgeRequest()
 
         if ip and data:
-            bridgeRequest = MoatBridgeRequest()
             bridgeRequest.client = IPAddress(ip)
             bridgeRequest.isValid(True)
             bridgeRequest.withIPversion()
@@ -519,6 +520,23 @@ class CaptchaCheckResource(CaptchaResource):
             bridgeRequest.withoutBlockInCountry(data)
             bridgeRequest.generateFilters()
 
+        return bridgeRequest
+
+    def getBridgeLines(self, bridgeRequest):
+        """Get bridge lines for a client's HTTP request.
+
+        :type bridgeRequest: :class:`MoatBridgeRequest`
+        :param bridgeRequest: A valid bridge request object with pre-generated
+            filters (as returned by :meth:`createBridgeRequest`).
+        :rtype: list
+        :returns: A list of bridge lines.
+        """
+        bridgeLines = list()
+        interval = self.schedule.intervalStart(time.time())
+
+        logging.debug("Replying to JSON API request from %s." % bridgeRequest.client)
+
+        if bridgeRequest.isValid():
             bridges = self.distributor.getBridges(bridgeRequest, interval)
             bridgeLines = [replaceControlChars(bridge.getBridgeLine(bridgeRequest))
                            for bridge in bridges]
@@ -599,17 +617,34 @@ class CaptchaCheckResource(CaptchaResource):
 
         return valid
 
-    def failureResponse(self, id, request):
-        """Respond with status code "419 No You're A Teapot"."""
-        error_response = resource419
-        error_response.type = 'moat-bridges'
+    def failureResponse(self, id, request, bridgeRequest=None):
+        """Respond with status code "419 No You're A Teapot" if the captcha
+        verification failed, or status code "404 Not Found" if there
+        were none of the type of bridges requested.
 
+        :param int id: The JSON API "id" field of the
+            ``JsonAPIErrorResource`` which should be returned.
+        :type request: :api:`twisted.web.http.Request`
+        :param request: The current request we're handling.
+        :type bridgeRequest: :class:`MoatBridgeRequest`
+        :param bridgeRequest: A valid bridge request object with pre-generated
+            filters (as returned by :meth:`createBridgeRequest`).
+        """
         if id == 4:
+            error_response = resource419
             error_response.id = 4
             error_response.detail = "The CAPTCHA solution was incorrect."
         elif id == 5:
+            error_response = resource419
             error_response.id = 5
             error_response.detail = "The CAPTCHA challenge timed out."
+        elif id == 6:
+            error_response = resource404
+            error_response.id = 6
+            error_response.detail = ("No bridges available to fulfill "
+                                     "request: %s.") % bridgeRequest
+
+        error_response.type = 'moat-bridges'
 
         return error_response.render(request)
 
@@ -665,7 +700,19 @@ class CaptchaCheckResource(CaptchaResource):
 
         if valid:
             qrcode = None
-            bridgeLines = self.getBridgeLines(clientIP, client_data)
+            bridgeRequest = self.createBridgeRequest(clientIP, client_data)
+            bridgeLines = self.getBridgeLines(bridgeRequest)
+
+            # If we can only return less than the configured
+            # MOAT_BRIDGES_PER_ANSWER then log a warning.
+            if len(bridgeLines) < self.nBridgesToGive:
+                logging.warn(("Not enough bridges of the type specified to "
+                              "fulfill the following request: %s") % bridgeRequest)
+
+            # If we have no bridges at all to give to the client, then
+            # return a JSON API 404 error.
+            if not bridgeLines:
+                return self.failureResponse(6, request)
 
             if include_qrcode:
                 qrjpeg = generateQR(bridgeLines)
diff --git a/bridgedb/test/test_distributors_moat_server.py b/bridgedb/test/test_distributors_moat_server.py
index 95796d1..2f84557 100644
--- a/bridgedb/test/test_distributors_moat_server.py
+++ b/bridgedb/test/test_distributors_moat_server.py
@@ -581,6 +581,14 @@ class CaptchaFetchResourceTests(unittest.TestCase):
         self.assert_data_is_ok(decoded)
 
 
+class MockCaptchaCheckResource(server.CaptchaCheckResource):
+    """A mocked :class:`server.CaptchaCheckResource` whose
+    ``getBridgeLines`` method returns no bridges.
+    """
+    def getBridgeLines(self, bridgeRequest):
+        return list()
+
+
 class CaptchaCheckResourceTests(unittest.TestCase):
     """Tests for :class:`bridgedb.distributors.moat.server.CaptchaCheckResource`."""
 
@@ -611,6 +619,14 @@ class CaptchaCheckResourceTests(unittest.TestCase):
             "iz_PdOD2GIGPeclwiHAWM1pOS4cQVsTQR_z4ojZbpLiSp35n4Qbb11YOoreovZzlbS"
             "7W38rAsTirkdeugcNq82AxKP3phEkyRcw--CzV")
 
+    def mock_getBridgeLines(self):
+        self.resource = MockCaptchaCheckResource(self.distributor,
+                                                 self.schedule, 3,
+                                                 self.hmacKey,
+                                                 self.publicKey,
+                                                 self.secretKey,
+                                                 useForwardedHeader=False)
+
     def create_POST_with_data(self, data):
         request = DummyRequest([self.pagename])
         request.requestHeaders.addRawHeader('Content-Type', 'application/vnd.api+json')
@@ -826,13 +842,24 @@ class CaptchaCheckResourceTests(unittest.TestCase):
         self.assertEqual(error['type'], "moat-bridges")
         self.assertEqual(error['id'], 4)
 
+    def test_createBridgeRequest(self):
+        request = self.create_valid_POST_with_challenge(self.expiredChallenge)
+        request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443)
+        encoded_content = request.content.read()
+        content = json.loads(encoded_content)['data'][0]
+
+        bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', content)
+
+        self.assertTrue(bridgeRequest.isValid())
+
     def test_getBridgeLines(self):
         request = self.create_valid_POST_with_challenge(self.expiredChallenge)
         request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443)
         encoded_content = request.content.read()
         content = json.loads(encoded_content)['data'][0]
 
-        bridgelines = self.resource.getBridgeLines('3.3.3.3', content)
+        bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', content)
+        bridgelines = self.resource.getBridgeLines(bridgeRequest)
 
         self.assertTrue(bridgelines)
 
@@ -840,9 +867,31 @@ class CaptchaCheckResourceTests(unittest.TestCase):
         request = self.create_valid_POST_with_challenge(self.expiredChallenge)
         request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443)
 
-        bridgelines = self.resource.getBridgeLines('3.3.3.3', None)
+        bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', None)
+        bridgelines = self.resource.getBridgeLines(bridgeRequest)
+
+        self.assertFalse(bridgeRequest.isValid())
+        self.assertEqual(len(bridgelines), 0)
+
+    def test_failureResponse_no_bridges(self):
+        request = self.create_valid_POST_with_challenge(self.expiredChallenge)
+        request.client = requesthelper.IPv4Address('TCP', '3.3.3.3', 443)
+        encoded_content = request.content.read()
+        content = json.loads(encoded_content)['data'][0]
+
+        bridgeRequest = self.resource.createBridgeRequest('3.3.3.3', content)
+
+        response = self.resource.failureResponse(6, request, bridgeRequest)
+
+        self.assertIn("No bridges available", response)
+
+    def test_render_POST_no_bridges(self):
+        self.mock_getBridgeLines()
+
+        request = self.create_valid_POST_make_new_challenge()
+        response = self.resource.render(request)
 
-        self.assertIsNone(bridgelines)
+        self.assertIn("No bridges available", response)
 
     def test_render_POST_unexpired(self):
         request = self.create_valid_POST_make_new_challenge()





More information about the tor-commits mailing list