Unverified Commit 26c2708f authored by Philipp Winter's avatar Philipp Winter
Browse files

Fix remaining str/bytes issues in HTTPS server.

parent 39ea49d5
Loading
Loading
Loading
Loading
+55 −30
Original line number Diff line number Diff line
@@ -95,6 +95,25 @@ supported_langs = []
metrix = metrics.HTTPSMetrics()


def stringifyRequestArgs(args):
    """Turn the given HTTP request arguments from bytes to str.

    :param dict args: A dictionary of request arguments.
    :rtype: dict
    :returns: A dictionary of request arguments.
    """

    # Convert all key/value pairs from bytes to str.
    str_args = {}
    for arg, values in args.items():
        arg = arg if isinstance(arg, str) else arg.decode("utf-8")
        values = [value.decode("utf-8") if isinstance(value, bytes)
                  else value for value in values]
        str_args[arg] = values

    return str_args


def replaceErrorPage(request, error, template_name=None, html=True):
    """Create a general error page for displaying in place of tracebacks.

@@ -114,9 +133,9 @@ def replaceErrorPage(request, error, template_name=None, html=True):
        or if **html** is ``False``, then we return a very simple HTML page
        (without CSS, Javascript, images, etc.)  which simply says
        ``"Sorry! Something went wrong with your request."``
    :rtype: str
    :returns: A string containing some content to serve to the client (rather
        than serving a Twisted traceback).
    :rtype: bytes
    :returns: A bytes object containing some content to serve to the client
        (rather than serving a Twisted traceback).
    """
    logging.error("Error while attempting to render %s: %s"
                  % (template_name or 'template',
@@ -135,15 +154,15 @@ def replaceErrorPage(request, error, template_name=None, html=True):
    errorMessage = _("Sorry! Something went wrong with your request.")

    if not html:
        return errorMessage
        return errorMessage.encode("utf-8")

    try:
        rendered = resource500.render(request)
    except Exception as err:
        logging.exception(err)
        rendered = errorMessage
        rendered = errorMessage.encode("utf-8")

    return rendered.decode('utf-8') if isinstance(rendered, bytes) else rendered
    return rendered


def redirectMaliciousRequest(request):
@@ -336,6 +355,7 @@ class ErrorResource(CSPResource):
        self.setCSPHeader(request)
        request.setHeader("Content-Type", "text/html; charset=utf-8")
        request.setResponseCode(self.code)
        request.args = stringifyRequestArgs(request.args)

        try:
            template = lookup.get_template(self.template)
@@ -343,7 +363,7 @@ class ErrorResource(CSPResource):
        except Exception as err:
            rendered = replaceErrorPage(request, err, html=False)

        return rendered.decode('utf-8') if isinstance(rendered, bytes) else rendered
        return rendered

    render_POST = render_GET

@@ -379,6 +399,7 @@ class TranslatedTemplateResource(CustomErrorHandlingResource, CSPResource):

    def render_GET(self, request):
        self.setCSPHeader(request)
        request.args = stringifyRequestArgs(request.args)
        rtl = False
        try:
            langs = translations.getLocaleFromHTTPRequest(request)
@@ -392,7 +413,7 @@ class TranslatedTemplateResource(CustomErrorHandlingResource, CSPResource):
        except Exception as err:  # pragma: no cover
            rendered = replaceErrorPage(request, err)
        request.setHeader("Content-Type", "text/html; charset=utf-8")
        return rendered.decode('utf-8') if isinstance(rendered, bytes) else rendered
        return rendered

    render_POST = render_GET

@@ -468,7 +489,7 @@ class CaptchaProtectedResource(CustomErrorHandlingResource, CSPResource):
        :type request: :api:`twisted.web.http.Request`
        :param request: A ``Request`` object for 'bridges.html'.
        :returns: A redirect for a request for a new CAPTCHA if there was a
            problem. Otherwise, returns a 2-tuple of strings, the first is the
            problem. Otherwise, returns a 2-tuple of bytes, the first is the
            client's CAPTCHA solution from the text input area, and the second
            is the challenge string.
        """
@@ -491,16 +512,17 @@ class CaptchaProtectedResource(CustomErrorHandlingResource, CSPResource):
        return False

    def render_GET(self, request):
        """Retrieve a ReCaptcha from the API server and serve it to the client.
        """Retrieve a CAPTCHA and serve it to the client.

        :type request: :api:`twisted.web.http.Request`
        :param request: A ``Request`` object for a page which should be
            protected by a CAPTCHA.
        :rtype: str
        :returns: A rendered HTML page containing a ReCaptcha challenge image
        :rtype: bytes
        :returns: A rendered HTML page containing a CAPTCHA challenge image
            for the client to solve.
        """
        self.setCSPHeader(request)
        request.args = stringifyRequestArgs(request.args)

        rtl = False
        image, challenge = self.getCaptchaImage(request)
@@ -509,20 +531,20 @@ class CaptchaProtectedResource(CustomErrorHandlingResource, CSPResource):
            langs = translations.getLocaleFromHTTPRequest(request)
            rtl = translations.usingRTLLang(langs)
            # TODO: this does not work for versions of IE < 8.0
            imgstr = b'data:image/jpeg;base64,%s' % base64.b64encode(image.encode('utf-8'))
            imgstr = b'data:image/jpeg;base64,%s' % base64.b64encode(image)
            template = lookup.get_template('captcha.html')
            rendered = template.render(strings,
                                       getSortedLangList(),
                                       rtl=rtl,
                                       lang=langs[0],
                                       langOverride=translations.isLangOverridden(request),
                                       imgstr=imgstr,
                                       challenge_field=challenge)
                                       imgstr=imgstr.decode("utf-8"),
                                       challenge_field=challenge.decode("utf-8"))
        except Exception as err:
            rendered = replaceErrorPage(request, err, 'captcha.html')

        request.setHeader("Content-Type", "text/html; charset=utf-8")
        return rendered.decode('utf-8') if isinstance(rendered, bytes) else rendered
        return rendered

    def render_POST(self, request):
        """Process a client's CAPTCHA solution.
@@ -543,6 +565,7 @@ class CaptchaProtectedResource(CustomErrorHandlingResource, CSPResource):
        """
        self.setCSPHeader(request)
        request.setHeader("Content-Type", "text/html; charset=utf-8")
        request.args = stringifyRequestArgs(request.args)

        try:
            if self.checkSolution(request) is True:
@@ -862,6 +885,7 @@ class ReCaptchaProtectedResource(CaptchaProtectedResource):
            HTML to the client.
        """
        self.setCSPHeader(request)
        request.args = stringifyRequestArgs(request.args)
        d = self.checkSolution(request)
        d.addCallback(self._renderDeferred)
        return NOT_DONE_YET
@@ -908,10 +932,11 @@ class BridgesResource(CustomErrorHandlingResource, CSPResource):
        :type request: :api:`twisted.web.http.Request`
        :param request: A ``Request`` object containing the HTTP method, full
            URI, and any URL/POST arguments and headers present.
        :rtype: str
        :rtype: bytes
        :returns: A plaintext or HTML response to serve.
        """
        self.setCSPHeader(request)
        request.args = stringifyRequestArgs(request.args)

        try:
            response = self.getBridgeRequestAnswer(request)
@@ -919,7 +944,7 @@ class BridgesResource(CustomErrorHandlingResource, CSPResource):
            logging.exception(err)
            response = self.renderAnswer(request)

        return response.decode('utf-8') if isinstance(response, bytes) else response
        return response.encode('utf-8') if isinstance(response, str) else response

    def getClientIP(self, request):
        """Get the client's IP address from the ``'X-Forwarded-For:'``
@@ -1015,7 +1040,7 @@ class BridgesResource(CustomErrorHandlingResource, CSPResource):
            to use a bridge. If ``None``, then the returned page will instead
            explain that there were no bridges of the type they requested,
            with instructions on how to proceed.
        :rtype: str
        :rtype: bytes
        :returns: A plaintext or HTML response to serve.
        """
        rtl = False
@@ -1048,7 +1073,7 @@ class BridgesResource(CustomErrorHandlingResource, CSPResource):
            except Exception as err:
                rendered = replaceErrorPage(request, err)

        return rendered.decode('utf-8') if isinstance(rendered, bytes) else rendered
        return rendered.encode("utf-8") if isinstance(rendered, str) else rendered


def addWebServer(config, distributor):
@@ -1107,14 +1132,14 @@ def addWebServer(config, distributor):
                          useForwardedHeader=fwdHeaders)

    root = CustomErrorHandlingResource()
    root.putChild('', index)
    root.putChild('robots.txt', robots)
    root.putChild('keys', keys)
    root.putChild('assets', assets)
    root.putChild('options', options)
    root.putChild('howto', howto)
    root.putChild('maintenance', maintenance)
    root.putChild('error', resource500)
    root.putChild(b'', index)
    root.putChild(b'robots.txt', robots)
    root.putChild(b'keys', keys)
    root.putChild(b'assets', assets)
    root.putChild(b'options', options)
    root.putChild(b'howto', howto)
    root.putChild(b'maintenance', maintenance)
    root.putChild(b'error', resource500)
    root.putChild(CSPResource.reportURI, csp)

    if config.RECAPTCHA_ENABLED:
