Skip to content

Commit

Permalink
Resolve encoding-related errors
Browse files Browse the repository at this point in the history
…and prefer explicit encoding/decoding over `smart_str()`. See #718
  • Loading branch information
jnm committed May 25, 2021
1 parent 46b6650 commit 77ab828
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 30 deletions.
3 changes: 1 addition & 2 deletions onadata/apps/api/tests/viewsets/test_briefcase_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

from django.urls import reverse
from django.core.files.storage import get_storage_class
from django.utils.encoding import smart_str
from django_digest.test import DigestAuth
from rest_framework.test import APIRequestFactory

Expand Down Expand Up @@ -224,7 +223,7 @@ def test_view_download_submission(self):
text = text.replace('{{xform_uuid}}',
self.xform.uuid)
self.assertContains(response, instance_id, status_code=200)
self.assertMultiLineEqual(smart_str(response.content), text)
self.assertMultiLineEqual(response.content.decode('utf-8'), text)

def test_view_download_submission_other_user(self):
view = BriefcaseApi.as_view({'get': 'retrieve'})
Expand Down
2 changes: 1 addition & 1 deletion onadata/apps/api/tests/viewsets/test_data_viewset.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
def enketo_mock(url, request):
response = requests.Response()
response.status_code = 201
response._content = '{"url": "https://hmh2a.enketo.formhub.org"}'
response._content = b'{"url": "https://hmh2a.enketo.formhub.org"}'
return response


Expand Down
19 changes: 9 additions & 10 deletions onadata/apps/api/tests/viewsets/test_xform_list_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

from django.conf import settings
from django_digest.test import DigestAuth
from django.utils.encoding import smart_str
from guardian.shortcuts import assign_perm

from onadata.apps.api.tests.viewsets.test_abstract_viewset import\
Expand Down Expand Up @@ -40,7 +39,7 @@ def test_get_xform_list(self):
form_list_xml = f.read().strip()
data = {"hash": self.xform.hash, "pk": self.xform.pk}
content = response.render().content
self.assertEqual(smart_str(content), form_list_xml % data)
self.assertEqual(content.decode('utf-8'), form_list_xml % data)
self.assertTrue(response.has_header('X-OpenRosa-Version'))
self.assertTrue(
response.has_header('X-OpenRosa-Accept-Content-Length'))
Expand All @@ -61,7 +60,7 @@ def test_get_xform_list_inactive_form(self):

