Merge "Don't try to read a body if the client didn't send one"

swift3/acl_handlers.py

@@ -61,8 +61,7 @@ def get_acl(headers, body, bucket_owner, object_owner=None):
     if acl is None:
         # Get acl from request body if possible.
         if not body:
-            msg = 'Your request was missing a required header'
-            raise MissingSecurityHeader(msg, missing_header_name='x-amz-acl')
+            raise MissingSecurityHeader(missing_header_name='x-amz-acl')
         try:
             elem = fromstring(body, ACL.root_tag)
             acl = ACL.from_elem(elem)

swift3/controllers/acl.py

@@ -20,7 +20,7 @@ from swift.common.utils import public
 from swift3.exception import ACLError
 from swift3.controllers.base import Controller
 from swift3.response import HTTPOk, S3NotImplemented, MalformedACLError, \
-    UnexpectedContent
+    UnexpectedContent, MissingSecurityHeader
 from swift3.etree import Element, SubElement, tostring
 from swift3.acl_utils import swift_acl_translate, XMLNS_XSI
 
@@ -103,18 +103,24 @@ class AclController(Controller):
         else:
             # Handle Bucket ACL
             xml = req.xml(MAX_ACL_BODY_SIZE)
-            if 'HTTP_X_AMZ_ACL' in req.environ and xml:
+            if all(['HTTP_X_AMZ_ACL' in req.environ, xml]):
                 # S3 doesn't allow to give ACL with both ACL header and body.
                 raise UnexpectedContent()
-            elif xml and 'HTTP_X_AMZ_ACL' not in req.environ:
-                # We very likely have an XML-based ACL request.
-                try:
-                    translated_acl = swift_acl_translate(xml, xml=True)
-                except ACLError:
-                    raise MalformedACLError()
+            elif not any(['HTTP_X_AMZ_ACL' in req.environ, xml]):
+                # Both canned ACL header and xml body are missing
+                raise MissingSecurityHeader(missing_header_name='x-amz-acl')
+            else:
+                # correct ACL exists in the request
+                if xml:
+                    # We very likely have an XML-based ACL request.
+                    # let's try to translate to the request header
+                    try:
+                        translated_acl = swift_acl_translate(xml, xml=True)
+                    except ACLError:
+                        raise MalformedACLError()
 
-                for header, acl in translated_acl:
-                    req.headers[header] = acl
+                    for header, acl in translated_acl:
+                        req.headers[header] = acl
 
             resp = req.get_response(self.app, 'POST')
             resp.status = HTTP_OK

swift3/controllers/multi_delete.py

@@ -21,7 +21,8 @@ from swift3.controllers.base import Controller, bucket_operation
 from swift3.etree import Element, SubElement, fromstring, tostring, \
     XMLSyntaxError, DocumentInvalid
 from swift3.response import HTTPOk, S3NotImplemented, NoSuchKey, \
-    ErrorResponse, MalformedXML, UserKeyMustBeSpecified, AccessDenied
+    ErrorResponse, MalformedXML, UserKeyMustBeSpecified, AccessDenied, \
+    MissingRequestBodyError
 from swift3.cfg import CONF
 from swift3.utils import LOGGER
 
@@ -64,7 +65,11 @@ class MultiObjectDeleteController(Controller):
                 yield key, version
 
         try:
-            xml = req.xml(MAX_MULTI_DELETE_BODY_SIZE, check_md5=True)
+            xml = req.xml(MAX_MULTI_DELETE_BODY_SIZE)
+            if not xml:
+                raise MissingRequestBodyError()
+
+            req.check_md5(xml)
             elem = fromstring(xml, 'Delete')
 
             quiet = elem.find('./Quiet')

swift3/controllers/multi_upload.py

@@ -558,6 +558,9 @@ class UploadController(Controller):
         previous_number = 0
         try:
             xml = req.xml(MAX_COMPLETE_UPLOAD_BODY_SIZE)
