From e06e826d577c8e5b42b83ae09bff93c80dad0db4 Mon Sep 17 00:00:00 2001 From: JJ Date: Sun, 21 Jun 2015 21:29:05 -0400 Subject: [PATCH 1/4] Add Exception and test for if request._hit_htmlmin is ever set to False (which it should not be) --- htmlmin/middleware.py | 3 +++ htmlmin/tests/test_middleware.py | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/htmlmin/middleware.py b/htmlmin/middleware.py index bf6a878c..904be906 100644 --- a/htmlmin/middleware.py +++ b/htmlmin/middleware.py @@ -20,6 +20,9 @@ def can_minify_response(self, request, response): req_ok = request._hit_htmlmin except AttributeError: return False + else: + if not req_ok: + raise ValueError("`request._hit_htmlmin` was set to False, when should only be True or not set.") if hasattr(settings, 'EXCLUDE_FROM_MINIFYING'): for url_pattern in settings.EXCLUDE_FROM_MINIFYING: diff --git a/htmlmin/tests/test_middleware.py b/htmlmin/tests/test_middleware.py index 183a3bf5..aaa80f2d 100644 --- a/htmlmin/tests/test_middleware.py +++ b/htmlmin/tests/test_middleware.py @@ -189,6 +189,14 @@ def test_should_set_flag_when_request_hits_middleware(self): MarkRequestMiddleware().process_request(request_mock) self.assertTrue(request_mock._hit_htmlmin) + def test_should_never_set_flag_to_false(self): + request_mock = RequestBareMock() + request_mock._hit_htmlmin = False + with self.assertRaises(ValueError): + response = HtmlMinifyMiddleware().process_response( + request_mock, ResponseMock(), + ) + def test_should_not_minify_when_request_did_not_hit_middleware(self): expected_output = " some text here " From 73837c2cd6bf3537384de76fd13695e64a0a9d2d Mon Sep 17 00:00:00 2001 From: JJ Date: Sun, 21 Jun 2015 21:29:53 -0400 Subject: [PATCH 2/4] Return False immediately from middleware.can_minify_response rather than perform unneeded checks --- htmlmin/middleware.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/htmlmin/middleware.py b/htmlmin/middleware.py index 904be906..6e08ce0d 100644 --- a/htmlmin/middleware.py +++ b/htmlmin/middleware.py @@ -28,14 +28,13 @@ def can_minify_response(self, request, response): for url_pattern in settings.EXCLUDE_FROM_MINIFYING: regex = re.compile(url_pattern) if regex.match(request.path.lstrip('/')): - req_ok = False - break + return False resp_ok = response.status_code == 200 resp_ok = resp_ok and 'text/html' in response['Content-Type'] if hasattr(response, 'minify_response'): resp_ok = resp_ok and response.minify_response - return req_ok and resp_ok + return resp_ok def process_response(self, request, response): minify = getattr(settings, "HTML_MINIFY", not settings.DEBUG) From fbb88594674582e3c9a8b137e0bcc99a53916232 Mon Sep 17 00:00:00 2001 From: JJ Date: Sun, 21 Jun 2015 21:36:51 -0400 Subject: [PATCH 3/4] Prevent middleware from throwing KeyError when 'Content-Type' header not set on request --- htmlmin/middleware.py | 2 +- htmlmin/tests/test_middleware.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/htmlmin/middleware.py b/htmlmin/middleware.py index 6e08ce0d..1cae6515 100644 --- a/htmlmin/middleware.py +++ b/htmlmin/middleware.py @@ -31,7 +31,7 @@ def can_minify_response(self, request, response): return False resp_ok = response.status_code == 200 - resp_ok = resp_ok and 'text/html' in response['Content-Type'] + resp_ok = resp_ok and 'text/html' in response.get('Content-Type', '') if hasattr(response, 'minify_response'): resp_ok = resp_ok and response.minify_response return resp_ok diff --git a/htmlmin/tests/test_middleware.py b/htmlmin/tests/test_middleware.py index aaa80f2d..9124c281 100644 --- a/htmlmin/tests/test_middleware.py +++ b/htmlmin/tests/test_middleware.py @@ -197,6 +197,18 @@ def test_should_never_set_flag_to_false(self): request_mock, ResponseMock(), ) + def test_does_not_throw_error_when_no_content_type_set(self): + response_mock = ResponseMock() + if 'Content-Type' in response_mock: + del response_mock['Content-Type'] + + # no self.assertNotRaises exists, but failing test would raise + # a KeyError + response = HtmlMinifyMiddleware().process_response( + RequestMock(), response_mock, + ) + + def test_should_not_minify_when_request_did_not_hit_middleware(self): expected_output = " some text here " From d1e3299233f12f48aaef480c4ab55685cc1d0818 Mon Sep 17 00:00:00 2001 From: JJ Date: Sun, 21 Jun 2015 21:53:16 -0400 Subject: [PATCH 4/4] Add Python 2.6 compatibility for test_should_never_set_flag_to_false middleware test --- htmlmin/tests/test_middleware.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/htmlmin/tests/test_middleware.py b/htmlmin/tests/test_middleware.py index 9124c281..78177ead 100644 --- a/htmlmin/tests/test_middleware.py +++ b/htmlmin/tests/test_middleware.py @@ -192,11 +192,14 @@ def test_should_set_flag_when_request_hits_middleware(self): def test_should_never_set_flag_to_false(self): request_mock = RequestBareMock() request_mock._hit_htmlmin = False - with self.assertRaises(ValueError): + + def process(): response = HtmlMinifyMiddleware().process_response( request_mock, ResponseMock(), ) + self.assertRaises(ValueError, process) + def test_does_not_throw_error_when_no_content_type_set(self): response_mock = ResponseMock() if 'Content-Type' in response_mock: