From 35ad952c93b8cfe6e7bc441134e93668f96147c2 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 23 Mar 2015 13:14:51 +0100 Subject: [PATCH 1/8] Revert "deactivated json" This reverts commit 3eefb8293dc5cfca2b6bd4bfb7b043b5ff541acb. --- cliquet/__init__.py | 7 +++---- cliquet/utils.py | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/cliquet/__init__.py b/cliquet/__init__.py index b2fdff72..97d29d1a 100644 --- a/cliquet/__init__.py +++ b/cliquet/__init__.py @@ -16,6 +16,7 @@ from cornice import Service from pyramid.events import NewRequest, NewResponse from pyramid.httpexceptions import HTTPTemporaryRedirect, HTTPGone +from pyramid.renderers import JSON as JSONRenderer from pyramid.security import NO_PERMISSION_REQUIRED from pyramid_multiauth import MultiAuthenticationPolicy from pyramid.settings import asbool, aslist @@ -80,10 +81,8 @@ def monkey_patch_json(config): requests.models.json = utils.json # Override json renderer using ujson - # deactivated, see https://github.com/mozilla-services/cliquet/pull/132 - # from pyramid.renderers import JSON as JSONRenderer - # renderer = JSONRenderer(serializer=lambda v, **kw: utils.json.dumps(v)) - # config.add_renderer('json', renderer) + renderer = JSONRenderer(serializer=lambda v, **kw: utils.json.dumps(v)) + config.add_renderer('json', renderer) def load_default_settings(config): diff --git a/cliquet/utils.py b/cliquet/utils.py index b6997505..bd5e8f38 100644 --- a/cliquet/utils.py +++ b/cliquet/utils.py @@ -6,7 +6,7 @@ from binascii import hexlify # XXX see https://github.com/mozilla-services/cliquet/issues/131 -# import ujson as json +#import ujson as json import json from cornice import cors From 459ac7cb4a4b9ed154eb1b1a374faadb7c9596c5 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 23 Mar 2015 13:15:01 +0100 Subject: [PATCH 2/8] Revert "revert ujson usage for now" This reverts commit de2714a05f6415e1fd288c16e926742be1097349. --- cliquet/utils.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cliquet/utils.py b/cliquet/utils.py index bd5e8f38..20a7cd9d 100644 --- a/cliquet/utils.py +++ b/cliquet/utils.py @@ -5,10 +5,7 @@ from base64 import b64decode, b64encode from binascii import hexlify -# XXX see https://github.com/mozilla-services/cliquet/issues/131 -#import ujson as json -import json - +import ujson as json from cornice import cors from colander import null From 58cbb5d34215b5f8436579afaf5880d6d820645d Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 23 Mar 2015 15:14:13 +0100 Subject: [PATCH 3/8] Do not serialize using JSON if not necessary (fixes #131) --- cliquet/storage/postgresql/__init__.py | 3 ++- cliquet/tests/test_storage.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/cliquet/storage/postgresql/__init__.py b/cliquet/storage/postgresql/__init__.py index 7afc7985..b3e3c6f5 100644 --- a/cliquet/storage/postgresql/__init__.py +++ b/cliquet/storage/postgresql/__init__.py @@ -466,7 +466,8 @@ def _format_conditions(self, resource, filters, prefix='filters'): # If field is missing, we default to ''. sql_field = "coalesce(data->>%%(%s)s, '')" % field_holder # JSON-ify the native value (e.g. True -> 'true') - value = json.dumps(filtr.value).strip('"') + if not isinstance(filtr.value, six.string_types): + value = json.dumps(filtr.value).strip('"') # Safely escape value value_holder = '%s_value_%s' % (prefix, i) diff --git a/cliquet/tests/test_storage.py b/cliquet/tests/test_storage.py index 7dc119a4..72b38cb6 100644 --- a/cliquet/tests/test_storage.py +++ b/cliquet/tests/test_storage.py @@ -383,6 +383,22 @@ def test_updating_raises_unicity_error(self): record['id'], {'phone': 'number'}) + def test_unicity_detection_supports_special_characters(self): + record = self.create_record() + values = ['b', 'http://moz.org', u"#131 \u2014 ujson", "C:\\\\win32"] + for value in values: + self.create_record({'phone': value}) + try: + error = None + self.storage.update(self.resource, + self.user_id, + record['id'], + {'phone': value}) + except exceptions.UnicityError as e: + error = e + msg = 'UnicityError not raised with %s' % value + self.assertIsNotNone(error, msg) + class DeletedRecordsTest(object): def _get_last_modified_filters(self): From 47b6b138e4ce43f4b9a5057ec08bee277319e88c Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2015 10:20:20 +0100 Subject: [PATCH 4/8] Fix crash of classic logger with unicode --- cliquet/logs.py | 6 +++--- cliquet/tests/test_logging.py | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cliquet/logs.py b/cliquet/logs.py index 91671369..70919a69 100644 --- a/cliquet/logs.py +++ b/cliquet/logs.py @@ -84,10 +84,10 @@ def __init__(self, settings): def __call__(self, logger, name, event_dict): if 'path' in event_dict: - pattern = ('"{method: <5} {path}{querystring}" {code} ({t} ms)' - ' {event} {context}') + pattern = (u'"{method: <5} {path}{querystring}" {code} ({t} ms)' + ' {event} {context}') else: - pattern = '{event} {context}' + pattern = u'{event} {context}' output = {} for field in ['method', 'path', 'code', 't', 'event']: diff --git a/cliquet/tests/test_logging.py b/cliquet/tests/test_logging.py index 3975e15c..c8d0a35f 100644 --- a/cliquet/tests/test_logging.py +++ b/cliquet/tests/test_logging.py @@ -86,6 +86,10 @@ def test_every_event_dict_entry_appears_in_log_message(self): self.assertEqual(('"GET /v1/?_sort=field" 200 (32 ms)' ' app.event nb_records=5'), value) + def test_fields_values_support_unicode(self): + value = self.renderer(self.logger, 'critical', {'value': u'\u2014'}) + self.assertIn(u'\u2014', value) + class MozillaHekaRendererTest(unittest.TestCase): def setUp(self): From 337f2ff5fe45c3afc9ea41e23c8496437f9f19c4 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2015 11:14:23 +0100 Subject: [PATCH 5/8] Fix crash of CloudStorage backend when remote returns 500 --- cliquet/storage/cloud_storage.py | 8 +++++--- cliquet/tests/test_storage.py | 12 ++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/cliquet/storage/cloud_storage.py b/cliquet/storage/cloud_storage.py index 3d350f05..eeb33623 100644 --- a/cliquet/storage/cloud_storage.py +++ b/cliquet/storage/cloud_storage.py @@ -29,11 +29,13 @@ def wrapped(*args, **kwargs): try: return func(*args, **kwargs) except RequestException as e: + status_code = body = None if e.response is not None: status_code = e.response.status_code - body = e.response.json() - else: - status_code = body = None + try: + body = e.response.json() + except ValueError: + body = e.response.content if status_code == 404: record_id = '?' diff --git a/cliquet/tests/test_storage.py b/cliquet/tests/test_storage.py index 72b38cb6..8fef9025 100644 --- a/cliquet/tests/test_storage.py +++ b/cliquet/tests/test_storage.py @@ -857,3 +857,15 @@ def setUp(self): self.storage._client, 'request', side_effect=requests.ConnectionError) + + def test_raises_backenderror_when_remote_returns_500(self): + with mock.patch.object(self.storage._client, 'request') as mocked: + error_response = requests.models.Response() + error_response.status_code = 500 + error_response._content_consumed = True + error_response._content = 'Internal Error' + mocked.return_value = error_response + self.assertRaises(exceptions.BackendError, + self.storage.get_all, + self.resource, + self.user_id) From c07ba83960bf8a44950145b429ac7edffe72ad95 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2015 11:35:19 +0100 Subject: [PATCH 6/8] Fix behaviour of CloudStorage with backslashes in querystring --- cliquet/storage/cloud_storage.py | 22 ++++++++++++++++------ cliquet/tests/test_storage.py | 3 ++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/cliquet/storage/cloud_storage.py b/cliquet/storage/cloud_storage.py index eeb33623..1796a8b3 100644 --- a/cliquet/storage/cloud_storage.py +++ b/cliquet/storage/cloud_storage.py @@ -175,6 +175,18 @@ def delete_all(self, resource, user_id, filters=None): headers=self._build_headers(resource)) resp.raise_for_status() + def _filters_as_params(self, filters): + params = [] + for k, v, op in filters: + if isinstance(v, six.string_types): + # Literals '\' should be preserved in querystring + v = v.replace("\\", "\\\\") + params.append(("%s%s" % (FILTERS[op], k), v)) + return params + + def _params_as_querystring(self, params): + return '&'.join(["%s=%s" % (p, v) for p, v in params]) + @wrap_http_error def get_all(self, resource, user_id, filters=None, sorting=None, pagination_rules=None, limit=None, include_deleted=False): @@ -192,8 +204,7 @@ def get_all(self, resource, user_id, filters=None, sorting=None, params += [("_sort", ','.join(sort_fields))] if filters: - params += [("%s%s" % (FILTERS[op], k), v) - for k, v, op in filters] + params += self._filters_as_params(filters) if limit: params.append(("_limit", limit)) @@ -210,7 +221,7 @@ def get_all(self, resource, user_id, filters=None, sorting=None, else: batch_payload = {'defaults': {'body': {}}, 'requests': []} - querystring = '&'.join(['%s=%s' % (p, v) for p, v in params]) + querystring = self._params_as_querystring(params) batch_payload['requests'].append({ 'method': 'HEAD', 'path': url + '?%s' % querystring, @@ -218,9 +229,8 @@ def get_all(self, resource, user_id, filters=None, sorting=None, for filters in pagination_rules: params_ = list(params) - params_ += [("%s%s" % (FILTERS[op], k), v) - for k, v, op in filters] - querystring = '&'.join(['%s=%s' % (p, v) for p, v in params_]) + params_ += self._filters_as_params(filters) + querystring = self._params_as_querystring(params_) batch_payload['requests'].append({ 'path': url + '?%s' % querystring, }) diff --git a/cliquet/tests/test_storage.py b/cliquet/tests/test_storage.py index 8fef9025..b9b71371 100644 --- a/cliquet/tests/test_storage.py +++ b/cliquet/tests/test_storage.py @@ -385,7 +385,8 @@ def test_updating_raises_unicity_error(self): def test_unicity_detection_supports_special_characters(self): record = self.create_record() - values = ['b', 'http://moz.org', u"#131 \u2014 ujson", "C:\\\\win32"] + values = ['b', 'http://moz.org', u"#131 \u2014 ujson", + "C:\\\\win32\\hosts"] for value in values: self.create_record({'phone': value}) try: From f442c983500d3cdb0ca52abab861fd7ebdb220d2 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2015 11:36:29 +0100 Subject: [PATCH 7/8] Pep8 --- cliquet/logs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cliquet/logs.py b/cliquet/logs.py index 70919a69..cbde9877 100644 --- a/cliquet/logs.py +++ b/cliquet/logs.py @@ -85,7 +85,7 @@ def __init__(self, settings): def __call__(self, logger, name, event_dict): if 'path' in event_dict: pattern = (u'"{method: <5} {path}{querystring}" {code} ({t} ms)' - ' {event} {context}') + ' {event} {context}') else: pattern = u'{event} {context}' From c59a346a397078435c85a5d7b12fe27b6fa5f37c Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 24 Mar 2015 14:39:20 +0100 Subject: [PATCH 8/8] Fix python3.4 segmentation fault --- cliquet/tests/test_storage.py | 2 +- cliquet/tests/test_views_batch.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cliquet/tests/test_storage.py b/cliquet/tests/test_storage.py index b9b71371..6a2efa3f 100644 --- a/cliquet/tests/test_storage.py +++ b/cliquet/tests/test_storage.py @@ -864,7 +864,7 @@ def test_raises_backenderror_when_remote_returns_500(self): error_response = requests.models.Response() error_response.status_code = 500 error_response._content_consumed = True - error_response._content = 'Internal Error' + error_response._content = u'Internal Error'.encode('utf8') mocked.return_value = error_response self.assertRaises(exceptions.BackendError, self.storage.get_all, diff --git a/cliquet/tests/test_views_batch.py b/cliquet/tests/test_views_batch.py index 792a0f54..234cb3c1 100644 --- a/cliquet/tests/test_views_batch.py +++ b/cliquet/tests/test_views_batch.py @@ -283,7 +283,7 @@ def test_subrequests_body_have_utf8_charset(self): self.post({'requests': [request]}) subrequest, = self.request.invoke_subrequest.call_args[0] self.assertIn('charset=utf-8', subrequest.headers['Content-Type']) - wanted = {"json": u"\ud83d\ude02"} + wanted = {"json": u"😂"} self.assertEqual(subrequest.body.decode('utf8'), json.dumps(wanted))