Gilles has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/316305

Change subject: Upgrade to 0.1.28
......................................................................

Upgrade to 0.1.28

Bug: T148363
Bug: T148361
Bug: T145623
Change-Id: Ic7de8d8e7926f9a3cc987bcd350f601eed32f5f6
---
M debian/changelog
M setup.py
M tests/integration/test_huge_video.py
M wikimedia_thumbor/engine/__init__.py
M wikimedia_thumbor/engine/vips/vips.py
M wikimedia_thumbor/loader/video/__init__.py
M wikimedia_thumbor/shell_runner/__init__.py
7 files changed, 74 insertions(+), 28 deletions(-)


  git pull 
ssh://gerrit.wikimedia.org:29418/operations/debs/python-thumbor-wikimedia 
refs/changes/05/316305/1

diff --git a/debian/changelog b/debian/changelog
index 63f4364..3973044 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+python-thumbor-wikimedia (0.1.28-1) jessie-wikimedia; urgency=low
+
+  * New upstream release
+
+ -- Gilles Dubuc <gil...@wikimedia.org>  Mon, 17 Oct 2016 11:43:00 +0000
+
 python-thumbor-wikimedia (0.1.27-1) jessie-wikimedia; urgency=low
 
   * New upstream release
diff --git a/setup.py b/setup.py
index 542992b..f0f61ed 100644
--- a/setup.py
+++ b/setup.py
@@ -12,7 +12,7 @@
 
 setup(
     name='wikimedia_thumbor',
-    version='0.1.27',
+    version='0.1.28',
     url='https://phabricator.wikimedia.org/diffusion/THMBREXT/',
     license='MIT',
     author='Gilles Dubuc, Wikimedia Foundation',
diff --git a/tests/integration/test_huge_video.py 
b/tests/integration/test_huge_video.py
index c648aa4..0c2384d 100644
--- a/tests/integration/test_huge_video.py
+++ b/tests/integration/test_huge_video.py
@@ -16,3 +16,15 @@
             0.98,
             1.0
         )
+
+    def test_404(self):
+        url = 'unsafe/320x/filters:page(82)/https://upload.wikimedia.org/'\
+            'wikipedia/commons/a/ab/Thisfiledoesntexist.webm'
+
+        try:
+            result = self.retrieve("/%s" % url)
+        except Exception as e:
+            assert False, 'Exception occured: %r' % e
+
+        assert result is not None, 'No result'
+        assert result.code == 404, 'Response code: %s' % result.code
diff --git a/wikimedia_thumbor/engine/__init__.py 
b/wikimedia_thumbor/engine/__init__.py
index d96413d..18df653 100644
--- a/wikimedia_thumbor/engine/__init__.py
+++ b/wikimedia_thumbor/engine/__init__.py
@@ -26,10 +26,6 @@
 
 
 class BaseWikimediaEngine(IMEngine):
-    # Put temp files and fifos into their own temp folder to avoid
-    # exploits where converters might access other files in the same folder
-    temp_dir = mkdtemp()
-
     @classmethod
     def add_format(cls, mime, ext, fn):
         # Unfortunately there is no elegant way to extend Thumbor to support
@@ -80,6 +76,9 @@
             return
 
         logger.debug('[BWE] Create source file from buffer')
+        # Put temp files into their own temp folder to avoid
+        # exploits where converters might access other files in the same folder
+        self.temp_dir = mkdtemp()
         self.source = os.path.join(self.temp_dir, 'source_file')
 
         with open(self.source, 'w') as source:
@@ -88,9 +87,8 @@
     def cleanup_source(self):
         if hasattr(self, 'source'):
             ShellRunner.rm_f(self.source)
