commit 26c2708f2449bd83ab7361f560ffbb0d870e8f51 Author: Philipp Winter phw@nymity.ch Date: Tue Jan 28 09:33:00 2020 -0800

Fix remaining str/bytes issues in HTTPS server. --- bridgedb/distributors/https/server.py | 85 ++++++++++++++++++++++------------- bridgedb/test/test_https.py | 2 +- bridgedb/test/test_https_server.py | 52 ++++++++++----------- bridgedb/test/test_translations.py | 6 +-- bridgedb/translations.py | 2 +- 5 files changed, 86 insertions(+), 61 deletions(-)

diff --git a/bridgedb/distributors/https/server.py b/bridgedb/distributors/https/server.py index b33065f..2d230b9 100644 --- a/bridgedb/distributors/https/server.py +++ b/bridgedb/distributors/https/server.py @@ -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 diff --git a/bridgedb/test/test_https.py b/bridgedb/test/test_https.py index b6c3e0f..52ad936 100644 --- a/bridgedb/test/test_https.py +++ b/bridgedb/test/test_https.py @@ -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?") diff --git a/bridgedb/test/test_https_server.py b/bridgedb/test/test_https_server.py index a25a99d..8482d9d 100644 --- a/bridgedb/test/test_https_server.py +++ b/bridgedb/test/test_https_server.py @@ -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): diff --git a/bridgedb/test/test_translations.py b/bridgedb/test/test_translations.py index 939511d..4b504f1 100644 --- a/bridgedb/test/test_translations.py +++ b/bridgedb/test/test_translations.py @@ -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)

diff --git a/bridgedb/translations.py b/bridgedb/translations.py index 194fa8a..b6a9ef3 100644 --- a/bridgedb/translations.py +++ b/bridgedb/translations.py @@ -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)