Merge ~nansari/django-piston:fix-for-SN-1445 into django-piston:master

[Najam Ahmed Ansari]

Fix for SN-1445

This fixes the bug in SCA wherein the SCA resource error handler the clobbers original exception message.

[Przemysław Suliga (suligap)]

Hi, skimming the django-piston and Django code, I suspect a potential cleaner fix might be the removal of django-piston's HttpResponseWrapper (or not using the wrapper when on new enough Django). Have you tried that?

Because there are other places in django piston that set response.content as a string it looks like. Django's HttpResponse handles conversion and the wrapper doesn't. It's possible the wrapper is not needed for the django version we use?

[Przemysław Suliga (suligap)]

But that might be going against the "just apply a minimal fix to piston" approach. So if this works for SCA, then it should be fine.

[Maximiliano Bertacchini (maxiberta)]

I think I agree with Przemek here: piston's HttpResponseWrapper is apparently just trying to workaround an issue in django's HttpResponse, according to a comment in HttpResponseWrapper's code. But the issue http://code.djangoproject.com/ticket/9403 was fixed 11 years ago!!! :D:D:D

Maybe we can just drop HttpResponseWrapper and update piston's rc_factory.__getattr__() to return a plain HttpResponse? Afaict, the fix shipped in Django 1.4 (see https://github.com/django/django/commit/50255e3305 which is linked from the main issue). Based on piston's tox tests, we support Django >=1.4 which sounds like a really great coincidence and in which case we can just drop it and forget about django versions.

09f52da... by Najam Ahmed Ansari <email address hidden> on 2023-07-06

Removing piston's custom HttpResponseWrapper

The wrapper class was attempting to work around a bug in Django which has since
been fixed, rendering the wrapper unnecessary. Removing the wrapper also fixes
SN-1445.

Unmerged commits

09f52da... by Najam Ahmed Ansari <email address hidden> on 2023-07-06

Removing piston's custom HttpResponseWrapper

The wrapper class was attempting to work around a bug in Django which has since
been fixed, rendering the wrapper unnecessary. Removing the wrapper also fixes
SN-1445.

diff --git a/piston/__init__.py b/piston/__init__.py
index ffdf661..7be60e2 100644
--- a/piston/__init__.py
+++ b/piston/__init__.py
@@ -1,4 +1,4 @@
-__version__ = '2.2.6'
+__version__ = '2.2.7'
 
 
 def get_version():
diff --git a/piston/tests.py b/piston/tests.py
index aacce0b..f82460d 100644
--- a/piston/tests.py
+++ b/piston/tests.py
@@ -99,7 +99,7 @@ class CustomResponseWithStatusCodeTest(TestCase):
 
              def create(self, request):
                  resp = rc.CREATED
-                 resp.content = response_data
+                 resp.content = json.dumps(response_data)
                  return resp
 
          resource = Resource(MyHandler)
@@ -146,11 +146,11 @@ class ErrorHandlerTest(TestCase):
                 # formatted as json
                 if isinstance(error, GoAwayError):
                     response = rc.FORBIDDEN
-                    response.content = dict(error=dict(
+                    response.content = json.dumps(dict(error=dict(
                         name=error.name,
                         message="Get out of here and dont come back",
                         reason=error.reason
-                    ))
+                    )))
 
                     return response
 
@@ -187,6 +187,9 @@ class ErrorHandlerTest(TestCase):
 
         self.assertTrue(isinstance(response, HttpResponse), "Expected a response, not: %s"
             % response)
+        error_str = response.content.decode('utf-8')
+        self.assertTrue(error_str.find('Method signature does not match'),
+            'Expected underlying exception to be handled')
 
 
     def test_other_error(self):
diff --git a/piston/utils.py b/piston/utils.py
index 8e76718..a48c201 100644
--- a/piston/utils.py
+++ b/piston/utils.py
@@ -51,43 +51,7 @@ class rc_factory(object):
         except TypeError:
             raise AttributeError(attr)
 
-        class HttpResponseWrapper(HttpResponse):
-            """
-            Wrap HttpResponse and make sure that the internal
-            _is_string/_base_content_is_iter flag is updated when the
-            _set_content method (via the content property) is called
-            """
-            def _set_content(self, content):
-                """
-                Set the _container and _base_content_is_iter properties based
-                on the type of the value parameter. This logic is in the
-                construtor for HttpResponse, but doesn't get repeated when
-                setting HttpResponse.content although this bug report
-                (feature request) suggests that it should:
-                http://code.djangoproject.com/ticket/9403
-                """
-                if not isinstance(content, basestring) and hasattr(content, '__iter__'):
-                    self._container = content
-                    self._use_emitter = True
-                else:
-                    self._container = [content]
-                    self._use_emitter = False
-                # Django 1.7 and above does not have any custom internals to
-                # define the type of the content
-                if django.VERSION < (1, 7):
-                    self._base_content_is_iter = self._use_emitter
-
-            if django.VERSION >= (1, 5):
-                # HttpResponse._get_content does not exists in Django >= 1.5
-
-                @HttpResponse.content.setter
-                def content(self, content):
-                    self._set_content(content)
-
-            else:
-                content = property(HttpResponse._get_content, _set_content)
-
-        return HttpResponseWrapper(r, content_type='text/plain', status=c)
+        return HttpResponse(r, content_type='text/plain', status=c)
 
 rc = rc_factory()