From 49f62f6ab7fd1b833e9b5bfbcaafa4b45b592d34 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 12 Sep 2019 10:59:08 -0700 Subject: [PATCH] bufferedhttp: ensure query params are properly quoted Recent versions of py27 [1] have begun raising InvalidURL if you try to include non-ASCII characters in the request path. This was observed recently in the periodic checks of stable/ocata and stable/pike. In particular, we would spin up some in-process servers in test.unit.proxy.test_server.TestSocketObjectVersions and do a container listing with a prefix param that included raw (unquoted) UTF-8. This query string would pass unmolested through the proxy, tripping the InvalidURL error when bufferedhttp called putrequest. More recent versions of Swift would not exhibit this particular failure, as the listing_formats middleware would force a decoding/re-encoding of the query string for account and container requests. However, object requests with errant query strings would likely be able to trip the same error. Swift on py3 should not exhibit this behavior, as we so thoroughly re-write the request line to avoid hitting https://bugs.python.org/issue33973. Now, always parse and re-encode the query string in bufferedhttp. This prevents any errors on object requests and cleans up any callers that might use bufferedhttp directly. [1] Anything after https://github.com/python/cpython/commit/bb8071a; see https://bugs.python.org/issue30458 Closes-Bug: 1843816 Change-Id: I73f84b96f164e6fc5d3cb890355871c26ed271a6 Related-Change: Id3ce37aa0402e2d8dd5784ce329d7cb4fbaf700d Related-Change: Ie648f5c04d4415f3b620fb196fa567ce7575d522 --- swift/common/bufferedhttp.py | 11 ++++++++++- test/unit/common/test_bufferedhttp.py | 5 +++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/swift/common/bufferedhttp.py b/swift/common/bufferedhttp.py index 0fed79ed2e..e95b81bf8c 100644 --- a/swift/common/bufferedhttp.py +++ b/swift/common/bufferedhttp.py @@ -34,7 +34,7 @@ import socket import eventlet from eventlet.green.httplib import CONTINUE, HTTPConnection, HTTPMessage, \ HTTPResponse, HTTPSConnection, _UNKNOWN -from six.moves.urllib.parse import quote +from six.moves.urllib.parse import quote, parse_qsl, urlencode import six if six.PY2: @@ -292,6 +292,15 @@ def http_connect_raw(ipaddr, port, method, path, headers=None, else: conn = BufferedHTTPConnection('%s:%s' % (ipaddr, port)) if query_string: + # Round trip to ensure proper quoting + if six.PY2: + query_string = urlencode(parse_qsl( + query_string, keep_blank_values=True)) + else: + query_string = urlencode( + parse_qsl(query_string, keep_blank_values=True, + encoding='latin1'), + encoding='latin1') path += '?' + query_string conn.path = path conn.putrequest(method, path, skip_host=(headers and 'Host' in headers)) diff --git a/test/unit/common/test_bufferedhttp.py b/test/unit/common/test_bufferedhttp.py index 7e0c9ec03e..38e62856cd 100644 --- a/test/unit/common/test_bufferedhttp.py +++ b/test/unit/common/test_bufferedhttp.py @@ -56,7 +56,8 @@ class TestBufferedHTTP(unittest.TestCase): b'RESPONSE') fp.flush() line = fp.readline() - path = b'/dev/' + expected_par + b'/path/..%25/?omg&no=%7f' + path = (b'/dev/' + expected_par + + b'/path/..%25/?omg=&no=%7F&%FF=%FF&no=%25ff') self.assertEqual( line, b'PUT ' + path + b' HTTP/1.1\r\n') @@ -83,7 +84,7 @@ class TestBufferedHTTP(unittest.TestCase): 'PUT', '/path/..%/', { 'content-length': 7, 'x-header': 'value'}, - query_string='omg&no=%7f') + query_string='omg&no=%7f&\xff=%ff&no=%25ff') conn.send(b'REQUEST\r\n') self.assertTrue(conn.sock.getsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY))