#14977: Make mailcap respect the order of the lines in the mailcap file.

This is required by RFC 1542, so despite the subtle behavior change we
are treating it as a bug.  Patch by Michael Lazar.

3 files changed, 72 insertions, 27 deletions

diff --git a/Lib/mailcap.py b/Lib/mailcap.py
index 97e3035..bd0fc09 100644
--- a/Lib/mailcap.py
+++ b/Lib/mailcap.py
@@ -1,9 +1,19 @@
 """Mailcap file handling.  See RFC 1524."""
 
 import os
+import warnings
 
 __all__ = ["getcaps","findmatch"]
 
+
+def lineno_sort_key(entry):
+    # Sort in ascending order, with unspecified entries at the end
+    if 'lineno' in entry:
+        return 0, entry['lineno']
+    else:
+        return 1, 0
+
+
 # Part 1: top-level interface.
 
 def getcaps():
@@ -17,13 +27,14 @@ def getcaps():
 
     """
     caps = {}
+    lineno = 0
     for mailcap in listmailcapfiles():
         try:
             fp = open(mailcap, 'r')
         except OSError:
             continue
         with fp:
-            morecaps = readmailcapfile(fp)
+            morecaps, lineno = _readmailcapfile(fp, lineno)
         for key, value in morecaps.items():
             if not key in caps:
                 caps[key] = value
@@ -49,8 +60,15 @@ def listmailcapfiles():
 
 
 # Part 2: the parser.
-
 def readmailcapfile(fp):
+    """Read a mailcap file and return a dictionary keyed by MIME type."""
+    warnings.warn('readmailcapfile is deprecated, use getcaps instead',
+                  DeprecationWarning, 2)
+    caps, _ = _readmailcapfile(fp, None)
+    return caps
+
+
+def _readmailcapfile(fp, lineno):
     """Read a mailcap file and return a dictionary keyed by MIME type.
 
     Each MIME type is mapped to an entry consisting of a list of
@@ -76,6 +94,9 @@ def readmailcapfile(fp):
         key, fields = parseline(line)
         if not (key and fields):
             continue
+        if lineno is not None:
+            fields['lineno'] = lineno
+            lineno += 1
         # Normalize the key
         types = key.split('/')
         for j in range(len(types)):
@@ -86,7 +107,7 @@ def readmailcapfile(fp):
             caps[key].append(fields)
         else:
             caps[key] = [fields]
