Repository: boto/botocore
Pull request #478 (2015-03-04 01:28:24)

Show XML body when unable to parse XML
Right now you'll get a vague message about invalid
syntax if we can't parse the XML for whatever reason.

Now, we catch this error and raise a specific exception that
also includes the XML body so a user can see why we couldn't parse
the exception.

BTW, this happens when:

* You provide an invalid endpoint-url.  Sometimes you might get some
  boiler plate HTML from a site which will trigger this exception.
* There are some cases are XML parser can't handle when we get
  invalid XML.  While the end goal is to handle these cases, we
  should at least be providing a better error message when this
  isn't the case.

cc @kyleknap @danielgtaylor 


Patch:
diff --git a/botocore/parsers.py b/botocore/parsers.py
index 01c9c6c9c..99eb4e340 100644
--- a/botocore/parsers.py
+++ b/botocore/parsers.py
@@ -97,8 +97,7 @@ import xml.etree.cElementTree
 import logging
 from pprint import pformat
 
-from botocore.compat import six
-from six.moves import http_client
+from botocore.compat import six, XMLParseError
 
 from botocore.utils import parse_timestamp
 
@@ -331,11 +330,16 @@ class BaseXMLResponseParser(ResponseParser):
         return xml_dict
 
     def _parse_xml_string_to_dom(self, xml_string):
-        parser = xml.etree.cElementTree.XMLParser(
-            target=xml.etree.cElementTree.TreeBuilder(),
-            encoding=self.DEFAULT_ENCODING)
-        parser.feed(xml_string)
-        root = parser.close()
+        try:
+            parser = xml.etree.cElementTree.XMLParser(
+                target=xml.etree.cElementTree.TreeBuilder(),
+                encoding=self.DEFAULT_ENCODING)
+            parser.feed(xml_string)
+            root = parser.close()
+        except XMLParseError as e:
+            raise ResponseParserError(
+                "Unable to parse response (%s), "
+                "invalid XML received:\n%s" % (e, xml_string))
         return root
 
     def _replace_nodes(self, parsed):
@@ -679,7 +683,7 @@ class RestXMLParser(BaseRestParser, BaseXMLResponseParser):
         return {
             'Error': {
                 'Code': str(response['status_code']),
-                'Message': http_client.responses.get(
+                'Message': six.moves.http_client.responses.get(
                     response['status_code'], ''),
             },
             'ResponseMetadata': {


Test patch:
diff --git a/tests/unit/test_parsers.py b/tests/unit/test_parsers.py
index c51a07fc7..0901b195e 100644
--- a/tests/unit/test_parsers.py
+++ b/tests/unit/test_parsers.py
@@ -27,10 +27,10 @@ class TestResponseMetadataParsed(unittest.TestCase):
         parser = parsers.QueryParser()
         response = (
             '<OperationNameResponse>'
-              '<OperationNameResult><Str>myname</Str></OperationNameResult>'
-              '<ResponseMetadata>'
-                '<RequestId>request-id</RequestId>'
-              '</ResponseMetadata>'
+            '  <OperationNameResult><Str>myname</Str></OperationNameResult>'
+            '  <ResponseMetadata>'
+            '    <RequestId>request-id</RequestId>'
+            '  </ResponseMetadata>'
             '</OperationNameResponse>').encode('utf-8')
         output_shape = model.StructureShape(
             'OutputShape',
@@ -68,8 +68,8 @@ class TestResponseMetadataParsed(unittest.TestCase):
         parser = parsers.EC2QueryParser()
         response = (
             '<OperationNameResponse>'
-              '<Str>myname</Str>'
-              '<requestId>request-id</requestId>'
+            '  <Str>myname</Str>'
+            '  <requestId>request-id</requestId>'
             '</OperationNameResponse>').encode('utf-8')
         output_shape = model.StructureShape(
             'OutputShape',
@@ -205,11 +205,11 @@ class TestResponseMetadataParsed(unittest.TestCase):
     def test_s3_error_response(self):
         body = (
             '<Error>'
-              '<Code>NoSuchBucket</Code>'
-              '<Message>error message</Message>'
-              '<BucketName>asdf</BucketName>'
-              '<RequestId>EF1EF43A74415102</RequestId>'
-              '<HostId>hostid</HostId>'
+            '  <Code>NoSuchBucket</Code>'
+            '  <Message>error message</Message>'
+            '  <BucketName>asdf</BucketName>'
+            '  <RequestId>EF1EF43A74415102</RequestId>'
+            '  <HostId>hostid</HostId>'
             '</Error>'
         ).encode('utf-8')
         headers = {
@@ -260,13 +260,13 @@ class TestResponseMetadataParsed(unittest.TestCase):
     def test_can_parse_sdb_error_response(self):
         body = (
             '<OperationNameResponse>'
-                '<Errors>'
-                    '<Error>'
-                        '<Code>1</Code>'
-                        '<Message>msg</Message>'
-                    '</Error>'
-                '</Errors>'
-                '<RequestId>abc-123</RequestId>'
+            '    <Errors>'
+            '        <Error>'
+            '            <Code>1</Code>'
+            '            <Message>msg</Message>'
+            '        </Error>'
+            '    </Errors>'
+            '    <RequestId>abc-123</RequestId>'
             '</OperationNameResponse>'
         ).encode('utf-8')
         parser = parsers.QueryParser()
@@ -438,3 +438,20 @@ class TestHandlesNoOutputShape(unittest.TestCase):
             parsed,
             {'ResponseMetadata': {'RequestId': 'request-id',
                                   'HTTPStatusCode': 200}})
+
+
+class TestHandlesInvalidXMLResponses(unittest.TestCase):
+    def test_invalid_xml_shown_in_error_message(self):
+        # Missing the closing XML tags.
+        invalid_xml = (
+            b'<DeleteTagsResponse xmlns="http://autoscaling.amazonaws.com/">'
+            b'  <ResponseMetadata>'
+        )
+        parser = parsers.QueryParser()
+        output_shape = None
+        # The XML body should be in the error message.
+        with self.assertRaisesRegexp(parsers.ResponseParserError,
+                                     '<DeleteTagsResponse'):
+            parser.parse(
+                {'body': invalid_xml, 'headers': {}, 'status_code': 200},
+                output_shape)