@@ -1147,10 +1172,10 @@ def addWebServer(config, distributor):
                            secretKey=secretKey,
                            useForwardedHeader=fwdHeaders,
                            protectedResource=bridges)
        root.putChild('bridges', protected)
        root.putChild(b'bridges', protected)
        logging.info("Protecting resources with %s." % captcha.func.__name__)
    else:
        root.putChild('bridges', bridges)
        root.putChild(b'bridges', bridges)

    site = Site(root)
    site.displayTracebacks = False
+1 −1
Original line number Diff line number Diff line
@@ -353,7 +353,7 @@ class _HTTPTranslationsTests(unittest.TestCase):
                                           localedir=self.i18n,
                                           languages=[locale,],
                                           fallback=True)
            expected = language.gettext("What are bridges?")
            expected = language.gettext("What are bridges?").encode("utf-8")

            if not locale.startswith('en'):
                self.assertNotEqual(expected, "What are bridges?")
+26 −26
Original line number Diff line number Diff line
@@ -70,8 +70,8 @@ class ReplaceErrorPageTests(unittest.TestCase):
        request = DummyRequest([''])
        exc = Exception("vegan gümmibären")
        errorPage = server.replaceErrorPage(request, exc)
        self.assertSubstring("Bad News Bears", errorPage)
        self.assertNotSubstring("vegan gümmibären", errorPage)
        self.assertSubstring(b"Bad News Bears", errorPage)
        self.assertNotSubstring("vegan gümmibären".encode("utf-8"), errorPage)

    def test_replaceErrorPage_matches_resource500(self):
        """``replaceErrorPage`` should return the error-500.html page."""