-
-    def cleanup(self):  # pragma: no cover
-        shutil.rmtree(self.temp_dir, True)
+        if hasattr(self, 'temp_dir'):
+            shutil.rmtree(self.temp_dir, True)
 
     def command(self, command, env=None, clean_on_error=True):
         returncode, stderr, stdout = ShellRunner.command(
diff --git a/wikimedia_thumbor/engine/vips/vips.py 
b/wikimedia_thumbor/engine/vips/vips.py
index e3cb80a..5d3e436 100644
--- a/wikimedia_thumbor/engine/vips/vips.py
+++ b/wikimedia_thumbor/engine/vips/vips.py
@@ -13,6 +13,8 @@
 
 import math
 import os
+import shutil
+from tempfile import mkdtemp
 
 from thumbor.utils import logger
 
@@ -124,8 +126,9 @@
             raise e
 
     def shrink_for_page(self, source, shrink_factor):
+        temp_dir = mkdtemp()
         destination = os.path.join(
-            self.temp_dir,
+            temp_dir,
             'vips_result%s' % self.target_format()
         )
 
@@ -142,6 +145,7 @@
             self.command(command, clean_on_error=False)
         except CommandError as e:  # pragma: no cover
             ShellRunner.rm_f(destination)
+            shutil.rmtree(temp_dir, True)
             raise e
 
         with open(destination, 'rb') as f:
@@ -149,5 +153,6 @@
 
         self.cleanup_source()
         ShellRunner.rm_f(destination)
+        shutil.rmtree(temp_dir, True)
 
         return result
diff --git a/wikimedia_thumbor/loader/video/__init__.py 
b/wikimedia_thumbor/loader/video/__init__.py
index 7285baf..3cb4939 100644
--- a/wikimedia_thumbor/loader/video/__init__.py
+++ b/wikimedia_thumbor/loader/video/__init__.py
@@ -15,6 +15,7 @@
 # original for subsequent requests
 
 import errno
+import re
 import os
 from functools import partial
 from tempfile import NamedTemporaryFile
@@ -69,7 +70,7 @@
 
     logger.debug('[Video] load_sync: %r' % command)
 
-    process = Subprocess(command, stdout=Subprocess.STREAM)
+    process = Subprocess(command, stdout=Subprocess.STREAM, 
stderr=Subprocess.STREAM)
     process.set_exit_callback(
         partial(
             _parse_time_status,
@@ -81,10 +82,30 @@
     )
 
 
+def _http_code_from_stderr(process, result):
+    result.successful = False
+    stderr = process.stderr.read_from_fd()
+
+    logger.warning('[Video] Fprobe/ffmpeg errored: %r' % stderr)
+    code = re.match('.*Server returned (\d\d\d).*', stderr)
+
+    if code:
+        code = int(code.group(1))
+
+    if code == 404:
+        result.error = LoaderResult.ERROR_NOT_FOUND
+    elif code == 599:
+        result.error = LoaderResult.ERROR_TIMEOUT
+    elif code == 500:
+        result.error = LoaderResult.ERROR_UPSTREAM
+
+
 def _parse_time_status(context, url, callback, process, status):
-    if status != 0:  # pragma: no cover
+    if status != 0:
         result = LoaderResult()
-        result.successful = False
+
+        _http_code_from_stderr(process, result)
+
         callback(result)
     else:
         process.stdout.read_until_close(
@@ -134,7 +155,7 @@
 
     logger.debug('[Video] _parse_time: %r' % command)
 
-    process = Subprocess(command, stdout=Subprocess.STREAM)
+    process = Subprocess(command, stdout=Subprocess.STREAM, 
stderr=Subprocess.STREAM)
     process.set_exit_callback(
         partial(
             _process_done,
@@ -166,7 +187,7 @@
     result = LoaderResult()
 
     if status != 0:  # pragma: no cover
-        result.successful = False
+        _http_code_from_stderr(process, result)
     else:
         result.successful = True
         result.buffer = output_file.read()
diff --git a/wikimedia_thumbor/shell_runner/__init__.py 
b/wikimedia_thumbor/shell_runner/__init__.py
index 9ca8dad..523e543 100644
--- a/wikimedia_thumbor/shell_runner/__init__.py
+++ b/wikimedia_thumbor/shell_runner/__init__.py
@@ -13,6 +13,7 @@
 
 import datetime
 import errno
+from functools import partial
 import os
 import subprocess
 
@@ -24,18 +25,7 @@
     def wrap_command(cls, command, context):
         wrapped_command = command
 
-        if (hasattr(context.config, 'SUBPROCESS_USE_CGEXEC')
-            and context.config.SUBPROCESS_USE_CGEXEC):  # pragma: no cover
-            cgroup = context.config.SUBPROCESS_CGROUP
-            cgexec_path = context.config.SUBPROCESS_CGEXEC_PATH
-            wrapped_command = [
-                cgexec_path,
-                '-g',
-                cgroup
-            ] + wrapped_command
-
-        if (hasattr(context.config, 'SUBPROCESS_USE_TIMEOUT')
-            and context.config.SUBPROCESS_USE_TIMEOUT):
+        if getattr(context.config, 'SUBPROCESS_USE_TIMEOUT', False):
             timeout = context.config.SUBPROCESS_TIMEOUT
             timeout_path = context.config.SUBPROCESS_TIMEOUT_PATH
             wrapped_command = [
@@ -45,6 +35,18 @@
             ] + wrapped_command
 
         return wrapped_command
+
+    @classmethod
+    def preexec(cls, context):  # pragma: no cover
+        if not getattr(context.config, 'SUBPROCESS_CGROUP_TASKS_PATH', False):
+            return
+
+        pid = os.getpid()
+
+        logger.debug('[ShellRunner] Adding pid %r to cgroup' % pid)
+
+        with open(context.config.SUBPROCESS_CGROUP_TASKS_PATH, 'a+') as tasks:
+            tasks.write('%s\n' % pid)
 
     @classmethod
     def popen(cls, command, context, stdin=None, env=None):
@@ -65,7 +67,8 @@
                 wrapped_command,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
-                env=combined_env
+                env=combined_env,
+                preexec_fn=partial(cls.preexec, context)
             )
         else:  # pragma: no cover
             proc = subprocess.Popen(
@@ -73,7 +76,8 @@
                 stdout=subprocess.PIPE,
                 stdin=subprocess.PIPE,
                 stderr=subprocess.PIPE,
-                env=combined_env
+                env=combined_env,
+                preexec_fn=partial(cls.preexec, context)
             )
             proc.stdin.write(stdin)
 

-- 
To view, visit https://gerrit.wikimedia.org/r/316305
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic7de8d8e7926f9a3cc987bcd350f601eed32f5f6
Gerrit-PatchSet: 1
Gerrit-Project: operations/debs/python-thumbor-wikimedia
Gerrit-Branch: master
Gerrit-Owner: Gilles <gdu...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to