From 6bb8f355a2467ef477ac37dfdf01c8ecef12e367 Mon Sep 17 00:00:00 2001
From: James Saryerwinnie <js@jamesls.com>
Date: Mon, 6 Oct 2014 17:00:49 -0700
Subject: [PATCH] Fix regression when handling 200 error responses from S3

We now do not try to parse the error response unless it's an error
HTTP status code.  This means the handler needs to be updated such
that _it_ is the one that does the Error parsing.  Fortunately, we
don't need to fully parse the XML error response.  We just need to
know that the top level key is an Error node before saying we
need to retry the request.
---
 botocore/handlers.py        | 23 +++++++++++++++++------
 tests/unit/test_handlers.py | 28 ++++++++++++----------------
 2 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/botocore/handlers.py b/botocore/handlers.py
index 2bcfc9c00f..9668695963 100644
--- a/botocore/handlers.py
+++ b/botocore/handlers.py
@@ -39,7 +39,7 @@
 
 
 
-def check_for_200_error(response, operation, **kwargs):
+def check_for_200_error(response, **kwargs):
     # From: http://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectCOPY.html
     # There are two opportunities for a copy request to return an error. One
     # can occur when Amazon S3 receives the copy request and the other can
@@ -60,12 +60,23 @@ def check_for_200_error(response, operation, **kwargs):
         # trying to retrieve the response.  See Endpoint._get_response().
         return
     http_response, parsed = response
+    if _looks_like_special_case_error(http_response):
+        logger.debug("Error found for response with 200 status code, "
+                        "errors: %s, changing status code to "
+                        "500.", parsed)
+        http_response.status_code = 500
+
+
+def _looks_like_special_case_error(http_response):
     if http_response.status_code == 200:
-        if 'Errors' in parsed:
-            logger.debug("Error found for response with 200 status code, "
-                         "operation: %s, errors: %s, changing status code to "
-                         "500.", operation, parsed)
-            http_response.status_code = 500
+        parser = xml.etree.cElementTree.XMLParser(
+            target=xml.etree.cElementTree.TreeBuilder(),
+            encoding='utf-8')
+        parser.feed(http_response.content)
+        root = parser.close()
+        if root.tag == 'Error':
+            return True
+    return False
 
 
 def decode_console_output(parsed, **kwargs):
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
index a1256ae5d3..a32fae41cc 100644
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -119,33 +119,29 @@ def test_destination_region_left_untouched(self):
     def test_500_status_code_set_for_200_response(self):
         http_response = mock.Mock()
         http_response.status_code = 200
-        parsed = {
-            'Errors': [{
-                "HostId": "hostid",
-                "Message": "An internal error occurred.",
-                "Code": "InternalError",
-                "RequestId": "123456789"
-            }]
-        }
-        handlers.check_for_200_error((http_response, parsed), 'MyOperationName')
+        http_response.content = """
+            <Error>
+              <Code>AccessDenied</Code>
+              <Message>Access Denied</Message>
+              <RequestId>id</RequestId>
+              <HostId>hostid</HostId>
+            </Error>
+        """
+        handlers.check_for_200_error((http_response, {}))
         self.assertEqual(http_response.status_code, 500)
 
     def test_200_response_with_no_error_left_untouched(self):
         http_response = mock.Mock()
         http_response.status_code = 200
-        parsed = {
-            'NotAnError': [{
-                'foo': 'bar'
-            }]
-        }
-        handlers.check_for_200_error((http_response, parsed), 'MyOperationName')
+        http_response.content = "<NotAnError></NotAnError>"
+        handlers.check_for_200_error((http_response, {}))
         # We don't touch the status code since there are no errors present.
         self.assertEqual(http_response.status_code, 200)
 
     def test_500_response_can_be_none(self):
         # A 500 response can raise an exception, which means the response
         # object is None.  We need to handle this case.
-        handlers.check_for_200_error(None, mock.Mock())
+        handlers.check_for_200_error(None)
 
     def test_sse_headers(self):
         prefix = 'x-amz-server-side-encryption-customer-'