twitter: preview/create: detect images over 5MB and return an error

snarfed · Oct 23, 2019 · 05a30d4 · 05a30d4
1 parent fa5ab60
commit 05a30d4
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 40 deletions.
diff --git a/README.md b/README.md
@@ -313,6 +313,8 @@ Changelog
 * Add Python 3.7 support, and improve overall Python 3 compatibility.
 * Update a number of dependencies.
 * Switch from Python's built in `json` module to [`ujson`](https://github.com/esnme/ultrajson/) to speed up JSON parsing and encoding.
+* Twitter:
+  * `[preview]_create()`: detect attempts to upload [images over 5MB](https://developer.twitter.com/en/docs/media/upload-media/uploading-media/media-best-practices#image-specs) and return an error.
 * Facebook:
   * Add `get_activities(scrape=True)` for scraping HTML from [m.facebook.com](https://m.facebook.com/). Requires `c_user` and `xs` cookies from a logged in session. ([snarfed/bridgy#886](https://github.com/snarfed/bridgy/issues/886)
   * [Upgrade Graph API version from 2.10 to 4.0.](https://developers.facebook.com/docs/graph-api/changelog)

diff --git a/granary/tests/test_twitter.py b/granary/tests/test_twitter.py
@@ -2285,7 +2285,7 @@ def test_create_with_multiple_photos(self):
     # test create
     for i, url in enumerate(image_urls[:-1]):
       content = 'picture response %d' % i
-      self.expect_urlopen(url, content)
+      self.expect_urlopen(url, content, response_headers={'Content-Length': 3})
       self.expect_requests_post(twitter.API_UPLOAD_MEDIA,
                                 json_dumps({'media_id_string': str(i)}),
                                 files={'media': content},
@@ -2315,7 +2315,8 @@ def test_create_reply_with_photo(self):
                       preview.content)
 
     # test create
-    self.expect_urlopen('http://my/picture', 'picture response')
+    self.expect_urlopen('http://my/picture', 'picture response',
+                        response_headers={'Content-Length': 3})
     self.expect_requests_post(twitter.API_UPLOAD_MEDIA,
                               json_dumps({'media_id_string': '123'}),
                               files={'media': 'picture response'},
@@ -2345,7 +2346,8 @@ def test_create_with_photo_no_content(self):
                      preview.content)
 
     # test create
-    self.expect_urlopen('http://my/picture', 'picture response')
+    self.expect_urlopen('http://my/picture', 'picture response',
+                        response_headers={'Content-Length': 3})
     self.expect_requests_post(twitter.API_UPLOAD_MEDIA,
                               json_dumps({'media_id_string': '123'}),
                               files={'media': 'picture response'},
@@ -2367,7 +2369,8 @@ def test_create_with_photo_error(self):
       'image': {'url': 'http://my/picture'},
     }
 
-    self.expect_urlopen('http://my/picture', 'picture response')
+    self.expect_urlopen('http://my/picture', 'picture response',
+                        response_headers={'Content-Length': 3})
     self.expect_requests_post(twitter.API_UPLOAD_MEDIA,
                               json_dumps({'media_id_string': '123'}),
                               files={'media': 'picture response'},
@@ -2382,7 +2385,8 @@ def test_create_with_photo_error(self):
     self.assertRaises(urllib_error.HTTPError, self.twitter.create, obj)
 
   def test_create_with_photo_upload_error(self):
-    self.expect_urlopen('http://my/picture', 'picture response')
+    self.expect_urlopen('http://my/picture', 'picture response',
+                        response_headers={'Content-Length': 3})
     self.expect_requests_post(twitter.API_UPLOAD_MEDIA,
                               files={'media': 'picture response'},
                               headers=mox.IgnoreArg(),
@@ -2398,14 +2402,15 @@ def test_create_with_photo_wrong_type(self):
       'objectType': 'note',
       'image': {'url': 'http://my/picture.tiff'},
     }
-    self.expect_urlopen('http://my/picture.tiff', '')
+    self.expect_urlopen('http://my/picture.tiff', '',
+                        response_headers={'Content-Length': 3})
     self.mox.ReplayAll()
 
     ret = self.twitter.create(obj)
     self.assertTrue(ret.abort)
     for msg in ret.error_plain, ret.error_html:
-      self.assertEqual('Twitter only supports JPG, PNG, GIF, and WEBP images; '
-                        'http://my/picture.tiff looks like image/tiff', msg)
+      self.assertIn('Twitter only supports JPG, PNG, GIF, and WEBP images;', msg)
+      self.assertIn('looks like image/tiff', msg)
 
   def test_create_with_photo_with_alt(self):
     obj = {
@@ -2423,7 +2428,8 @@ def test_create_with_photo_with_alt(self):
                      preview.content)
 
     # test create
-    self.expect_urlopen('http://my/picture.png', 'picture response')
+    self.expect_urlopen('http://my/picture.png', 'picture response',
+                        response_headers={'Content-Length': 3})
     self.expect_requests_post(twitter.API_UPLOAD_MEDIA,
                               json_dumps({'media_id_string': '123'}),
                               files={'media': 'picture response'},
@@ -2442,8 +2448,23 @@ def test_create_with_photo_with_alt(self):
     self.assert_equals({'url': 'http://posted/picture', 'type': 'post'},
                        self.twitter.create(obj).content)
 
+  def test_create_with_photo_too_big(self):
+    self.expect_urlopen(
+      'http://my/picture.png', '',
+      response_headers={'Content-Length': twitter.MAX_IMAGE_SIZE + 1})
+    self.mox.ReplayAll()
+
+    # test create
+    got = self.twitter.create({
+      'objectType': 'note',
+      'image': {'url': 'http://my/picture.png'},
+    })
+    self.assertTrue(got.abort)
+    self.assertIn("larger than Twitter's 5MB limit:", got.error_plain)
+
   def test_create_with_photo_with_alt_error(self):
-    self.expect_urlopen('http://my/picture.png', 'picture response')
+    self.expect_urlopen('http://my/picture.png', 'picture response',
+                        response_headers={'Content-Length': 3})
     self.expect_requests_post(twitter.API_UPLOAD_MEDIA,
                               json_dumps({'media_id_string': '123'}),
                               files={'media': 'picture response'},
@@ -2528,8 +2549,8 @@ def test_create_with_video_too_big(self):
       'stream': {'url': 'http://my/video'},
     })
     self.assertTrue(ret.abort)
-    self.assertIn("larger than Twitter's 512MB limit.", ret.error_plain)
-    self.assertIn("larger than Twitter's 512MB limit.", ret.error_html)
+    self.assertIn("larger than Twitter's 512MB limit:", ret.error_plain)
+    self.assertIn("larger than Twitter's 512MB limit:", ret.error_html)
 
   def test_create_with_video_wrong_type(self):
     self.expect_urlopen('http://my/video', '',

diff --git a/granary/twitter.py b/granary/twitter.py
@@ -97,6 +97,7 @@
 VIDEO_MIME_TYPES = frozenset(('video/mp4',))
 MB = 1024 * 1024
 # https://developer.twitter.com/en/docs/media/upload-media/uploading-media/media-best-practices
+MAX_IMAGE_SIZE = 5 * MB
 MAX_VIDEO_SIZE = 512 * MB
 UPLOAD_CHUNK_SIZE = 5 * MB
 MAX_ALT_LENGTH = 420
@@ -899,10 +900,10 @@ def upload_images(self, images):
         continue
 
       image_resp = util.urlopen(url)
-      bad_type = self._check_mime_type(url, image_resp, IMAGE_MIME_TYPES,
-                                       'JPG, PNG, GIF, and WEBP images')
-      if bad_type:
-        return bad_type
+      error = self._check_media(url, image_resp, IMAGE_MIME_TYPES,
+                                'JPG, PNG, GIF, and WEBP images', MAX_IMAGE_SIZE)
+      if error:
+        return error
 
       headers = twitter_auth.auth_header(
         API_UPLOAD_MEDIA, self.access_token_key, self.access_token_secret, 'POST')
@@ -946,26 +947,17 @@ def upload_video(self, url):
       string media id or :class:`CreationResult` on error
     """
     video_resp = util.urlopen(url)
-    bad_type = self._check_mime_type(url, video_resp, VIDEO_MIME_TYPES, 'MP4 videos')
-    if bad_type:
-      return bad_type
-
-    length = video_resp.headers.get('Content-Length')
-    if not util.is_int(length):
-      msg = "Couldn't determine your video's size."
-      return source.creation_result(abort=True, error_plain=msg, error_html=msg)
-
-    length = int(length)
-    if int(length) > MAX_VIDEO_SIZE:
-      msg = "Your %sMB video is larger than Twitter's %dMB limit." % (
-        length // MB, MAX_VIDEO_SIZE // MB)
-      return source.creation_result(abort=True, error_plain=msg, error_html=msg)
+    error = self._check_media(url, video_resp, VIDEO_MIME_TYPES, 'MP4 videos',
+                              MAX_VIDEO_SIZE)
+    if error:
+      return error
 
     # INIT
     media_id = self.urlopen(API_UPLOAD_MEDIA, data=urllib.parse.urlencode({
       'command': 'INIT',
       'media_type': 'video/mp4',
-      'total_bytes': length,
+      # _check_media checked that Content-Length is set
+      'total_bytes': video_resp.headers['Content-Length'],
     }))['media_id_string']
 
     # APPEND
@@ -997,27 +989,41 @@ def upload_video(self, url):
     return media_id
 
   @staticmethod
-  def _check_mime_type(url, resp, allowed, label):
-    """Checks that a URL is in a set of allowed MIME type(s).
+  def _check_media(url, resp, types, label, max_size):
+    """Checks that an image or video is an allowed type and size.
 
     Args:
       url: string
       resp: urlopen result object
-      allowed: sequence of allowed string MIME types
-      label: human-readable description of the allowed MIME types, to be used in
-        an error message
+      types: sequence of allowed string MIME types
+      label: string, human-readable description of the allowed MIME types, to be
+        used in an error message
+      max_size: integer, maximum allowed size, in bytes
 
     Returns:
-      None if the url's MIME type is in the set, :class:`CreationResult`
-      with abort=True if it isn't
+      None if the url's type and size are valid, :class:`CreationResult`
+      with abort=True otherwise
     """
     type = resp.headers.get('Content-Type')
     if not type:
       type, _ = mimetypes.guess_type(url)
-    if type and type not in allowed:
-      msg = 'Twitter only supports %s; %s looks like %s' % (label, url, type)
+    if type and type not in types:
+      msg = 'Twitter only supports %s; %s looks like %s' % (
+        label, util.pretty_link(url), type)
       return source.creation_result(abort=True, error_plain=msg, error_html=msg)
 
+    length = resp.headers.get('Content-Length')
+    if not util.is_int(length):
+      msg = "Couldn't determine the size of %s" % util.pretty_link(url)
+      return source.creation_result(abort=True, error_plain=msg, error_html=msg)
+
+    length = int(length)
+    if int(length) > max_size:
+      msg = "Your %.2fMB file is larger than Twitter's %dMB limit: %s" % (
+        length // MB, max_size // MB, util.pretty_link(url))
+      return source.creation_result(abort=True, error_plain=msg, error_html=msg)
+
+
   def delete(self, id):
     """Deletes a tweet. The authenticated user must have authored it.