Repository: tornadoweb/tornado

Issue:
Unable to use non-ascii characters in user/password for basic auth in curl_httpclient
Steps to reproduce (Python 3.4):
1. Create tornado.httpclient.HTTPRequest with auth_username or auth_password which contains non-ascii (lower range, 0-128), for example pound sterling £ (which is 153 in ascii).
2. Execute curl_httpclient fetch using that request

Expected result:
1. The request is successfully completed

Actual result:
2. HTTP 599 is returned and internal exception is:
'ascii' codec can't encode character '\xa3' in position 55: ordinal not in range(128)


I am not sure if I am not aware of the proper solution, but I have tried providing bytes as auth_password, but it does not solve the issue because https://github.com/tornadoweb/tornado/blob/master/tornado/curl_httpclient.py#L438 internally uses string formatting. Reading through pycurl docs (http://pycurl.io/docs/latest/unicode.html) suggests that for Python3 bytes array should be used when using curl setopt. It seems like Python3 vs Python2 issue?

Discussion:
ploxiln: functionally looks great

Patch:
diff --git a/tornado/curl_httpclient.py b/tornado/curl_httpclient.py
index 54fc5b36..ef98225c 100644
--- a/tornado/curl_httpclient.py
+++ b/tornado/curl_httpclient.py
@@ -348,8 +348,8 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
             curl.setopt(pycurl.PROXY, request.proxy_host)
             curl.setopt(pycurl.PROXYPORT, request.proxy_port)
             if request.proxy_username:
