gh-91520: Rewrite imghdr inlining for clarity and completeness (#91521)

* Rewrite imghdr inlining for clarity and completeness

* Move MIMEImage class back closer to the top of the file since it's the
  important thing.
* Use a decorate to mark a given rule function and simplify the rule function
  names for clarity.
* Copy over all the imghdr test data files into the email package's test data
  directory.  This way when imghdr is actually removed, it won't affect the
  MIMEImage guessing tests.
* Rewrite and extend the MIMEImage tests to test for all supported
  auto-detected MIME image subtypes.
* Remove the now redundant PyBanner048.gif data file.

* See https://github.com/python/cpython/pull/91461#discussion_r850313336

Co-authored-by: Oleg Iarygin <dralife@yandex.ru>

Co-authored-by: Oleg Iarygin <dralife@yandex.ru>

Doc/includes/email-mime.py

@@ -1,7 +1,7 @@
-# Import smtplib for the actual sending function
+# Import smtplib for the actual sending function.
 import smtplib
 
-# Here are the email package modules we'll need
+# Here are the email package modules we'll need.
 from email.message import EmailMessage
 
 # Create the container email message.
@@ -13,13 +13,13 @@ msg['From'] = me
 msg['To'] = ', '.join(family)
 msg.preamble = 'You will not see this in a MIME-aware mail reader.\n'
 
-# Open the files in binary mode.  Use imghdr to figure out the
-# MIME subtype for each specific image.
+# Open the files in binary mode.  You can also omit the subtype
+# if you want MIMEImage to guess it.
 for file in pngfiles:
     with open(file, 'rb') as fp:
         img_data = fp.read()
     msg.add_attachment(img_data, maintype='image',
-                                 subtype='jpeg')
+                                 subtype='png')
 
 # Send the email via our own SMTP server.
 with smtplib.SMTP('localhost') as s:

Lib/email/mime/image.py

@@ -10,109 +10,6 @@ from email import encoders
 from email.mime.nonmultipart import MIMENonMultipart
 
 
-# Originally from the imghdr module.
-def _what(h):
-    for tf in tests:
-        if res := tf(h):
-            return res
-    else:
-        return None
-
-tests = []
-
-def _test_jpeg(h):
-    """JPEG data with JFIF or Exif markers; and raw JPEG"""
-    if h[6:10] in (b'JFIF', b'Exif'):
-        return 'jpeg'
-    elif h[:4] == b'\xff\xd8\xff\xdb':
-        return 'jpeg'
-
-tests.append(_test_jpeg)
-
-def _test_png(h):
-    if h.startswith(b'\211PNG\r\n\032\n'):
-        return 'png'
-
-tests.append(_test_png)
-
-def _test_gif(h):
-    """GIF ('87 and '89 variants)"""
-    if h[:6] in (b'GIF87a', b'GIF89a'):
-        return 'gif'
-
-tests.append(_test_gif)
-
-def _test_tiff(h):
-    """TIFF (can be in Motorola or Intel byte order)"""
-    if h[:2] in (b'MM', b'II'):
-        return 'tiff'
-
-tests.append(_test_tiff)
-
-def _test_rgb(h):
-    """SGI image library"""
-    if h.startswith(b'\001\332'):
-        return 'rgb'
-
-tests.append(_test_rgb)
-
-def _test_pbm(h):
-    """PBM (portable bitmap)"""
-    if len(h) >= 3 and \
-        h[0] == ord(b'P') and h[1] in b'14' and h[2] in b' \t\n\r':
-        return 'pbm'
-
-tests.append(_test_pbm)
-
-def _test_pgm(h):
-    """PGM (portable graymap)"""
-    if len(h) >= 3 and \
-        h[0] == ord(b'P') and h[1] in b'25' and h[2] in b' \t\n\r':
-        return 'pgm'
-
-tests.append(_test_pgm)
-
-def _test_ppm(h):
-    """PPM (portable pixmap)"""
-    if len(h) >= 3 and \
-        h[0] == ord(b'P') and h[1] in b'36' and h[2] in b' \t\n\r':
-        return 'ppm'
-
-tests.append(_test_ppm)
-
-def _test_rast(h):
-    """Sun raster file"""
-    if h.startswith(b'\x59\xA6\x6A\x95'):
-        return 'rast'
-
-tests.append(_test_rast)
-
-def _test_xbm(h):
-    """X bitmap (X10 or X11)"""
-    if h.startswith(b'#define '):
-        return 'xbm'
-
-tests.append(_test_xbm)
-
-def _test_bmp(h):
-    if h.startswith(b'BM'):
-        return 'bmp'
-
-tests.append(_test_bmp)
-
-def _test_webp(h):
-    if h.startswith(b'RIFF') and h[8:12] == b'WEBP':
-        return 'webp'
-
-tests.append(_test_webp)
-
-def _test_exr(h):
-    if h.startswith(b'\x76\x2f\x31\x01'):
-        return 'exr'
-
-tests.append(_test_exr)
-
-
 class MIMEImage(MIMENonMultipart):
     """Class for generating image/* type MIME documents."""
 