+            if not xml:
+                raise InvalidRequest(msg='You must specify at least one part')
+
             complete_elem = fromstring(xml, 'CompleteMultipartUpload')
             for part_elem in complete_elem.iterchildren('Part'):
                 part_number = int(part_elem.find('./PartNumber').text)

swift3/request.py

@@ -682,7 +682,7 @@ class Request(swob.Request):
         """
         raise AttributeError("No attribute 'body'")
 
-    def xml(self, max_length, check_md5=False):
+    def xml(self, max_length):
         """
         Similar to swob.Request.body, but it checks the content length before
         creating a body string.
@@ -697,11 +697,13 @@ class Request(swob.Request):
         if self.message_length() > max_length:
             raise MalformedXML()
 
-        # Limit the read similar to how SLO handles manifests
-        body = self.body_file.read(max_length)
-
-        if check_md5:
-            self.check_md5(body)
+        if te or self.message_length():
+            # Limit the read similar to how SLO handles manifests
+            body = self.body_file.read(max_length)
+        else:
+            # No (or zero) Content-Length provided, and not chunked transfer;
+            # no body. Assume zero-length, and enforce a required body below.
+            return None
 
         return body
 

swift3/test/unit/helpers.py

@@ -159,3 +159,26 @@ class FakeSwift(object):
 
     def clear_calls(self):
         del self._calls[:]
+
+
+class UnreadableInput(object):
+    # Some clients will send neither a Content-Length nor a Transfer-Encoding
+    # header, which will cause (some versions of?) eventlet to bomb out on
+    # reads. This class helps us simulate that behavior.
+    def __init__(self, test_case):
+        self.calls = 0
+        self.test_case = test_case
+
+    def read(self, *a, **kw):
+        self.calls += 1
+        # Calling wsgi.input.read with neither a Content-Length nor
+        # a Transfer-Encoding header will raise TypeError (See
+        # https://bugs.launchpad.net/swift3/+bug/1593870 in detail)
+        # This unreadable class emulates the behavior
+        raise TypeError
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *args):
+        self.test_case.assertEqual(0, self.calls)

swift3/test/unit/test_acl.py

@@ -15,10 +15,12 @@
 
 import unittest
 from cStringIO import StringIO
+from hashlib import md5
 
 from swift.common.swob import Request, HTTPAccepted
 
 from swift3.test.unit import Swift3TestCase
+from swift3.test.unit.helpers import UnreadableInput
 from swift3.etree import fromstring, tostring, Element, SubElement, XMLNS_XSI
 from swift3.test.unit.test_s3_acl import s3acl
 import mock
@@ -125,6 +127,40 @@ class TestSwift3Acl(Swift3TestCase):
         self.assertEqual(self._get_error_code(body),
                          'UnexpectedContent')
 
