[tor-commits] [bridgedb/master] Fix remaining str/bytes issues in HTTPS server.
phw at torproject.org
phw at torproject.org
Wed Feb 19 18:26:38 UTC 2020
commit 26c2708f2449bd83ab7361f560ffbb0d870e8f51
Author: Philipp Winter <phw at 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)
More information about the tor-commits
mailing list