Repository: stripe/stripe-python
Issue: Problem with uploading binary files with Python 3

With Python 3, uploading a binary file will fail:

``` python
with open('/path/to/a/file.jpg', 'r') as fp:
    file_upload = stripe.FileUpload.create(
        purpose='dispute_evidence',
        file=fp
    )
```

fails with:

```
Traceback (most recent call last):
  <snip>
  File "/Users/ob/.pyenv/versions/3.5.1/lib/python3.5/site-packages/stripe/multipart_data_generator.py", line 28, in add_params
    self._write_file(value)
  File "/Users/ob/.pyenv/versions/3.5.1/lib/python3.5/site-packages/stripe/multipart_data_generator.py", line 54, in _write_file
    file_contents = f.read(self.chunk_size)
  File "/Users/ob/.pyenv/versions/3.5.1/lib/python3.5/codecs.py", line 321, in decode
    (result, consumed) = self._buffer_decode(data, self.errors, final)
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x89 in position 0: invalid start byte
```

Opening the file as binary (e.g. replacing `'r'` by `'rb'` yields a different issue:

```
Traceback (most recent call last):
  <snip>
  File "/Users/ob/.pyenv/versions/3.5.1/lib/python3.5/site-packages/stripe/multipart_data_generator.py", line 28, in add_params
    self._write_file(value)
  File "/Users/ob/.pyenv/versions/3.5.1/lib/python3.5/site-packages/stripe/multipart_data_generator.py", line 57, in _write_file
    self._write(file_contents)
  File "/Users/ob/.pyenv/versions/3.5.1/lib/python3.5/site-packages/stripe/multipart_data_generator.py", line 50, in _write
    self.data.write(bytes(value, 'utf-8'))
TypeError: encoding or errors without a string argument
```

I think we ought to:
1. Update the Python file upload examples to use `mode='rb'` rather than `mode='r'` (since image files are binary files and it makes no sense to apply an encoding to them).
2. Patch [this method](https://github.com/stripe/stripe-python/blob/master/stripe/multipart_data_generator.py#L46-L50) that assumes that Python 3 will always deal with Unicode strings. Something like this should work:

``` python
def _write(self, value):
  if sys.version_info < (3, 0) or isinstance(value, bytes):
    self.data.write(value)
  else:
    self.data.write(bytes(value, 'utf-8'))
```

@brandur, any thoughts? Should I proceed with the above?


Changed files:
stripe/test/test_multipart_data_generator.py, stripe/multipart_data_generator.py

diff --git a/stripe/multipart_data_generator.py b/stripe/multipart_data_generator.py
index d23367db..dd6c7fda 100644
--- a/stripe/multipart_data_generator.py
+++ b/stripe/multipart_data_generator.py
@@ -18,8 +18,11 @@ class MultipartDataGenerator(object):
             self._write(self.param_header())
             self._write(self.line_break)
             if hasattr(value, 'read'):
-                self._write("Content-Disposition: form-data; name=\\"%s\\"; "
-                            "filename=\\"%s\\"" % (key, value.name))
+                self._write("Content-Disposition: form-data; name=\\"")
+                self._write(key)
+                self._write("\\"; filename=\\"")
+                self._write(value.name)
+                self._write("\\"")
                 self._write(self.line_break)
                 self._write("Content-Type: application/octet-stream")
                 self._write(self.line_break)
@@ -27,8 +30,9 @@ class MultipartDataGenerator(object):
 
                 self._write_file(value)
             else:
-                self._write("Content-Disposition: form-data; name=\\"%s\\"" %
-                            (key,))
+                self._write("Content-Disposition: form-data; name=\\"")
+                self._write(key)
+                self._write("\\"")
                 self._write(self.line_break)
                 self._write(self.line_break)
                 self._write(value)
@@ -44,10 +48,22 @@ class MultipartDataGenerator(object):
         return self.data.getvalue()
 
     def _write(self, value):
-        if sys.version_info < (3, 0):
-            self.data.write(value)
+        if sys.version_info < (3,):
+            binary_type = str
+            text_type = unicode
         else:
-            self.data.write(bytes(value, 'utf-8'))
+            binary_type = bytes
+            text_type = str
+
+        if isinstance(value, binary_type):
+            array = bytearray(value)
+        elif isinstance(value, text_type):
+            array = bytearray(value, encoding='utf-8')
+        else:
+            raise TypeError("unexpected type: {value_type}"
+                            .format(value_type=type(value)))
+
+        self.data.write(array)
 
     def _write_file(self, f):
         while True:
diff --git a/stripe/test/test_multipart_data_generator.py b/stripe/test/test_multipart_data_generator.py
index 3f18e61b..f6423d94 100644
--- a/stripe/test/test_multipart_data_generator.py
+++ b/stripe/test/test_multipart_data_generator.py
@@ -1,3 +1,5 @@
+# -*- encoding: utf-8 -*-
+
 import re
 import sys
 
@@ -7,34 +9,44 @@ from stripe.test.helper import StripeTestCase
 
 class MultipartDataGeneratorTests(StripeTestCase):
 
-    def test_generate_simple_multipart_data(self):
-        with open(__file__) as test_file:
-            params = {
-                "key1": "value1",
-                "key2": "value2",
-                "key3": test_file
-            }
-            generator = MultipartDataGenerator()
-            generator.add_params(params)
-            post_data = generator.get_post_data()
-
-            if sys.version_info < (3, 0):
-                http_body = "".join([line for line in post_data])
-            else:
-                byte_array = bytearray([i for i in post_data])
-                http_body = byte_array.decode('utf-8')
-
-            self.assertTrue(re.search(
-                r"Content-Disposition: form-data; name=\\"key1\\"", http_body))
-            self.assertTrue(re.search(
-                r"Content-Disposition: form-data; name=\\"key2\\"", http_body))
-            self.assertTrue(re.search(
-                r"Content-Disposition: form-data; name=\\"key3\\"; "
-                r"filename=\\".+\\"",
-                http_body))
-            self.assertTrue(re.search(
-                r"Content-Type: application/octet-stream", http_body))
-
-            test_file.seek(0)
-            file_contents = test_file.read()
-            self.assertNotEqual(-1, http_body.find(file_contents))
+    def run_test_multipart_data_with_file(self, test_file):
+        params = {
+            "key1": b"ASCII value",
+            "key2": u"Üñìçôdé value",
+            "key3": test_file
+        }
+        generator = MultipartDataGenerator()
+        generator.add_params(params)
+        http_body = generator.get_post_data()
+
+        if sys.version_info >= (3,):
+            http_body = http_body.decode('utf-8')
+
+        self.assertTrue(re.search(
+            r"Content-Disposition: form-data; name=\\"key1\\"", http_body))
+        self.assertTrue(re.search(r"ASCII value", http_body))
+        self.assertTrue(re.search(
+            r"Content-Disposition: form-data; name=\\"key2\\"", http_body))
+        self.assertTrue(re.search(r"Üñìçôdé value", http_body))
+        self.assertTrue(re.search(
+            r"Content-Disposition: form-data; name=\\"key3\\"; "
+            r"filename=\\".+\\"",
+            http_body))
+        self.assertTrue(re.search(
+            r"Content-Type: application/octet-stream", http_body))
+
+        test_file.seek(0)
+        file_contents = test_file.read()
+
+        if sys.version_info >= (3,) and isinstance(file_contents, bytes):
+            file_contents = file_contents.decode('utf-8')
+
+        self.assertNotEqual(-1, http_body.find(file_contents))
+
+    def test_multipart_data_file_text(self):
+        with open(__file__, mode='r') as test_file:
+            self.run_test_multipart_data_with_file(test_file)
+
+    def test_multipart_data_file_binary(self):
+        with open(__file__, mode='rb') as test_file:
+            self.run_test_multipart_data_with_file(test_file)