From 44f267a5964ea8dbb59c26c319e43fad84afb45a Mon Sep 17 00:00:00 2001
From: jvoisin <julien.voisin@dustri.org>
Date: Mon, 22 Oct 2018 16:45:30 +0200
Subject: [PATCH] Improve problematic filenames support
---
libmat2/exiftool.py | 24 ++++++++++--------------
libmat2/video.py | 17 +++++++++--------
tests/test_libmat2.py | 13 +++++++++++++
3 files changed, 32 insertions(+), 22 deletions(-)
diff --git a/libmat2/exiftool.py b/libmat2/exiftool.py
index e17d31b..331ae0c 100644
--- a/libmat2/exiftool.py
+++ b/libmat2/exiftool.py
@@ -5,7 +5,7 @@ import shutil
import subprocess
import tempfile
-from typing import Dict, Union, Set
+from typing import Dict, Union, Set, Callable, Any
from . import abstract
@@ -20,27 +20,23 @@ class ExiftoolParser(abstract.AbstractParser):
"""
meta_whitelist = set() # type: Set[str]
- @staticmethod
- def __handle_problematic_filename(filename: str, callback) -> bytes:
- """ This method takes a filename with a problematic name,
- and safely applies it a `callback`."""
+ def _handle_problematic_filename(self, callback: Callable[[str], Any]) -> bytes:
+ """ This method takes a filename with a potentially problematic name,
+ and safely applies a `callback` to it.
+ """
+ if re.search('^[a-z0-9/]', self.filename) is not None:
+ return callback(self.filename)
+
tmpdirname = tempfile.mkdtemp()
fname = os.path.join(tmpdirname, "temp_file")
- shutil.copy(filename, fname)
+ shutil.copy(self.filename, fname)
out = callback(fname)
shutil.rmtree(tmpdirname)
return out
def get_meta(self) -> Dict[str, Union[str, dict]]:
- """ There is no way to escape the leading(s) dash(es) of the current
- self.filename to prevent parameter injections, so we need to take care
- of this.
- """
fun = lambda f: subprocess.check_output([_get_exiftool_path(), '-json', f])
- if re.search('^[a-z0-9/]', self.filename) is None:
- out = self.__handle_problematic_filename(self.filename, fun)
- else:
- out = fun(self.filename)
+ out = self._handle_problematic_filename(fun)
meta = json.loads(out.decode('utf-8'))[0]
for key in self.meta_whitelist:
meta.pop(key, None)
diff --git a/libmat2/video.py b/libmat2/video.py
index 658affa..2fa65e8 100644
--- a/libmat2/video.py
+++ b/libmat2/video.py
@@ -1,5 +1,6 @@
import os
import subprocess
+import logging
from . import exiftool
@@ -23,13 +24,10 @@ class AVIParser(exiftool.ExiftoolParser):
'SampleRate', 'AvgBytesPerSec', 'BitsPerSample',
'Duration', 'ImageSize', 'Megapixels'}
- def remove_all(self) -> bool:
- """
- TODO: handle problematic filenames starting with `-` and `--`,
- check exiftool.py
- """
+
+ def __remove_all_internal(self, filename: str):
cmd = [_get_ffmpeg_path(),
- '-i', self.filename, # input file
+ '-i', filename, # input file
'-y', # overwrite existing output file
'-loglevel', 'panic', # Don't show log
'-hide_banner', # hide the banner
@@ -40,10 +38,13 @@ class AVIParser(exiftool.ExiftoolParser):
'-flags:v', '+bitexact', # don't add any metadata
'-flags:a', '+bitexact', # don't add any metadata
self.output_filename]
+ subprocess.check_call(cmd)
+ def remove_all(self) -> bool:
try:
- subprocess.check_call(cmd)
- except subprocess.CalledProcessError:
+ self._handle_problematic_filename(self.__remove_all_internal)
+ except subprocess.CalledProcessError as e:
+ logging.error("Something went wrong during the processing of %s: %s", self.filename, e)
return False
return True
diff --git a/tests/test_libmat2.py b/tests/test_libmat2.py
index e5cc8a3..241c6eb 100644
--- a/tests/test_libmat2.py
+++ b/tests/test_libmat2.py
@@ -37,6 +37,19 @@ class TestParameterInjection(unittest.TestCase):
self.assertEqual(meta['ModifyDate'], "2018:03:20 21:59:25")
os.remove('-ver')
+ def test_ffmpeg_injection(self):
+ try:
+ video._get_ffmpeg_path()
+ except RuntimeError:
+ raise unittest.SkipTest
+
+ shutil.copy('./tests/data/dirty.avi', './--output')
+ p = video.AVIParser('--output')
+ meta = p.get_meta()
+ print(meta)
+ self.assertEqual(meta['Software'], 'MEncoder SVN-r33148-4.0.1')
+ os.remove('--output')
+
class TestUnsupportedEmbeddedFiles(unittest.TestCase):
def test_odt_with_svg(self):
--
GitLab