+    def _test_put_no_body(self, use_content_length=False,
+                          use_transfer_encoding=False, string_to_md5=''):
+        content_md5 = md5(string_to_md5).digest().encode('base64').strip()
+        with UnreadableInput(self) as fake_input:
+            req = Request.blank(
+                '/bucket?acl',
+                environ={
+                    'REQUEST_METHOD': 'PUT',
+                    'wsgi.input': fake_input},
+                headers={
+                    'Authorization': 'AWS test:tester:hmac',
+                    'Date': self.get_date_header(),
+                    'Content-MD5': content_md5},
+                body='')
+            if not use_content_length:
+                req.environ.pop('CONTENT_LENGTH')
+            if use_transfer_encoding:
+                req.environ['HTTP_TRANSFER_ENCODING'] = 'chunked'
+            status, headers, body = self.call_swift3(req)
+        self.assertEqual(status, '400 Bad Request')
+        self.assertEqual(self._get_error_code(body), 'MissingSecurityHeader')
+        self.assertEqual(self._get_error_message(body),
+                         'Your request was missing a required header.')
+        self.assertIn('<MissingHeaderName>x-amz-acl</MissingHeaderName>', body)
+
+    @s3acl
+    def test_bucket_fails_with_neither_acl_header_nor_xml_PUT(self):
+        self._test_put_no_body()
+        self._test_put_no_body(string_to_md5='test')
+        self._test_put_no_body(use_content_length=True)
+        self._test_put_no_body(use_content_length=True, string_to_md5='test')
+        self._test_put_no_body(use_transfer_encoding=True)
+        self._test_put_no_body(use_transfer_encoding=True, string_to_md5='zz')
+
     def test_object_acl_GET(self):
         req = Request.blank('/bucket/object?acl',
                             environ={'REQUEST_METHOD': 'GET'},

swift3/test/unit/test_bucket.py

@@ -21,6 +21,7 @@ from swift.common.swob import Request
 from swift.common.utils import json
 
 from swift3.test.unit import Swift3TestCase
+from swift3.test.unit.helpers import UnreadableInput
 from swift3.etree import Element, SubElement, fromstring, tostring
 from swift3.test.unit.test_s3_acl import s3acl
 from swift3.subresource import Owner, encode_acl, ACLPublicRead
@@ -501,6 +502,7 @@ class TestSwift3Bucket(Swift3TestCase):
                             headers={'Authorization': 'AWS test:tester:hmac',
                                      'Date': self.get_date_header()})
         status, headers, body = self.call_swift3(req)
+        self.assertEqual(body, '')
         self.assertEqual(status.split()[0], '200')
         self.assertEqual(headers['Location'], '/bucket')
 
@@ -512,6 +514,19 @@ class TestSwift3Bucket(Swift3TestCase):
                                      'Date': self.get_date_header(),
                                      'Transfer-Encoding': 'chunked'})
         status, headers, body = self.call_swift3(req)
+        self.assertEqual(body, '')
+        self.assertEqual(status.split()[0], '200')
+        self.assertEqual(headers['Location'], '/bucket')
+
+        with UnreadableInput(self) as fake_input:
+            req = Request.blank(
+                '/bucket',
+                environ={'REQUEST_METHOD': 'PUT',
+                         'wsgi.input': fake_input},
+                headers={'Authorization': 'AWS test:tester:hmac',
+                         'Date': self.get_date_header()})
+            status, headers, body = self.call_swift3(req)
+        self.assertEqual(body, '')
         self.assertEqual(status.split()[0], '200')
         self.assertEqual(headers['Location'], '/bucket')
 

swift3/test/unit/test_multi_delete.py

@@ -22,6 +22,7 @@ from swift.common import swob
 from swift.common.swob import Request
 
 from swift3.test.unit import Swift3TestCase
+from swift3.test.unit.helpers import UnreadableInput
 from swift3.etree import fromstring, tostring, Element, SubElement
 from swift3.cfg import CONF
 from swift3.test.unit.test_s3_acl import s3acl
@@ -77,11 +78,10 @@ class TestSwift3MultiDelete(Swift3TestCase):
         req = Request.blank('/bucket?delete',
                             environ={'REQUEST_METHOD': 'POST'},
                             headers={'Authorization': 'AWS test:tester:hmac',
+                                     'Content-Type': 'multipart/form-data',
                                      'Date': self.get_date_header(),
                                      'Content-MD5': content_md5},
                             body=body)
-        req.date = datetime.now()
-        req.content_type = 'text/plain'
         status, headers, body = self.call_swift3(req)
         self.assertEqual(status.split()[0], '200')
 
@@ -249,5 +249,36 @@ class TestSwift3MultiDelete(Swift3TestCase):
         elem = fromstring(body)
         self.assertEqual(len(elem.findall('Deleted')), len(self.keys))
 
