Escape % in manifest filenames as required by RFC

src/bagit/__init__.py

@@ -1423,18 +1423,19 @@ def generate_manifest_lines(filename, algorithms=DEFAULT_CHECKSUMS):
             for hasher in hashers.values():
                 hasher.update(block)
 
-    decoded_filename = _decode_filename(filename)
-
     # We'll generate a list of results in roughly manifest format but prefixed with the algorithm:
     results = [
-        (alg, hasher.hexdigest(), decoded_filename, total_bytes)
+        (alg, hasher.hexdigest(), filename, total_bytes)
         for alg, hasher in hashers.items()
     ]
 
     return results
 
 
 def _encode_filename(s):
+    # needs to be first to not escape the % added by escaping \n and \r
+    s = s.replace("%", "%25")
+
     s = s.replace("\r", "%0D")
     s = s.replace("\n", "%0A")
     return s
@@ -1443,6 +1444,8 @@ def _encode_filename(s):
 def _decode_filename(s):
     s = re.sub(r"%0D", "\r", s, flags=re.IGNORECASE)
     s = re.sub(r"%0A", "\n", s, flags=re.IGNORECASE)
+    # needs to be last to not potentially introduce matches for \n / \r
+    s = s.replace("%25", "%")
     return s
 
 

test.py

@@ -111,7 +111,7 @@
 
     def test_validate_fast(self):
         bag = bagit.make_bag(self.tmpdir)
         self.assertEqual(self.validate(bag, fast=True), True)
         os.remove(j(self.tmpdir, "data", "loc", "2478433644_2839c5e8b8_o_d.jpg"))
         self.assertRaises(bagit.BagValidationError, self.validate, bag, fast=True)
 
@@ -827,10 +827,31 @@
         os.remove(j(self.tmpdir, "tagmanifest-md5.txt"))
         self.assertTrue(bag.is_valid())
 
-    def test_carriage_return_manifest(self):
-        with open(j(self.tmpdir, "newline\r"), "w") as whatever:
-            whatever.write("ugh")
+    def test_special_characters_in_filename(self):
+        names = [
+            "carriage-return\r",
+            "newline\n",
+            "percent%",
+            "escapes%0A%0D%25", # has all the escape sequences already in the name
+        ]
+
+        for name in names:
+            with open(j(self.tmpdir, name), "w") as f:
+                f.write("ugh")
+
         bag = bagit.make_bag(self.tmpdir)
+        bag_names = slurp_text_file(next(bag.manifest_files()))
+
+        escaped_names = [
+            "carriage-return%0D",
+            "newline%0A",
+            "percent%25",
+            "escapes%250A%250D%2525",
+        ]
+
+        for escaped_name in escaped_names:
+            self.assertIn(f"data/{escaped_name}", bag_names)
+
         self.assertTrue(bag.is_valid())
 
     def test_payload_permissions(self):
@@ -1144,7 +1165,7 @@
     def test_directory_required(self, mock_stderr):
         testargs = ["bagit.py"]
 
         with self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1158,7 +1179,7 @@
     def test_not_enough_processes(self, mock_stderr):
         testargs = ["bagit.py", "--processes", "0", self.tmpdir]
 
         with self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1173,7 +1194,7 @@
         bagit.make_bag(self.tmpdir)
         testargs = ["bagit.py", "--fast", self.tmpdir]
 
         with self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1188,7 +1209,7 @@
         os.remove(j(self.tmpdir, "data", "loc", "2478433644_2839c5e8b8_o_d.jpg"))
         testargs = ["bagit.py", "--validate", "--completeness-only", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1202,7 +1223,7 @@
         bagit.make_bag(self.tmpdir)
         testargs = ["bagit.py", "--validate", "--fast", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1217,7 +1238,7 @@
         bagit.make_bag(self.tmpdir)
         testargs = ["bagit.py", "--completeness-only", self.tmpdir]
 
         with self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1235,7 +1256,7 @@
 
         testargs = ["bagit.py", "--validate", "--completeness-only", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1249,7 +1270,7 @@
         bagit.make_bag(self.tmpdir)
         testargs = ["bagit.py", "--validate", "--completeness-only", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1269,7 +1290,7 @@
 
         testargs = ["bagit.py", "--validate", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1280,7 +1301,7 @@
         bagit.make_bag(self.tmpdir)
         testargs = ["bagit.py", "--validate", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1292,7 +1313,7 @@
 
         testargs = ["bagit.py", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()
 
@@ -1305,7 +1326,7 @@
     def test_create_bag(self):
         testargs = ["bagit.py", self.tmpdir]
 
         with self.assertLogs() as captured, self.assertRaises(SystemExit) as cm:
             with mock.patch.object(sys, "argv", testargs):
                 bagit.main()