-                credentials = '%s:%s' % (request.proxy_username,
-                                         request.proxy_password)
+                credentials = httputil.encode_username_password(request.proxy_username,
+                                                                request.proxy_password)
                 curl.setopt(pycurl.PROXYUSERPWD, credentials)
 
             if (request.proxy_auth_mode is None or
@@ -441,8 +441,6 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
                 curl.setopt(pycurl.INFILESIZE, len(request.body or ''))
 
         if request.auth_username is not None:
-            userpwd = "%s:%s" % (request.auth_username, request.auth_password or '')
-
             if request.auth_mode is None or request.auth_mode == "basic":
                 curl.setopt(pycurl.HTTPAUTH, pycurl.HTTPAUTH_BASIC)
             elif request.auth_mode == "digest":
@@ -450,7 +448,9 @@ class CurlAsyncHTTPClient(AsyncHTTPClient):
             else:
                 raise ValueError("Unsupported auth_mode %s" % request.auth_mode)
 
-            curl.setopt(pycurl.USERPWD, native_str(userpwd))
+            userpwd = httputil.encode_username_password(request.auth_username,
+                                                        request.auth_password)
+            curl.setopt(pycurl.USERPWD, userpwd)
             curl_log.debug("%s %s (username: %r)", request.method, request.url,
                            request.auth_username)
         else:
diff --git a/tornado/httputil.py b/tornado/httputil.py
index 22a64c31..d1ace5a8 100644
--- a/tornado/httputil.py
+++ b/tornado/httputil.py
@@ -29,11 +29,12 @@ import email.utils
 import numbers
 import re
 import time
+import unicodedata
 import warnings
 
 from tornado.escape import native_str, parse_qs_bytes, utf8
 from tornado.log import gen_log
-from tornado.util import ObjectDict, PY3
+from tornado.util import ObjectDict, PY3, unicode_type
 
 if PY3:
     import http.cookies as Cookie
@@ -949,6 +950,20 @@ def _encode_header(key, pdict):
     return '; '.join(out)
 
 
+def encode_username_password(username, password):
+    """Encodes a username/password pair in the format used by HTTP auth.
+
+    The return value is a byte string in the form ``username:password``.
+
+    .. versionadded:: 5.1
+    """
+    if isinstance(username, unicode_type):
+        username = unicodedata.normalize('NFC', username)
+    if isinstance(password, unicode_type):
+        password = unicodedata.normalize('NFC', password)
+    return utf8(username) + b":" + utf8(password)
+
+
 def doctests():
     import doctest
     return doctest.DocTestSuite()
diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py
index 4df4898a..35c71936 100644
--- a/tornado/simple_httpclient.py
+++ b/tornado/simple_httpclient.py
@@ -1,6 +1,6 @@
 from __future__ import absolute_import, division, print_function
 
-from tornado.escape import utf8, _unicode
+from tornado.escape import _unicode
 from tornado import gen
 from tornado.httpclient import HTTPResponse, HTTPError, AsyncHTTPClient, main, _RequestProxy
 from tornado import httputil
@@ -308,9 +308,9 @@ class _HTTPConnection(httputil.HTTPMessageDelegate):
                     if self.request.auth_mode not in (None, "basic"):
                         raise ValueError("unsupported auth_mode %s",
                                          self.request.auth_mode)
-                    auth = utf8(username) + b":" + utf8(password)
-                    self.request.headers["Authorization"] = (b"Basic " +
-                                                             base64.b64encode(auth))
+                    self.request.headers["Authorization"] = (
+                        b"Basic " + base64.b64encode(
+                            httputil.encode_username_password(username, password)))
                 if self.request.user_agent:
                     self.request.headers["User-Agent"] = self.request.user_agent
                 if not self.request.allow_nonstandard_methods:


Test patch:
diff --git a/tornado/test/curl_httpclient_test.py b/tornado/test/curl_httpclient_test.py
index b7a85952..4230d4cd 100644
--- a/tornado/test/curl_httpclient_test.py
+++ b/tornado/test/curl_httpclient_test.py
@@ -32,13 +32,15 @@ class CurlHTTPClientCommonTestCase(httpclient_test.HTTPClientCommonTestCase):
 
 
 class DigestAuthHandler(RequestHandler):
+    def initialize(self, username, password):
+        self.username = username
+        self.password = password
+
     def get(self):
         realm = 'test'
         opaque = 'asdf'
         # Real implementations would use a random nonce.
         nonce = "1234"
-        username = 'foo'
-        password = 'bar'
 
         auth_header = self.request.headers.get('Authorization', None)
         if auth_header is not None:
@@ -53,9 +55,9 @@ class DigestAuthHandler(RequestHandler):
             assert param_dict['realm'] == realm
             assert param_dict['opaque'] == opaque
             assert param_dict['nonce'] == nonce
-            assert param_dict['username'] == username
+            assert param_dict['username'] == self.username
             assert param_dict['uri'] == self.request.path
-            h1 = md5(utf8('%s:%s:%s' % (username, realm, password))).hexdigest()
+            h1 = md5(utf8('%s:%s:%s' % (self.username, realm, self.password))).hexdigest()
             h2 = md5(utf8('%s:%s' % (self.request.method,
                                      self.request.path))).hexdigest()
             digest = md5(utf8('%s:%s:%s' % (h1, nonce, h2))).hexdigest()
@@ -88,7 +90,8 @@ class CurlHTTPClientTestCase(AsyncHTTPTestCase):
 
     def get_app(self):
         return Application([
-            ('/digest', DigestAuthHandler),
+            ('/digest', DigestAuthHandler, {'username': 'foo', 'password': 'bar'}),
+            ('/digest_non_ascii', DigestAuthHandler, {'username': 'foo', 'password': 'barユ£'}),
             ('/custom_reason', CustomReasonHandler),
             ('/custom_fail_reason', CustomFailReasonHandler),
         ])
@@ -143,3 +146,8 @@ class CurlHTTPClientTestCase(AsyncHTTPTestCase):
                 # during the setup phase doesn't lead the request to
                 # be dropped on the floor.
                 response = self.fetch(u'/ユニコード', raise_error=True)
+
+    def test_digest_auth_non_ascii(self):
+        response = self.fetch('/digest_non_ascii', auth_mode='digest',
+                              auth_username='foo', auth_password='barユ£')
+        self.assertEqual(response.body, b'ok')
diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py
index 60c8f490..fb8b12d5 100644
--- a/tornado/test/httpclient_test.py
+++ b/tornado/test/httpclient_test.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 from __future__ import absolute_import, division, print_function
 
 import base64
@@ -8,6 +9,7 @@ import sys
 import threading
 import datetime
 from io import BytesIO
+import unicodedata
 
 from tornado.escape import utf8, native_str
 from tornado import gen
@@ -237,6 +239,7 @@ Transfer-Encoding: chunked
         self.assertIs(exc_info[0][0], ZeroDivisionError)
 
     def test_basic_auth(self):
+        # This test data appears in section 2 of RFC 7617.
         self.assertEqual(self.fetch("/auth", auth_username="Aladdin",
                                     auth_password="open sesame").body,
                          b"Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==")
@@ -247,6 +250,20 @@ Transfer-Encoding: chunked
                                     auth_mode="basic").body,
                          b"Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==")
 
+    def test_basic_auth_unicode(self):
+        # This test data appears in section 2.1 of RFC 7617.
+        self.assertEqual(self.fetch("/auth", auth_username="test",
+                                    auth_password="123£").body,
+                         b"Basic dGVzdDoxMjPCow==")
+
+        # The standard mandates NFC. Give it a decomposed username
+        # and ensure it is normalized to composed form.
+        username = unicodedata.normalize("NFD", u"josé")
+        self.assertEqual(self.fetch("/auth",
+                                    auth_username=username,
+                                    auth_password="səcrət").body,
+                         b"Basic am9zw6k6c8mZY3LJmXQ=")
+
     def test_unsupported_auth_mode(self):
         # curl and simple clients handle errors a bit differently; the
         # important thing is that they don't fall back to basic auth