From b22174adf57e96e71eeb6fb4dc54e254343ff80c Mon Sep 17 00:00:00 2001
From: Roberto Rosario <roberto.rosario.gonzalez@gmail.com>
Date: Sat, 4 Aug 2018 00:49:58 -0400
Subject: [PATCH] Fix crop transformation argument parsing. Add error checking
 to the crop transformation arguments. Thanks to Jordan Wages (@wagesj45) for
 the report and investigation on the issue. Closes GitLab issue #490

Signed-off-by: Roberto Rosario <roberto.rosario.gonzalez@gmail.com>
---
 HISTORY.rst                                   |  4 ++
 .../converter/tests/test_transformations.py   | 56 ++++++++++++++++++-
 mayan/apps/converter/transformations.py       | 42 +++++++++++++-
 3 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/HISTORY.rst b/HISTORY.rst
index f9fa25a071..3d8ea93243 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -9,6 +9,10 @@
 - Mailer app: Add natural key support to the mailer app.
 - Cabinets: Redirect to the cabinet list view after creating a new cabinet.
 - Builds: Limit the number of branches that trigger the full test suit.
+- Converter app: Fix crop transformation argument parsing.
+- Converter app: Add error checking to the crop transformation arguments.
+  Thanks to Jordan Wages (@wagesj45) for the report and investigation on the issue.
+  Closes GitLab issue #490
 
 3.0.1 (2018-07-08)
 =================
diff --git a/mayan/apps/converter/tests/test_transformations.py b/mayan/apps/converter/tests/test_transformations.py
index 5c6f5c85b6..ad613dfaa8 100644
--- a/mayan/apps/converter/tests/test_transformations.py
+++ b/mayan/apps/converter/tests/test_transformations.py
@@ -2,9 +2,12 @@ from __future__ import unicode_literals
 
 from django.test import TestCase
 
+from documents.tests import GenericDocumentTestCase
+
+from ..models import Transformation
 from ..transformations import (
-    BaseTransformation, TransformationResize, TransformationRotate,
-    TransformationZoom
+    BaseTransformation, TransformationCrop, TransformationResize,
+    TransformationRotate, TransformationZoom
 )
 
 TRANSFORMATION_RESIZE_WIDTH = 123
@@ -20,7 +23,7 @@ TRANSFORMATION_ZOOM_PERCENT = 49
 TRANSFORMATION_ZOOM_CACHE_HASH = '1b333603674469e0'
 
 
-class TransformationTestCase(TestCase):
+class TransformationBaseTestCase(TestCase):
     def test_cache_uniqness(self):
         transformation_1 = TransformationResize(width=640, height=640)
 
@@ -101,3 +104,50 @@ class TransformationTestCase(TestCase):
                 (transformation_rotate, transformation_resize, transformation_zoom)
             ), TRANSFORMATION_COMBINED_CACHE_HASH
         )
+
+
+class TransformationTestCase(GenericDocumentTestCase):
+    def test_crop_transformation_optional_arguments(self):
+        document_page = self.document.pages.first()
+
+        Transformation.objects.add_for_model(
+            obj=document_page, transformation=TransformationCrop,
+            arguments={'top': '10'}
+        )
+
+        self.assertTrue(document_page.generate_image().startswith('page'))
+
+    def test_crop_transformation_invalid_arguments(self):
+        document_page = self.document.pages.first()
+
+        Transformation.objects.add_for_model(
+            obj=document_page, transformation=TransformationCrop,
+            arguments={'top': 'x', 'left': '-'}
+        )
+
+        self.assertTrue(document_page.generate_image().startswith('page'))
+
+    def test_crop_transformation_non_valid_range_arguments(self):
+        document_page = self.document.pages.first()
+
+        Transformation.objects.add_for_model(
+            obj=document_page, transformation=TransformationCrop,
+            arguments={'top': '-1000', 'bottom': '100000000'}
+        )
+
+        self.assertTrue(document_page.generate_image().startswith('page'))
+
+    def test_crop_transformation_overlapping_ranges_arguments(self):
+        document_page = self.document.pages.first()
+
+        Transformation.objects.add_for_model(
+            obj=document_page, transformation=TransformationCrop,
+            arguments={'top': '1000', 'bottom': '1000'}
+        )
+
+        Transformation.objects.add_for_model(
+            obj=document_page, transformation=TransformationCrop,
+            arguments={'left': '1000', 'right': '10000'}
+        )
+
+        self.assertTrue(document_page.generate_image().startswith('page'))
diff --git a/mayan/apps/converter/transformations.py b/mayan/apps/converter/transformations.py
index 171d1d5768..f2a83cfc68 100644
--- a/mayan/apps/converter/transformations.py
+++ b/mayan/apps/converter/transformations.py
@@ -88,10 +88,48 @@ class TransformationCrop(BaseTransformation):
     def execute_on(self, *args, **kwargs):
         super(TransformationCrop, self).execute_on(*args, **kwargs)
 
-        return self.image.crop(
-            (self.left, self.top, self.right, self.bottom)
+        left = float(self.left or '0')
+        top = float(self.top or '0')
+        right = self.image.size[0] - float(self.right or '0')
+        bottom = self.image.size[1] - float(self.bottom or '0')
+
+        if left < 0:
+            left = 0
+
+        if left > self.image.size[0] - 1:
+            left = self.image.size[0] - 1
+
+        if top < 0:
+            top = 0
+
+        if top > self.image.size[1] - 1:
+            top = self.image.size[1] - 1
+
+        if right < 0:
+            right = 0
+
+        if right > self.image.size[0] - 1:
+            right = self.image.size[0] - 1
+
+        if bottom < 0:
+            bottom = 0
+
+        if bottom > self.image.size[1] - 1:
+            bottom = self.image.size[1] - 1
+
+        if left > right:
+            left = right - 1
+
+        if top > bottom:
+            top = bottom - 1
+
+        logger.debug(
+            'left: %f, top: %f, right: %f, bottom: %f', left, top, right,
+            bottom
         )
 
+        return self.image.crop((left, top, right, bottom))
+
 
 class TransformationFlip(BaseTransformation):
     arguments = ()