diff --git a/swift/common/swob.py b/swift/common/swob.py index af30bd9419..5ad202638a 100644 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -163,7 +163,7 @@ def _header_property(header): (Used by both request and response) If a value of None is given, the header is deleted. - :param header: name of the header, e.g. "Content-Length" + :param header: name of the header, e.g. "Transfer-Encoding" """ def getter(self): return self.headers.get(header, None) @@ -905,6 +905,46 @@ def split_path(self, minsegs=1, maxsegs=None, rest_with_last=False): self.environ.get('SCRIPT_NAME', '') + self.environ['PATH_INFO'], minsegs, maxsegs, rest_with_last) + def message_length(self): + """ + Properly determine the message length for this request. It will return + an integer if the headers explicitly contain the message length, or + None if the headers don't contain a length. The ValueError exception + will be raised if the headers are invalid. + + :raises ValueError: if either transfer-encoding or content-length + headers have bad values + :raises AttributeError: if the last value of the transfer-encoding + header is not "chunked" + """ + te = self.headers.get('transfer-encoding') + if te: + encodings = te.split(',') + if len(encodings) > 1: + raise AttributeError('Unsupported Transfer-Coding header' + ' value specified in Transfer-Encoding' + ' header') + # If there are more than one transfer encoding value, the last + # one must be chunked, see RFC 2616 Sec. 3.6 + if encodings[-1].lower() == 'chunked': + chunked = True + else: + raise ValueError('Invalid Transfer-Encoding header value') + else: + chunked = False + if not chunked: + # Because we are not using chunked transfer encoding we can pay + # attention to the content-length header. + fsize = self.headers.get('content-length', None) + if fsize is not None: + try: + fsize = int(fsize) + except ValueError: + raise ValueError('Invalid Content-Length header value') + else: + fsize = None + return fsize + def content_range_header_value(start, stop, size): return 'bytes %s-%s/%s' % (start, (stop - 1), size) @@ -1139,6 +1179,7 @@ def __getitem__(self, key): HTTPClientDisconnect = status_map[499] HTTPServerError = status_map[500] HTTPInternalServerError = status_map[500] +HTTPNotImplemented = status_map[501] HTTPBadGateway = status_map[502] HTTPServiceUnavailable = status_map[503] HTTPInsufficientStorage = status_map[507] diff --git a/swift/obj/server.py b/swift/obj/server.py index 4b716c0c12..82a99c8e09 100644 --- a/swift/obj/server.py +++ b/swift/obj/server.py @@ -716,6 +716,11 @@ def PUT(self, request): if new_delete_at and new_delete_at < time.time(): return HTTPBadRequest(body='X-Delete-At in past', request=request, content_type='text/plain') + try: + fsize = request.message_length() + except ValueError as e: + return HTTPBadRequest(body=e.message, request=request, + content_type='text/plain') if self.mount_check and not check_mount(self.devices, device): return HTTPInsufficientStorage(drive=device, request=request) disk_file = DiskFile(self.devices, device, partition, account, @@ -726,9 +731,6 @@ def PUT(self, request): orig_timestamp = disk_file.metadata.get('X-Timestamp') upload_expiration = time.time() + self.max_upload_time etag = md5() - fsize = request.headers.get('content-length', None) - if fsize is not None: - fsize = int(fsize) elapsed_time = 0 try: with disk_file.writer(size=fsize) as writer: diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index 5bdd9be362..c3a1844d5e 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -53,7 +53,7 @@ from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \ HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPRequestTimeout, \ HTTPServerError, HTTPServiceUnavailable, Request, Response, \ - HTTPClientDisconnect + HTTPClientDisconnect, HTTPNotImplemented def segment_listing_iter(listing): @@ -703,6 +703,16 @@ def PUT(self, req): return aresp if not containers: return HTTPNotFound(request=req) + try: + ml = req.message_length() + except ValueError as e: + return HTTPBadRequest(request=req, content_type='text/plain', + body=e.message) + except AttributeError as e: + return HTTPNotImplemented(request=req, content_type='text/plain', + body=e.message) + if ml is not None and ml > MAX_FILE_SIZE: + return HTTPRequestEntityTooLarge(request=req) if 'x-delete-after' in req.headers: try: x_delete_after = int(req.headers['x-delete-after']) @@ -867,7 +877,8 @@ def PUT(self, req): node_iter = GreenthreadSafeIterator( self.iter_nodes(self.app.object_ring, partition)) pile = GreenPile(len(nodes)) - chunked = req.headers.get('transfer-encoding') + te = req.headers.get('transfer-encoding', '') + chunked = ('chunked' in te) outgoing_headers = self._backend_requests( req, len(nodes), container_partition, containers, diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index 44b4798a3d..2eadd51ccb 100644 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -276,6 +276,9 @@ def test_accept_invalid(self): 'text/xml']) self.assertEquals(match, None) + def test_repr(self): + acc = swift.common.swob.Accept("application/json") + self.assertEquals(repr(acc), "application/json") class TestRequest(unittest.TestCase): def test_blank(self): @@ -437,6 +440,11 @@ def test_path_qs(self): 'QUERY_STRING': 'hello=equal&acl'}) self.assertEqual(req.path_qs, '/hi/there?hello=equal&acl') + def test_url(self): + req = swift.common.swob.Request.blank('/hi/there?hello=equal&acl') + self.assertEqual(req.url, + 'http://localhost/hi/there?hello=equal&acl') + def test_wsgify(self): used_req = [] @@ -566,6 +574,60 @@ def test_as_referer(self): exp_url = '%(sche)s://%(host)s%(pi)s?%(qs)s' % locals() self.assertEqual(req.as_referer(), 'POST ' + exp_url) + def test_message_length_just_content_length(self): + req = swift.common.swob.Request.blank( + u'/', + environ={'REQUEST_METHOD': 'PUT', 'PATH_INFO': '/'}) + self.assertEquals(req.message_length(), None) + + req = swift.common.swob.Request.blank( + u'/', + environ={'REQUEST_METHOD': 'PUT', 'PATH_INFO': '/'}, + body='x'*42) + self.assertEquals(req.message_length(), 42) + + req.headers['Content-Length'] = 'abc' + try: + l = req.message_length() + except ValueError as e: + self.assertEquals(e.message, "Invalid Content-Length header value") + else: + self.fail("Expected a ValueError raised for 'abc'") + + def test_message_length_transfer_encoding(self): + req = swift.common.swob.Request.blank( + u'/', + environ={'REQUEST_METHOD': 'PUT', 'PATH_INFO': '/'}, + headers={'transfer-encoding': 'chunked'}, + body='x'*42) + self.assertEquals(req.message_length(), None) + + req.headers['Transfer-Encoding'] = 'gzip,chunked' + try: + l = req.message_length() + except AttributeError as e: + self.assertEquals(e.message, "Unsupported Transfer-Coding header" + " value specified in Transfer-Encoding header") + else: + self.fail("Expected an AttributeError raised for 'gzip'") + + req.headers['Transfer-Encoding'] = 'gzip' + try: + l = req.message_length() + except ValueError as e: + self.assertEquals(e.message, "Invalid Transfer-Encoding header value") + else: + self.fail("Expected a ValueError raised for 'gzip'") + + req.headers['Transfer-Encoding'] = 'gzip,identity' + try: + l = req.message_length() + except AttributeError as e: + self.assertEquals(e.message, "Unsupported Transfer-Coding header" + " value specified in Transfer-Encoding header") + else: + self.fail("Expected an AttributeError raised for 'gzip,identity'") + class TestStatusMap(unittest.TestCase): def test_status_map(self): diff --git a/test/unit/obj/test_server.py b/test/unit/obj/test_server.py index 34410c1d49..647d955d7f 100755 --- a/test/unit/obj/test_server.py +++ b/test/unit/obj/test_server.py @@ -671,15 +671,6 @@ def test_PUT_zero_content_length(self): resp = self.object_controller.PUT(req) self.assertEquals(resp.status_int, 201) - def test_PUT_zero_content_length_mismatched(self): - req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'}, - headers={'X-Timestamp': normalize_timestamp(time()), - 'Content-Type': 'application/octet-stream'}) - req.body = 'VERIFY' - req.headers['Content-Length'] = '0' - resp = self.object_controller.PUT(req) - self.assertEquals(resp.status_int, 499) - def test_PUT_common(self): timestamp = normalize_timestamp(time()) req = Request.blank('/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'}, @@ -1528,13 +1519,46 @@ def test_chunked_put(self): 'Transfer-Encoding: chunked\r\n\r\n' '2\r\noh\r\n4\r\n hai\r\n0\r\n\r\n') fd.flush() - readuntil2crlfs(fd) + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 201' + self.assertEquals(headers[:len(exp)], exp) + sock = connect_tcp(('localhost', port)) + fd = sock.makefile() + fd.write('GET /sda1/p/a/c/o HTTP/1.1\r\nHost: localhost\r\n' + 'Connection: close\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 200' + self.assertEquals(headers[:len(exp)], exp) + response = fd.read() + self.assertEquals(response, 'oh hai') + killer.kill() + + def test_chunked_content_length_mismatch_zero(self): + listener = listen(('localhost', 0)) + port = listener.getsockname()[1] + killer = spawn(wsgi.server, listener, self.object_controller, + NullLogger()) + sock = connect_tcp(('localhost', port)) + fd = sock.makefile() + fd.write('PUT /sda1/p/a/c/o HTTP/1.1\r\nHost: localhost\r\n' + 'Content-Type: text/plain\r\n' + 'Connection: close\r\nX-Timestamp: 1.0\r\n' + 'Content-Length: 0\r\n' + 'Transfer-Encoding: chunked\r\n\r\n' + '2\r\noh\r\n4\r\n hai\r\n0\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 201' + self.assertEquals(headers[:len(exp)], exp) sock = connect_tcp(('localhost', port)) fd = sock.makefile() fd.write('GET /sda1/p/a/c/o HTTP/1.1\r\nHost: localhost\r\n' 'Connection: close\r\n\r\n') fd.flush() - readuntil2crlfs(fd) + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 200' + self.assertEquals(headers[:len(exp)], exp) response = fd.read() self.assertEquals(response, 'oh hai') killer.kill() diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 603e5643ad..1abc115d18 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -805,6 +805,174 @@ def test_connect(ipaddr, port, device, partition, method, path, res = controller.PUT(req) self.assertTrue(res.status.startswith('201 ')) + def test_PUT_message_length_using_content_length(self): + prolis = _test_sockets[0] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + obj = 'j' * 20 + fd.write('PUT /v1/a/c/o.content-length HTTP/1.1\r\n' + 'Host: localhost\r\n' + 'Connection: close\r\n' + 'X-Storage-Token: t\r\n' + 'Content-Length: %s\r\n' + 'Content-Type: application/octet-stream\r\n' + '\r\n%s' % (str(len(obj)), obj)) + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 201' + self.assertEqual(headers[:len(exp)], exp) + + def test_PUT_message_length_using_transfer_encoding(self): + prolis = _test_sockets[0] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('PUT /v1/a/c/o.chunked HTTP/1.1\r\n' + 'Host: localhost\r\n' + 'Connection: close\r\n' + 'X-Storage-Token: t\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Transfer-Encoding: chunked\r\n\r\n' + '2\r\n' + 'oh\r\n' + '4\r\n' + ' say\r\n' + '4\r\n' + ' can\r\n' + '4\r\n' + ' you\r\n' + '4\r\n' + ' see\r\n' + '3\r\n' + ' by\r\n' + '4\r\n' + ' the\r\n' + '8\r\n' + ' dawns\'\n\r\n' + '0\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 201' + self.assertEqual(headers[:len(exp)], exp) + + def test_PUT_message_length_using_both(self): + prolis = _test_sockets[0] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('PUT /v1/a/c/o.chunked HTTP/1.1\r\n' + 'Host: localhost\r\n' + 'Connection: close\r\n' + 'X-Storage-Token: t\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Content-Length: 33\r\n' + 'Transfer-Encoding: chunked\r\n\r\n' + '2\r\n' + 'oh\r\n' + '4\r\n' + ' say\r\n' + '4\r\n' + ' can\r\n' + '4\r\n' + ' you\r\n' + '4\r\n' + ' see\r\n' + '3\r\n' + ' by\r\n' + '4\r\n' + ' the\r\n' + '8\r\n' + ' dawns\'\n\r\n' + '0\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 201' + self.assertEqual(headers[:len(exp)], exp) + + def test_PUT_bad_message_length(self): + prolis = _test_sockets[0] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('PUT /v1/a/c/o.chunked HTTP/1.1\r\n' + 'Host: localhost\r\n' + 'Connection: close\r\n' + 'X-Storage-Token: t\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Content-Length: 33\r\n' + 'Transfer-Encoding: gzip\r\n\r\n' + '2\r\n' + 'oh\r\n' + '4\r\n' + ' say\r\n' + '4\r\n' + ' can\r\n' + '4\r\n' + ' you\r\n' + '4\r\n' + ' see\r\n' + '3\r\n' + ' by\r\n' + '4\r\n' + ' the\r\n' + '8\r\n' + ' dawns\'\n\r\n' + '0\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 400' + self.assertEqual(headers[:len(exp)], exp) + + def test_PUT_message_length_unsup_xfr_encoding(self): + prolis = _test_sockets[0] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('PUT /v1/a/c/o.chunked HTTP/1.1\r\n' + 'Host: localhost\r\n' + 'Connection: close\r\n' + 'X-Storage-Token: t\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Content-Length: 33\r\n' + 'Transfer-Encoding: gzip,chunked\r\n\r\n' + '2\r\n' + 'oh\r\n' + '4\r\n' + ' say\r\n' + '4\r\n' + ' can\r\n' + '4\r\n' + ' you\r\n' + '4\r\n' + ' see\r\n' + '3\r\n' + ' by\r\n' + '4\r\n' + ' the\r\n' + '8\r\n' + ' dawns\'\n\r\n' + '0\r\n\r\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 501' + self.assertEqual(headers[:len(exp)], exp) + + def test_PUT_message_length_too_large(self): + swift.proxy.controllers.obj.MAX_FILE_SIZE = 10 + try: + prolis = _test_sockets[0] + sock = connect_tcp(('localhost', prolis.getsockname()[1])) + fd = sock.makefile() + fd.write('PUT /v1/a/c/o.chunked HTTP/1.1\r\n' + 'Host: localhost\r\n' + 'Connection: close\r\n' + 'X-Storage-Token: t\r\n' + 'Content-Type: application/octet-stream\r\n' + 'Content-Length: 33\r\n\r\n' + 'oh say can you see by the dawns\'\n') + fd.flush() + headers = readuntil2crlfs(fd) + exp = 'HTTP/1.1 413' + self.assertEqual(headers[:len(exp)], exp) + finally: + swift.proxy.controllers.obj.MAX_FILE_SIZE = MAX_FILE_SIZE + def test_expirer_DELETE_on_versioned_object(self): test_errors = []