xml = '<?xml version="1.0" encoding="utf-8"?>\n<xforms '
xml += 'xmlns="http://openrosa.org/xforms/xformsList"></xforms>'
content = smart_str(response.render().content)
content = response.render().content.decode('utf-8')
self.assertEqual(content, xml)
self.assertTrue(response.has_header('X-OpenRosa-Version'))
self.assertTrue(
Expand All @@ -85,7 +84,7 @@ def test_get_xform_list_anonymous_user(self):
form_list_xml = f.read().strip()
data = {"hash": self.xform.hash, "pk": self.xform.pk}
content = response.render().content
self.assertEqual(smart_str(content), form_list_xml % data)
self.assertEqual(content.decode('utf-8'), form_list_xml % data)
self.assertTrue(response.has_header('X-OpenRosa-Version'))
self.assertTrue(
response.has_header('X-OpenRosa-Accept-Content-Length'))
Expand Down Expand Up @@ -121,7 +120,7 @@ def test_get_xform_list_other_user_with_no_role(self):
request.META.update(auth(request.META, response))
response = self.view(request)
self.assertEqual(response.status_code, 200)
content = smart_str(response.render().content)
content = response.render().content.decode('utf-8')
self.assertNotIn(self.xform.id_string, content)
self.assertEqual(
content, '<?xml version="1.0" encoding="utf-8"?>\n<xforms '
Expand Down Expand Up @@ -152,7 +151,7 @@ def test_get_xform_list_other_user_with_readonly_role(self):
request.META.update(auth(request.META, response))
response = self.view(request)
self.assertEqual(response.status_code, 200)
content = smart_str(response.render().content)
content = response.render().content.decode('utf-8')
self.assertNotIn(self.xform.id_string, content)
self.assertEqual(
content, '<?xml version="1.0" encoding="utf-8"?>\n<xforms '
Expand Down Expand Up @@ -192,7 +191,7 @@ def test_get_xform_list_other_user_with_dataentry_role(self):
form_list_xml = f.read().strip()
data = {"hash": self.xform.hash, "pk": self.xform.pk}
content = response.render().content
self.assertEqual(smart_str(content), form_list_xml % data)
self.assertEqual(content.decode('utf-8'), form_list_xml % data)
self.assertTrue(response.has_header('X-OpenRosa-Version'))
self.assertTrue(
response.has_header('X-OpenRosa-Accept-Content-Length'))
Expand Down Expand Up @@ -250,7 +249,7 @@ def test_retrieve_xform_xml(self):
with open(path) as f:
form_xml = f.read().strip()
data = {"form_uuid": self.xform.uuid}
content = smart_str(response.render().content).strip()
content = response.render().content.decode('utf-8').strip()
self.assertEqual(content, form_xml % data)

def _load_metadata(self, xform=None):
Expand Down Expand Up @@ -282,7 +281,7 @@ def test_retrieve_xform_manifest(self):
<manifest xmlns="http://openrosa.org/xforms/xformsManifest"><mediaFile><filename>screenshot.png</filename><hash>%(hash)s</hash><downloadUrl>http://testserver/bob/xformsMedia/%(xform)s/%(pk)s.png</downloadUrl></mediaFile></manifest>""" # noqa
data = {"hash": self.metadata.hash, "pk": self.metadata.pk,
"xform": self.xform.pk}
content = smart_str(response.render().content).strip()
content = response.render().content.decode('utf-8').strip()
self.assertEqual(content, manifest_xml % data)
self.assertTrue(response.has_header('X-OpenRosa-Version'))
self.assertTrue(
Expand All @@ -306,7 +305,7 @@ def test_retrieve_xform_manifest_anonymous_user(self):
<manifest xmlns="http://openrosa.org/xforms/xformsManifest"><mediaFile><filename>screenshot.png</filename><hash>%(hash)s</hash><downloadUrl>http://testserver/bob/xformsMedia/%(xform)s/%(pk)s.png</downloadUrl></mediaFile></manifest>""" # noqa
data = {"hash": self.metadata.hash, "pk": self.metadata.pk,
"xform": self.xform.pk}
content = smart_str(response.render().content).strip()
content = response.render().content.decode('utf-8').strip()
self.assertEqual(content, manifest_xml % data)
self.assertTrue(response.has_header('X-OpenRosa-Version'))
self.assertTrue(
Expand Down
12 changes: 7 additions & 5 deletions onadata/apps/api/tests/viewsets/test_xform_submission_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import simplejson as json
from django.contrib.auth.models import AnonymousUser
from django.core.files.uploadedfile import InMemoryUploadedFile
from django.utils.encoding import smart_str
from django_digest.test import DigestAuth
from guardian.shortcuts import assign_perm

Expand Down Expand Up @@ -163,8 +162,8 @@ def test_post_submission_authenticated_bad_json(self):
'..',
'fixtures',
'transport_submission_bad.json')
with open(path, 'rb') as f:
data = json.loads(smart_str(f.read()))
with open(path) as f:
data = json.loads(f.read())
request = self.factory.post('/submission', data, format='json')
response = self.view(request)
self.assertEqual(response.status_code, 401)
Expand All @@ -175,8 +174,11 @@ def test_post_submission_authenticated_bad_json(self):
response = self.view(request)
rendered_response = response.render()
self.assertTrue('error' in rendered_response.data)
self.assertTrue(smart_str(rendered_response.data['error']).
startswith("b'Received empty submission"))
self.assertTrue(
rendered_response.data['error'].startswith(
'Received empty submission'
)
)
self.assertTrue(rendered_response.status_code == 400)
self.assertTrue(rendered_response.has_header('X-OpenRosa-Version'))
self.assertTrue(
Expand Down
7 changes: 4 additions & 3 deletions onadata/apps/api/viewsets/xform_submission_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from django.conf import settings
from django.contrib.auth.models import User
from django.shortcuts import get_object_or_404
from django.utils.encoding import smart_str
from django.utils.translation import ugettext as _

from rest_framework import permissions
Expand Down Expand Up @@ -211,9 +210,11 @@ def error_response(self, error, is_json_request, request):
elif not is_json_request:
return error
else:
error_msg = xml_error_re.search(smart_str(error.content)).groups()[0]
error_msg = xml_error_re.search(
error.content.decode('utf-8')
).groups()[0]
status_code = error.status_code

return Response({'error': smart_str(error_msg)},
return Response({'error': error_msg},
headers=self.get_openrosa_headers(request),
status=status_code)
2 changes: 1 addition & 1 deletion onadata/apps/logger/xform_instance_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def get_deprecated_uuid_from_xml(xml):
def clean_and_parse_xml(xml_string):
clean_xml_str = xml_string.strip()
clean_xml_str = re.sub(r">\s+<", "><", smart_str(clean_xml_str))
xml_obj = minidom.parseString(smart_str(clean_xml_str))
xml_obj = minidom.parseString(clean_xml_str)
return xml_obj


Expand Down
12 changes: 8 additions & 4 deletions onadata/libs/utils/logger_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -558,10 +558,14 @@ class OpenRosaResponse(BaseOpenRosaResponse):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
# wrap content around xml
self.content = '''<?xml version='1.0' encoding='UTF-8' ?>
<OpenRosaResponse xmlns="http://openrosa.org/http/response">
<message nature="">%s</message>
</OpenRosaResponse>''' % self.content
self.content = (
b"<?xml version='1.0' encoding='UTF-8' ?>\n"
b'<OpenRosaResponse xmlns="http://openrosa.org/http/response">\n'
b' <message nature="">'
) + self.content + (
b'</message>\n'
b'</OpenRosaResponse>'
)


class OpenRosaResponseNotFound(OpenRosaResponse):
Expand Down
6 changes: 2 additions & 4 deletions onadata/libs/utils/viewer_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from django.core.files.storage import get_storage_class
from django.core.files.uploadedfile import InMemoryUploadedFile
from django.core.mail import mail_admins
from django.utils.encoding import smart_str
from django.utils.translation import ugettext as _

from onadata.libs.utils import common_tags
Expand Down Expand Up @@ -204,8 +203,7 @@ def enketo_url(form_url, id_string, instance_xml=None,

if req.status_code in [200, 201]:
try:
# ToDo find out why req.json() does not work
response = json.loads(smart_str(req.content))
response = req.json()
except ValueError:
pass
else:
Expand All @@ -217,7 +215,7 @@ def enketo_url(form_url, id_string, instance_xml=None,
return response['url']
else:
try:
response = json.loads(smart_str(req.content))
response = req.json()
except ValueError:
pass
else:
Expand Down

0 comments on commit 77ab828

Please sign in to comment.