@@ -137,10 +34,119 @@ class MIMEImage(MIMENonMultipart):
         constructor, which turns them into parameters on the Content-Type
         header.
         """
+        _subtype = _what(_imagedata) if _subtype is None else _subtype
         if _subtype is None:
-            if (_subtype := _what(_imagedata)) is None:
-                raise TypeError('Could not guess image MIME subtype')
+            raise TypeError('Could not guess image MIME subtype')
         MIMENonMultipart.__init__(self, 'image', _subtype, policy=policy,
                                   **_params)
         self.set_payload(_imagedata)
         _encoder(self)
+
+
+_rules = []
+
+
+# Originally from the imghdr module.
+def _what(data):
+    for rule in _rules:
+        if res := rule(data):
+            return res
+    else:
+        return None
+
+
+def rule(rulefunc):
+    _rules.append(rulefunc)
+    return rulefunc
+
+
+@rule
+def _jpeg(h):
+    """JPEG data with JFIF or Exif markers; and raw JPEG"""
+    if h[6:10] in (b'JFIF', b'Exif'):
+        return 'jpeg'
+    elif h[:4] == b'\xff\xd8\xff\xdb':
+        return 'jpeg'
+
+
+@rule
+def _png(h):
+    if h.startswith(b'\211PNG\r\n\032\n'):
+        return 'png'
+
+
+@rule
+def _gif(h):
+    """GIF ('87 and '89 variants)"""
+    if h[:6] in (b'GIF87a', b'GIF89a'):
+        return 'gif'
+
+
+@rule
+def _tiff(h):
+    """TIFF (can be in Motorola or Intel byte order)"""
+    if h[:2] in (b'MM', b'II'):
+        return 'tiff'
+
+
+@rule
+def _rgb(h):
+    """SGI image library"""
+    if h.startswith(b'\001\332'):
+        return 'rgb'
+
+
+@rule
+def _pbm(h):
+    """PBM (portable bitmap)"""
+    if len(h) >= 3 and \
+            h[0] == ord(b'P') and h[1] in b'14' and h[2] in b' \t\n\r':
+        return 'pbm'
+
+
+@rule
+def _pgm(h):
+    """PGM (portable graymap)"""
+    if len(h) >= 3 and \
+            h[0] == ord(b'P') and h[1] in b'25' and h[2] in b' \t\n\r':
+        return 'pgm'
+
+
+@rule
+def _ppm(h):
+    """PPM (portable pixmap)"""
+    if len(h) >= 3 and \
+            h[0] == ord(b'P') and h[1] in b'36' and h[2] in b' \t\n\r':
+        return 'ppm'
+
+
+@rule
+def _rast(h):
+    """Sun raster file"""
+    if h.startswith(b'\x59\xA6\x6A\x95'):
+        return 'rast'
+
+
+@rule
+def _xbm(h):
+    """X bitmap (X10 or X11)"""
+    if h.startswith(b'#define '):
+        return 'xbm'
+
+
+@rule
+def _bmp(h):
+    if h.startswith(b'BM'):
+        return 'bmp'
+
+
+@rule
+def _webp(h):
+    if h.startswith(b'RIFF') and h[8:12] == b'WEBP':
+        return 'webp'
+
+
+@rule
+def _exr(h):
+    if h.startswith(b'\x76\x2f\x31\x01'):
+        return 'exr'

Lib/test/test_email/data/python.pbm

@@ -0,0 +1,3 @@
+P4
+16 16
+ûñ¿úßÕ­±[ñ¥a_ÁX
°
°
ðððð?ÿÿ

Lib/test/test_email/data/python.xbm

@@ -0,0 +1,6 @@
+#define python_width 16
+#define python_height 16
+static char python_bits[] = {
+  0xDF, 0xFE, 0x8F, 0xFD, 0x5F, 0xFB, 0xAB, 0xFE, 0xB5, 0x8D, 0xDA, 0x8F, 
+  0xA5, 0x86, 0xFA, 0x83, 0x1A, 0x80, 0x0D, 0x80, 0x0D, 0x80, 0x0F, 0xE0, 
+  0x0F, 0xF8, 0x0F, 0xF8, 0x0F, 0xFC, 0xFF, 0xFF, };

Lib/test/test_email/test_email.py

@@ -798,7 +798,7 @@ class TestMessageAPI(TestEmailBase):
 class TestEncoders(unittest.TestCase):
 
     def test_EncodersEncode_base64(self):
-        with openfile('PyBanner048.gif', 'rb') as fp:
+        with openfile('python.gif', 'rb') as fp:
             bindata = fp.read()
         mimed = email.mime.image.MIMEImage(bindata)
         base64ed = mimed.get_payload()
@@ -1555,24 +1555,44 @@ class TestMIMEAudio(unittest.TestCase):
 
 # Test the basic MIMEImage class
 class TestMIMEImage(unittest.TestCase):
-    def setUp(self):
-        with openfile('PyBanner048.gif', 'rb') as fp:
+    def _make_image(self, ext):
+        with openfile(f'python.{ext}', 'rb') as fp:
             self._imgdata = fp.read()
         self._im = MIMEImage(self._imgdata)
 
     def test_guess_minor_type(self):
-        self.assertEqual(self._im.get_content_type(), 'image/gif')
+        for ext, subtype in {
+            'bmp': None,
+            'exr': None,
+            'gif': None,
+            'jpg': 'jpeg',
+            'pbm': None,
+            'pgm': None,
+            'png': None,
+            'ppm': None,
+            'ras': 'rast',
+            'sgi': 'rgb',
+            'tiff': None,
+            'webp': None,
+            'xbm': None,
+        }.items():
+            self._make_image(ext)
+            subtype = ext if subtype is None else subtype
+            self.assertEqual(self._im.get_content_type(), f'image/{subtype}')
 
     def test_encoding(self):
+        self._make_image('gif')
         payload = self._im.get_payload()
         self.assertEqual(base64.decodebytes(bytes(payload, 'ascii')),
-                self._imgdata)
+                         self._imgdata)
 
     def test_checkSetMinor(self):
+        self._make_image('gif')
         im = MIMEImage(self._imgdata, 'fish')
         self.assertEqual(im.get_content_type(), 'image/fish')
 
     def test_add_header(self):
+        self._make_image('gif')
         eq = self.assertEqual
         self._im.add_header('Content-Disposition', 'attachment',
                             filename='dingusfish.gif')
@@ -1747,7 +1767,7 @@ class TestMIMEText(unittest.TestCase):
 # Test complicated multipart/* messages
 class TestMultipart(TestEmailBase):
     def setUp(self):
-        with openfile('PyBanner048.gif', 'rb') as fp:
+        with openfile('python.gif', 'rb') as fp:
             data = fp.read()
         container = MIMEBase('multipart', 'mixed', boundary='BOUNDARY')
         image = MIMEImage(data, name='dingusfish.gif')
@@ -3444,7 +3464,7 @@ multipart/report
     def test_mime_classes_policy_argument(self):
         with openfile('audiotest.au', 'rb') as fp:
             audiodata = fp.read()
-        with openfile('PyBanner048.gif', 'rb') as fp:
+        with openfile('python.gif', 'rb') as fp:
             bindata = fp.read()
         classes = [
             (MIMEApplication, ('',)),