Repository: boto/botocore
Pull request #953 (2016-06-16 22:43:12)

Replace chars we can't decode with placeholder char
It's possible that the console output from EC2 contains
non string chars we can't utf-8 decode.  When that happens
we were previously just leaving the input untouched, i.e
base64 encoded.

This made for an awkward interface for customers.  We couldn't guarantee
whether or not the content was base64 decoded because it would depend
on the actual ec2 console output content.

This change attempts to make things more consistent by
replacing any chars we can't decode with the unicode
`\ufffd` char (the unicode replace char, i.e the question mark).

The one downside with this approach is that it is possible
to sometimes not get the unaltered contents of the console
output because chars have been replaced with `\ufffd`.
We'll need to weigh whether or not we think that's worth
the benefit.  Given this content is usually text content,
this seems like a reasonable compromise.

cc @kyleknap @JordonPhillips 


Patch:
diff --git a/.changes/next-release/bugfix-EC2-81604.json b/.changes/next-release/bugfix-EC2-81604.json
new file mode 100644
index 000000000..2a0c72115
--- /dev/null
+++ b/.changes/next-release/bugfix-EC2-81604.json
@@ -0,0 +1,5 @@
+{
+  "category": "EC2", 
+  "type": "bugfix", 
+  "description": "Replace chars in the EC2 console output we can't decode with replacement chars.  We were previously returning either the decoded content or the original base64 encoded content.  We now will consistently return decoded output, any any chars we can't decode are substituted with a replacement char. (`#953 <https://github.com/boto/botocore/issues/953>`__)"
+}
diff --git a/botocore/handlers.py b/botocore/handlers.py
index 03b5fa818..ba3e48d64 100644
--- a/botocore/handlers.py
+++ b/botocore/handlers.py
@@ -99,7 +99,11 @@ def _looks_like_special_case_error(http_response):
 def decode_console_output(parsed, **kwargs):
     if 'Output' in parsed:
         try:
-            value = base64.b64decode(six.b(parsed['Output'])).decode('utf-8')
+            # We're using 'replace' for errors because it is
+            # possible that console output contains non string
+            # chars we can't utf-8 decode.
+            value = base64.b64decode(six.b(parsed['Output'])).decode(
+                'utf-8', 'replace')
             parsed['Output'] = value
         except (ValueError, TypeError, AttributeError):
             logger.debug('Error decoding base64', exc_info=True)


Test patch:
diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py
index 471434cde..ab9774c70 100644
--- a/tests/unit/test_handlers.py
+++ b/tests/unit/test_handlers.py
@@ -47,6 +47,12 @@ class TestHandlers(BaseSessionTest):
         handlers.decode_console_output(parsed)
         self.assertEqual(parsed['Output'], 1)
 
+    def test_get_console_output_bad_unicode_errors(self):
+        original = base64.b64encode(b'before\xffafter').decode('utf-8')
+        parsed = {'Output': original}
+        handlers.decode_console_output(parsed)
+        self.assertEqual(parsed['Output'], u'before\ufffdafter')
+
     def test_noop_if_output_key_does_not_exist(self):
         original = {'foo': 'bar'}
         parsed = original.copy()