[PATCH 3/4] sources: Forbid mixing hash types

Mathieu Bridon bochecha at fedoraproject.org
Tue Feb 17 03:33:45 UTC 2015


From: Mathieu Bridon <bochecha at daitauha.fr>

There theoretically isn't any problem with mixing various hashes, as far
as rpkg is concerned.

However, taking for example the following file:

    $ cat sources
    MD5 (pygit2-0.22.0.tar.gz) = 31dc65b3cbe6e39d75a39db86dd7cd8c
    SHA512 (pygit2-0.22.0.tar.gz) =2079adc3285bf08fc7227b412b9d3dc[...]

When someone tries verifying it with `md5sum -c` they would get:

    $ md5sum -c sources
    pygit2-0.22.0.tar.gz: OK
    md5sum: WARNING: 1 line is improperly formatted

They'd need to also to check the file with `sha512sum -c`, but then they
would get:

    $ sha512sum -c sources
    pygit2-0.22.0.tar.gz: OK
    sha512sum: WARNING: 1 line is improperly formatted

This is clearly not ideal, and it might cause confusion.

Mixing might happen when a file contains lines with the old hash, then
after the migration a packager tries to upload an additional source
file.

This commit prevents mixing, by forcing packagers to use `new-sources`
and redo the source file entirely with the new hash.

It's a one time annoyance, but in addition to reducing future confusion,
it helps moving towards the goal of completely dropping MD5.
---
 src/pyrpkg/__init__.py | 20 ++++++++++++++++++--
 src/pyrpkg/sources.py  | 18 ++++++++++++++++--
 test/test_sources.py   | 11 +++++++++++
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py
index 7ecc8b4..57d9b75 100644
--- a/src/pyrpkg/__init__.py
+++ b/src/pyrpkg/__init__.py
@@ -38,7 +38,7 @@ try:
 except ImportError:
     pass
 
-from pyrpkg.sources import SourcesFile
+from pyrpkg.sources import HashtypeMixingError, SourcesFile
 
 
 # Define our own error class
@@ -2250,7 +2250,23 @@ class Commands(object):
             file_hash = self._hash_file(f, self.lookasidehash)
             self.log.info("Uploading: %s  %s" % (file_hash, f))
             file_basename = os.path.basename(f)
-            sourcesf.add_entry(self.lookasidehash, file_basename, file_hash)
+
+            try:
+                sourcesf.add_entry(self.lookasidehash, file_basename,
+                                   file_hash)
+            except HashtypeMixingError as e:
+                msg = '\n'.join([
+                    'Can not upload a new source file with a %(newhash)s '
+                    'hash, as the "%(sources)s" file contains at least one '
+                    'line with a %(existinghash)s hash.', '',
+                    'Please redo the whole "%(sources)s" file using:',
+                    '    `%(arg0)s new-sources file1 file2 ...`']) % {
+                        'newhash': e.new_hashtype,
+                        'existinghash': e.existing_hashtype,
+                        'sources': self.sources_filename,
+                        'arg0': sys.argv[0],
+                    }
+                raise rpkgError(msg)
 
             # Add this file to .gitignore if it's not already there:
             if not gitignore.match(file_basename):
diff --git a/src/pyrpkg/sources.py b/src/pyrpkg/sources.py
index 340c3e1..9c3b22e 100644
--- a/src/pyrpkg/sources.py
+++ b/src/pyrpkg/sources.py
@@ -24,6 +24,14 @@ LINE_PATTERN = re.compile(
     r'^(?P<hashtype>[^ ]+?) \((?P<file>[^ )]+?)\) = (?P<hash>[^ ]+?)$')
 
 
+class HashtypeMixingError(Exception):
+    def __init__(self, existing_hashtype, new_hashtype):
+        super(HashtypeMixingError, self).__init__()
+
+        self.existing_hashtype = existing_hashtype
+        self.new_hashtype = new_hashtype
+
+
 class MalformedLineError(Exception):
     pass
 
@@ -67,8 +75,14 @@ class SourcesFile(object):
     def add_entry(self, hashtype, file, hash):
         entry = SourceFileEntry(hashtype, file, hash)
 
-        if entry not in self.entries:
-            self.entries.append(entry)
+        for e in self.entries:
+            if entry.hashtype != e.hashtype:
+                raise HashtypeMixingError(e.hashtype, entry.hashtype)
+
+            if entry == e:
+                return
+
+        self.entries.append(entry)
 
     def write(self):
         with open(self.sourcesfile, 'w') as f:
diff --git a/test/test_sources.py b/test/test_sources.py
index fb91b3e..ba90427 100644
--- a/test/test_sources.py
+++ b/test/test_sources.py
@@ -189,6 +189,17 @@ class SourcesFileTestCase(unittest.TestCase):
         s.add_entry('md5', 'afile', 'ahash')
         self.assertEqual(len(s.entries), 1)
 
+    def test_add_entry_mixing_hashtypes(self):
+        s = sources.SourcesFile(self.sourcesfile)
+        self.assertEqual(len(s.entries), 0)
+
+        s.add_entry('md5', 'afile', 'ahash')
+        self.assertEqual(len(s.entries), 1)
+        self.assertEqual(str(s.entries[-1]), 'MD5 (afile) = ahash\n')
+
+        with self.assertRaises(sources.HashtypeMixingError):
+            s.add_entry('sha512', 'anotherfile', 'anotherhash')
+
     def test_write_new_file(self):
         s = sources.SourcesFile(self.sourcesfile)
         self.assertEqual(len(s.entries), 0)
-- 
2.1.0



More information about the buildsys mailing list