+    def _test_no_body(self, use_content_length=False,
+                      use_transfer_encoding=False, string_to_md5=''):
+        content_md5 = md5(string_to_md5).digest().encode('base64').strip()
+        with UnreadableInput(self) as fake_input:
+            req = Request.blank(
+                '/bucket?delete',
+                environ={
+                    'REQUEST_METHOD': 'POST',
+                    'wsgi.input': fake_input},
+                headers={
+                    'Authorization': 'AWS test:tester:hmac',
+                    'Date': self.get_date_header(),
+                    'Content-MD5': content_md5},
+                body='')
+            if not use_content_length:
+                req.environ.pop('CONTENT_LENGTH')
+            if use_transfer_encoding:
+                req.environ['HTTP_TRANSFER_ENCODING'] = 'chunked'
+            status, headers, body = self.call_swift3(req)
+        self.assertEqual(status, '400 Bad Request')
+        self.assertEqual(self._get_error_code(body), 'MissingRequestBodyError')
+
+    @s3acl
+    def test_object_multi_DELETE_empty_body(self):
+        self._test_no_body()
+        self._test_no_body(string_to_md5='test')
+        self._test_no_body(use_content_length=True)
+        self._test_no_body(use_content_length=True, string_to_md5='test')
+        self._test_no_body(use_transfer_encoding=True)
+        self._test_no_body(use_transfer_encoding=True, string_to_md5='test')
+
 if __name__ == '__main__':
     unittest.main()

swift3/test/unit/test_multi_upload.py

@@ -14,10 +14,11 @@
 # limitations under the License.
 
 import base64
+from hashlib import md5
+from mock import patch
 import os
 import time
 import unittest
-from mock import patch
 from urllib import quote
 
 from swift.common import swob
@@ -25,6 +26,7 @@ from swift.common.swob import Request
 from swift.common.utils import json
 
 from swift3.test.unit import Swift3TestCase
+from swift3.test.unit.helpers import UnreadableInput
 from swift3.etree import fromstring, tostring
 from swift3.subresource import Owner, Grant, User, ACL, encode_acl, \
     decode_acl, ACLPublicRead
@@ -1670,6 +1672,39 @@ class TestSwift3MultiUpload(Swift3TestCase):
         self.assertEqual('bytes=0-9', put_headers['Range'])
         self.assertEqual('/src_bucket/src_obj', put_headers['X-Copy-From'])
 
+    def _test_no_body(self, use_content_length=False,
+                      use_transfer_encoding=False, string_to_md5=''):
+        content_md5 = md5(string_to_md5).digest().encode('base64').strip()
+        with UnreadableInput(self) as fake_input:
+            req = Request.blank(
+                '/bucket/object?uploadId=X',
+                environ={
+                    'REQUEST_METHOD': 'POST',
+                    'wsgi.input': fake_input},
+                headers={
+                    'Authorization': 'AWS test:tester:hmac',
+                    'Date': self.get_date_header(),
+                    'Content-MD5': content_md5},
+                body='')
+            if not use_content_length:
+                req.environ.pop('CONTENT_LENGTH')
+            if use_transfer_encoding:
+                req.environ['HTTP_TRANSFER_ENCODING'] = 'chunked'
+            status, headers, body = self.call_swift3(req)
+        self.assertEqual(status, '400 Bad Request')
+        self.assertEqual(self._get_error_code(body), 'InvalidRequest')
+        self.assertEqual(self._get_error_message(body),
+                         'You must specify at least one part')
+
+    @s3acl
+    def test_object_multi_upload_empty_body(self):
+        self._test_no_body()
+        self._test_no_body(string_to_md5='test')
+        self._test_no_body(use_content_length=True)
+        self._test_no_body(use_content_length=True, string_to_md5='test')
+        self._test_no_body(use_transfer_encoding=True)
+        self._test_no_body(use_transfer_encoding=True, string_to_md5='test')
+
 
 class TestSwift3MultiUploadNonUTC(TestSwift3MultiUpload):
     def setUp(self):