-    return caps
+    return caps, lineno
 
 def parseline(line):
     """Parse one entry in a mailcap file and return a dictionary.
@@ -165,6 +186,7 @@ def lookup(caps, MIMEtype, key=None):
         entries = entries + caps[MIMEtype]
     if key is not None:
         entries = [e for e in entries if key in e]
+    entries = sorted(entries, key=lineno_sort_key)
     return entries
 
 def subst(field, MIMEtype, filename, plist=[]):
diff --git a/Lib/test/mailcap.txt b/Lib/test/mailcap.txt
index f61135d..08a76e6 100644
--- a/Lib/test/mailcap.txt
+++ b/Lib/test/mailcap.txt
@@ -35,5 +35,5 @@ message/external-body; showexternal %s %{access-type} %{name} %{site} \
 text/richtext; shownonascii iso-8859-8 -e richtext -p %s; test=test "`echo \
     %{charset} | tr '[A-Z]' '[a-z]'`"  = iso-8859-8; copiousoutput
 
-video/mpeg; mpeg_play %s
 video/*; animate %s
+video/mpeg; mpeg_play %s
\ No newline at end of file
diff --git a/Lib/test/test_mailcap.py b/Lib/test/test_mailcap.py
index 22b2fcc..a85c691 100644
--- a/Lib/test/test_mailcap.py
+++ b/Lib/test/test_mailcap.py
@@ -1,6 +1,7 @@
 import mailcap
 import os
 import shutil
+import copy
 import test.support
 import unittest
 
@@ -14,43 +15,55 @@ MAILCAPDICT = {
         [{'compose': 'moviemaker %s',
           'x11-bitmap': '"/usr/lib/Zmail/bitmaps/movie.xbm"',
           'description': '"Movie"',
-          'view': 'movieplayer %s'}],
+          'view': 'movieplayer %s',
+          'lineno': 4}],
     'application/*':
         [{'copiousoutput': '',
-          'view': 'echo "This is \\"%t\\" but        is 50 \\% Greek to me" \\; cat %s'}],
+          'view': 'echo "This is \\"%t\\" but        is 50 \\% Greek to me" \\; cat %s',
+          'lineno': 5}],
     'audio/basic':
         [{'edit': 'audiocompose %s',
           'compose': 'audiocompose %s',
           'description': '"An audio fragment"',
-          'view': 'showaudio %s'}],
+          'view': 'showaudio %s',
+          'lineno': 6}],
     'video/mpeg':
-        [{'view': 'mpeg_play %s'}],
+        [{'view': 'mpeg_play %s', 'lineno': 13}],
     'application/postscript':
-        [{'needsterminal': '', 'view': 'ps-to-terminal %s'},
-         {'compose': 'idraw %s', 'view': 'ps-to-terminal %s'}],
+        [{'needsterminal': '', 'view': 'ps-to-terminal %s', 'lineno': 1},
+         {'compose': 'idraw %s', 'view': 'ps-to-terminal %s', 'lineno': 2}],
     'application/x-dvi':
-        [{'view': 'xdvi %s'}],
+        [{'view': 'xdvi %s', 'lineno': 3}],
     'message/external-body':
         [{'composetyped': 'extcompose %s',
           'description': '"A reference to data stored in an external location"',
           'needsterminal': '',
-          'view': 'showexternal %s %{access-type} %{name} %{site}     %{directory} %{mode} %{server}'}],
+          'view': 'showexternal %s %{access-type} %{name} %{site}     %{directory} %{mode} %{server}',
+          'lineno': 10}],
     'text/richtext':
         [{'test': 'test "`echo     %{charset} | tr \'[A-Z]\' \'[a-z]\'`"  = iso-8859-8',
           'copiousoutput': '',
-          'view': 'shownonascii iso-8859-8 -e richtext -p %s'}],
+          'view': 'shownonascii iso-8859-8 -e richtext -p %s',
+          'lineno': 11}],
     'image/x-xwindowdump':
-        [{'view': 'display %s'}],
+        [{'view': 'display %s', 'lineno': 9}],
     'audio/*':
-        [{'view': '/usr/local/bin/showaudio %t'}],
+        [{'view': '/usr/local/bin/showaudio %t', 'lineno': 7}],
     'video/*':
-        [{'view': 'animate %s'}],
+        [{'view': 'animate %s', 'lineno': 12}],
     'application/frame':
-        [{'print': '"cat %s | lp"', 'view': 'showframe %s'}],
+        [{'print': '"cat %s | lp"', 'view': 'showframe %s', 'lineno': 0}],
     'image/rgb':
-        [{'view': 'display %s'}]
+        [{'view': 'display %s', 'lineno': 8}]
 }
 
+# For backwards compatibility, readmailcapfile() and lookup() still support
+# the old version of mailcapdict without line numbers.
+MAILCAPDICT_DEPRECATED = copy.deepcopy(MAILCAPDICT)
+for entry_list in MAILCAPDICT_DEPRECATED.values():
+    for entry in entry_list:
+        entry.pop('lineno')
+
 
 class HelperFunctionTest(unittest.TestCase):
 
@@ -76,12 +89,14 @@ class HelperFunctionTest(unittest.TestCase):
     def test_readmailcapfile(self):
         # Test readmailcapfile() using test file. It should match MAILCAPDICT.
         with open(MAILCAPFILE, 'r') as mcf:
-            d = mailcap.readmailcapfile(mcf)
-        self.assertDictEqual(d, MAILCAPDICT)
+            with self.assertWarns(DeprecationWarning):
+                d = mailcap.readmailcapfile(mcf)
+        self.assertDictEqual(d, MAILCAPDICT_DEPRECATED)
 
     def test_lookup(self):
         # Test without key
-        expected = [{'view': 'mpeg_play %s'}, {'view': 'animate %s'}]
+        expected = [{'view': 'animate %s', 'lineno': 12},
+                    {'view': 'mpeg_play %s', 'lineno': 13}]
         actual = mailcap.lookup(MAILCAPDICT, 'video/mpeg')
         self.assertListEqual(expected, actual)
 
@@ -90,10 +105,16 @@ class HelperFunctionTest(unittest.TestCase):
         expected = [{'edit': 'audiocompose %s',
                      'compose': 'audiocompose %s',
                      'description': '"An audio fragment"',
-                     'view': 'showaudio %s'}]
+                     'view': 'showaudio %s',
+                     'lineno': 6}]
         actual = mailcap.lookup(MAILCAPDICT, 'audio/basic', key)
         self.assertListEqual(expected, actual)
 
+        # Test on user-defined dicts without line numbers
+        expected = [{'view': 'mpeg_play %s'}, {'view': 'animate %s'}]
+        actual = mailcap.lookup(MAILCAPDICT_DEPRECATED, 'video/mpeg')
+        self.assertListEqual(expected, actual)
+
     def test_subst(self):
         plist = ['id=1', 'number=2', 'total=3']
         # test case: ([field, MIMEtype, filename, plist=[]], <expected string>)
@@ -152,14 +173,16 @@ class FindmatchTest(unittest.TestCase):
             'edit': 'audiocompose %s',
             'compose': 'audiocompose %s',
             'description': '"An audio fragment"',
-            'view': 'showaudio %s'
+            'view': 'showaudio %s',
+            'lineno': 6
         }
-        audio_entry = {"view": "/usr/local/bin/showaudio %t"}
-        video_entry = {'view': 'animate %s'}
+        audio_entry = {"view": "/usr/local/bin/showaudio %t", 'lineno': 7}
+        video_entry = {'view': 'animate %s', 'lineno': 12}
         message_entry = {
             'composetyped': 'extcompose %s',
             'description': '"A reference to data stored in an external location"', 'needsterminal': '',
-            'view': 'showexternal %s %{access-type} %{name} %{site}     %{directory} %{mode} %{server}'
+            'view': 'showexternal %s %{access-type} %{name} %{site}     %{directory} %{mode} %{server}',
+            'lineno': 10,
         }
 
         # test case: (findmatch args, findmatch keyword args, expected output)
@@ -169,7 +192,7 @@ class FindmatchTest(unittest.TestCase):
         cases = [
             ([{}, "video/mpeg"], {}, (None, None)),
             ([c, "foo/bar"], {}, (None, None)),
-            ([c, "video/mpeg"], {}, ('mpeg_play /dev/null', {'view': 'mpeg_play %s'})),
+            ([c, "video/mpeg"], {}, ('animate /dev/null', video_entry)),
             ([c, "audio/basic", "edit"], {}, ("audiocompose /dev/null", audio_basic_entry)),
             ([c, "audio/basic", "compose"], {}, ("audiocompose /dev/null", audio_basic_entry)),
             ([c, "audio/basic", "description"], {}, ('"An audio fragment"', audio_basic_entry)),