@@ -89,9 +89,9 @@ class ReplaceErrorPageTests(unittest.TestCase):
        exc = Exception("vegan gümmibären")
        server.resource500 = None
        errorPage = server.replaceErrorPage(request, exc)
        self.assertNotSubstring("Bad News Bears", errorPage)
        self.assertNotSubstring("vegan gümmibären", errorPage)
        self.assertSubstring("Sorry! Something went wrong with your request.",
        self.assertNotSubstring(b"Bad News Bears", errorPage)
        self.assertNotSubstring("vegan gümmibären".encode("utf-8"), errorPage)
        self.assertSubstring(b"Sorry! Something went wrong with your request.",
                             errorPage)

class ErrorResourceTests(unittest.TestCase):
@@ -103,17 +103,17 @@ class ErrorResourceTests(unittest.TestCase):
    def test_resource404(self):
        """``server.resource404`` should display the error-404.html page."""
        page = server.resource404.render(self.request)
        self.assertSubstring('We dug around for the page you requested', page)
        self.assertSubstring(b'We dug around for the page you requested', page)

    def test_resource500(self):
        """``server.resource500`` should display the error-500.html page."""
        page = server.resource500.render(self.request)
        self.assertSubstring('Bad News Bears', page)
        self.assertSubstring(b'Bad News Bears', page)

    def test_maintenance(self):
        """``server.maintenance`` should display the error-503.html page."""
        page = server.maintenance.render(self.request)
        self.assertSubstring('Under Maintenance', page)
        self.assertSubstring(b'Under Maintenance', page)


class CustomErrorHandlingResourceTests(unittest.TestCase):
@@ -214,7 +214,7 @@ class IndexResourceTests(unittest.TestCase):
        request = DummyRequest([self.pagename])
        request.method = b'GET'
        page = self.indexResource.render_GET(request)
        self.assertSubstring("add the bridges to Tor Browser", page)
        self.assertSubstring(b"add the bridges to Tor Browser", page)

    def test_IndexResource_render_GET_lang_ta(self):
        """renderGet() with ?lang=ta should return the index page in Tamil."""
@@ -224,9 +224,9 @@ class IndexResourceTests(unittest.TestCase):

        request = DummyRequest([self.pagename])
        request.method = b'GET'
        request.addArg('lang', 'ta')
        request.addArg(b'lang', 'ta')
        page = self.indexResource.render_GET(request)
        self.assertSubstring("bridge-களை Tor Browser-உள்", page)
        self.assertSubstring("bridge-களை Tor Browser-உள்".encode("utf-8"), page)


class HowtoResourceTests(unittest.TestCase):
@@ -243,7 +243,7 @@ class HowtoResourceTests(unittest.TestCase):
        request = DummyRequest([self.pagename])
        request.method = b'GET'
        page = self.howtoResource.render_GET(request)
        self.assertSubstring("the wizard", page)
        self.assertSubstring(b"the wizard", page)

    def test_HowtoResource_render_GET_lang_ru(self):
        """renderGet() with ?lang=ru should return the howto page in Russian."""
@@ -253,9 +253,9 @@ class HowtoResourceTests(unittest.TestCase):

        request = DummyRequest([self.pagename])
        request.method = b'GET'
        request.addArg('lang', 'ru')
        request.addArg(b'lang', 'ru')
        page = self.howtoResource.render_GET(request)
        self.assertSubstring("следовать инструкциям установщика", page)
        self.assertSubstring("следовать инструкциям установщика".encode("utf-8"), page)


class CaptchaProtectedResourceTests(unittest.TestCase):
@@ -279,7 +279,7 @@ class CaptchaProtectedResourceTests(unittest.TestCase):
        request.method = b'GET'
        page = self.captchaResource.render_GET(request)
        self.assertSubstring(
            "Your browser is not displaying images properly", page)
            b"Your browser is not displaying images properly", page)

    def test_render_GET_missingTemplate(self):
        """render_GET() with a missing template should raise an error and
@@ -816,10 +816,10 @@ class BridgesResourceTests(unittest.TestCase):
        request.headers.update({'accept-language': 'ar,en,en_US,'})

        page = self.bridgesResource.render(request)
        self.assertSubstring("rtl.css", page)
        self.assertSubstring(b"rtl.css", page)
        self.assertSubstring(
            # "I need an alternative way to get bridges!"
            "أحتاج إلى وسيلة بديلة للحصول على bridges", page)
            "أحتاج إلى وسيلة بديلة للحصول على bridges".encode("utf-8"), page)

        for bridgeLine in self.parseBridgesFromHTMLPage(page):
            # Check that each bridge line had the expected number of fields:
@@ -843,12 +843,12 @@ class BridgesResourceTests(unittest.TestCase):
        request.args.update({'transport': ['obfs3']})

        page = self.bridgesResource.render(request)
        self.assertSubstring("rtl.css", page)
        self.assertSubstring(b"rtl.css", page)
        self.assertSubstring(
            # "How to use the above bridge lines" (since there should be
            # bridges in this response, we don't tell them about alternative
            # mechanisms for getting bridges)
            "چگونگی از پل‌های خود استفاده کنید", page)
            "چگونگی از پل‌های خود استفاده کنید".encode("utf-8"), page)

        for bridgeLine in self.parseBridgesFromHTMLPage(page):
            # Check that each bridge line had the expected number of fields:
@@ -884,14 +884,14 @@ class BridgesResourceTests(unittest.TestCase):
        #
        # (Yes, there are two leading spaces at the beginning of each line)
        #
        bridgeLines = [line.strip() for line in page.strip().split('\n')]
        bridgeLines = [line.strip() for line in page.strip().split(b'\n')]

        for bridgeLine in bridgeLines:
            bridgeLine = bridgeLine.split(' ')
            bridgeLine = bridgeLine.split(b' ')
            self.assertEqual(len(bridgeLine), 2)

            # Check that the IP and port seem okay:
            ip, port = bridgeLine[0].rsplit(':')
            ip, port = bridgeLine[0].rsplit(b':')
            self.assertIsInstance(ipaddr.IPv4Address(ip), ipaddr.IPv4Address)
            self.assertIsInstance(int(port), int)
            self.assertGreater(int(port), 0)
@@ -913,8 +913,8 @@ class BridgesResourceTests(unittest.TestCase):
        page = self.bridgesResource.renderAnswer(request, bridgeLines=None)

        # We don't want the fancy version:
        self.assertNotSubstring("Bad News Bears", page)
        self.assertSubstring("Sorry! Something went wrong with your request.",
        self.assertNotSubstring(b"Bad News Bears", page)
        self.assertSubstring(b"Sorry! Something went wrong with your request.",
                             page)


@@ -944,8 +944,8 @@ class OptionsResourceTests(unittest.TestCase):
        request.args.update({'transport': ['obfs2']})

        page = self.optionsResource.render(request)
        self.assertSubstring("rtl.css", page)
        self.assertSubstring("מהם גשרים?", page)
        self.assertSubstring(b"rtl.css", page)
        self.assertSubstring("מהם גשרים?".encode("utf-8"), page)


class HTTPSServerServiceTests(unittest.TestCase):
+3 −3
Original line number Diff line number Diff line
@@ -40,8 +40,8 @@ class TranslationsMiscTests(unittest.TestCase):
        request = DummyRequest([b"bridges"])
        request.headers.update(REALISH_HEADERS)
        request.args.update({
            b'transport': [b'obfs3',],
            b'lang': [b'ar',],
            'transport': ['obfs3',],
            'lang': ['ar',],
        })

        parsed = translations.getLocaleFromHTTPRequest(request)
@@ -59,7 +59,7 @@ class TranslationsMiscTests(unittest.TestCase):
        """
        request = DummyRequest([b"options"])
        request.headers.update(ACCEPT_LANGUAGE_HEADER)
        request.args.update({b'lang': [b'fa']})
        request.args.update({'lang': ['fa']})

        parsed = translations.getLocaleFromHTTPRequest(request)

+1 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ def getLocaleFromHTTPRequest(request):
        logging.debug("Client Accept-Language (top 5): %s" % langs[:5])

    # Check if we got a ?lang=foo argument, and if we did, insert it first
    chosenLang = request.args.get(b"lang", [None,])[0]
    chosenLang = request.args.get("lang", [None,])[0]
    if chosenLang:
        logging.debug("Client requested language: %r" % chosenLang)
        langs.insert(0, chosenLang)