From 571316fcf43bd310d4e45ed865b8c2a374e95900 Mon Sep 17 00:00:00 2001 From: Michael Bideau Date: Tue, 23 Jul 2019 15:51:29 +0200 Subject: [PATCH] JSON Schema input validation enabled (again), utf-8 decode error fixed, and more Feature: * JSON Schema input validation enabled (again) Fixes: * utf-8 decode error fixed: added unicode to litterals * removed useless and maybe problematic 'str()' forced casting * remove useless and maybe problematic 'join()' on list of file ids Refactoring: * property 'extra_debug_enabled' removed (not used since some commits) * in function 'normalize()' only cast to 'unicode' if value is not already 'unicode' --- atreal_openads/models.py | 114 +++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 58 deletions(-) diff --git a/atreal_openads/models.py b/atreal_openads/models.py index d113e52..d782703 100644 --- a/atreal_openads/models.py +++ b/atreal_openads/models.py @@ -83,7 +83,9 @@ def normalize(value): """Normalize a value to be send to openADS.API.""" if value is None: return '' - return clean_spaces(unicode(value)) + if not isinstance(value, unicode): + value = unicode(value) + return clean_spaces(value) def get_file_data(path, b64=True): @@ -113,6 +115,7 @@ def dict_deref_multi(data, keys): key = int(keys[0]) if isinstance(data, list) else keys[0] return dict_deref_multi(data[key], keys[1:]) if keys else data + # TODO implement wildcard filtering def dict_replace_content_by_paths(dic, paths_to_replace=[], path_separator='/', replace_by=''): """Replace a dict content, with a string, from a list of paths.""" @@ -131,7 +134,7 @@ def dict_replace_content_by_paths(dic, paths_to_replace=[], path_separator='/', except TypeError: continue if not isinstance(value, str) and not isinstance(value, unicode): - raise ValueError("path '%s' is not a string ([%s] %s)" % (p, str(type(value)), str(value))) + raise ValueError(u"path '%s' is not a string ([%s] %s)" % (p, type(value), value)) # newdic is still untouched if newdic is dic: # create a copy of it @@ -203,25 +206,19 @@ class AtrealOpenads(BaseResource, HTTPResource): verbose_name = _('openADS') - @property - def extra_debug_enabled(self): - """Return True if 'extra debug' is enabled.""" - return bool(self.extra_debug) - - def log_json_payload(self, payload, title='payload', paths_to_replace=[]): """Log a json paylod surrounded by dashes and with file content filtered.""" - self.logger.debug("----- %s (begining) -----", title) + self.logger.debug(u"----- %s (begining) -----", title) if paths_to_replace: - self.logger.debug("%s", LogJsonPayloadWithFileContent( + self.logger.debug(u"%s", LogJsonPayloadWithFileContent( payload, paths_to_replace = paths_to_replace, - path_separator = '.', - replace_by = '' + path_separator = u'.', + replace_by = u'' )) else: - self.logger.debug("%s", LogJsonPayloadWithFileContent(payload)) - self.logger.debug("----- %s (end) -----", title) + self.logger.debug(u"%s", LogJsonPayloadWithFileContent(payload)) + self.logger.debug(u"----- %s (end) -----", title) def get_files_from_json_payload(self, payload, title='payload'): @@ -230,7 +227,7 @@ class AtrealOpenads(BaseResource, HTTPResource): # check the 'files' key if 'files' not in payload: self.log_json_payload(payload, title) - raise APIError("Expecting '%s' key in JSON %s" % + raise APIError(u"Expecting '%s' key in JSON %s" % ('files', title)) files = payload.get('files') @@ -238,12 +235,12 @@ class AtrealOpenads(BaseResource, HTTPResource): if not isinstance(files, list): self.log_json_payload(payload, title) raise APIError( - "Expecting '%s' value in JSON %s to be a %s (not a %s)" % - ('files', title, 'list', str(type(files)))) + u"Expecting '%s' value in JSON %s to be a %s (not a %s)" % + ('files', title, 'list', type(files))) if len(files) <= 0: self.log_json_payload(payload, title) - raise APIError("Expecting non-empty '%s' value in JSON %s" % + raise APIError(u"Expecting non-empty '%s' value in JSON %s" % ('files', title)) # log the response @@ -267,15 +264,15 @@ class AtrealOpenads(BaseResource, HTTPResource): # check content existence if content_key not in dict_file: - raise APIError("Expecting 'files.0.%s' key in JSON %s" % (content_key, title)) + raise APIError(u"Expecting 'files.0.%s' key in JSON %s" % (content_key, title)) # get its content file_content = dict_file[content_key] if not isinstance(file_content, str) and not isinstance(file_content, unicode): raise APIError( - "Expecting '%s' value in JSON %s to be a %s (not a %s)" % - ('file.%s' % content_key, title, 'string', str(type(file_content)))) + u"Expecting '%s' value in JSON %s to be a %s (not a %s)" % + ('file.%s' % content_key, title, 'string', type(file_content))) # check filename if ( @@ -284,8 +281,8 @@ class AtrealOpenads(BaseResource, HTTPResource): and not isinstance(dict_file['filename'], unicode) ): raise APIError( - "Expecting '%s' value in file dict to be a %s (not a %s)" % - ('file.filename', title, 'string', str(type(dict_file['filename'])))) + u"Expecting '%s' value in file dict to be a %s (not a %s)" % + ('file.filename', title, 'string', type(dict_file['filename']))) def get_first_file_from_json_payload(self, payload, title='payload', ensure_content=True, b64=True): @@ -331,18 +328,18 @@ class AtrealOpenads(BaseResource, HTTPResource): parameters={ 'type_dossier': {'description': _("Type of 'dossier'"), 'example_value': 'DIA'} }, -#~ post={'description': _("Create an openADS 'dossier'"), -#~ 'request_body': { -#~ 'schema': { -#~ 'application/json': JSON_SCHEMA_CREATE_DOSSIER_IN -#~ } -#~ }, + post={'description': _("Create an openADS 'dossier'"), + 'request_body': { + 'schema': { + 'application/json': JSON_SCHEMA_CREATE_DOSSIER_IN + } + }, #~ 'response_body': { #~ 'schema': { #~ 'application/json': JSON_SCHEMA_CREATE_DOSSIER_OUT #~ } #~ } -#~ } + } ) def create_dossier(self, request, type_dossier, *args, **kwargs): @@ -432,7 +429,7 @@ class AtrealOpenads(BaseResource, HTTPResource): # add it to the payload payload[key] = [requerant] - self.logger.debug("Added '%s' to payload: %s %s", key, requerant['prenom'], requerant['nom']) + self.logger.debug(u"Added '%s' to payload: %s %s", key, requerant['prenom'], requerant['nom']) # log the payload self.log_json_payload(payload) @@ -482,14 +479,14 @@ class AtrealOpenads(BaseResource, HTTPResource): # response is an error code if response.status_code // 100 != 2: error = self.get_response_error(response) - self.logger.warning("Request [POST] '%s' failed with error: '%s'", url, error) + self.logger.warning(u"Request [POST] '%s' failed with error: '%s'", url, error) raise APIError(error) # load the response JSON content try: result = response.json() except ValueError: - raise APIError('No JSON content returned: %r' % response.content[:1000]) + raise APIError(u'No JSON content returned: %r' % response.content[:1000]) # get the recepisse recepisse = self.get_first_file_from_json_payload(result, title='response') @@ -501,7 +498,7 @@ class AtrealOpenads(BaseResource, HTTPResource): and recepisse['content_type'] != 'application/pdf' ): self.logger.debug( - "Forcing 'recepisse' content type to '%s' instead of '%s'.", + u"Forcing 'recepisse' content type to '%s' instead of '%s'.", 'application/pdf', recepisse['content_type'] ) @@ -522,10 +519,11 @@ class AtrealOpenads(BaseResource, HTTPResource): if not isinstance(numero_dossier, str) and not isinstance(numero_dossier, unicode): raise APIError( - "Expecting '%s' value in JSON response to be a %s (not a %s)" % - ('numero_dossier', 'string', str(type(numero_dossier)))) + u"Expecting '%s' value in JSON response to be a %s (not a %s)" % + ('numero_dossier', 'string', type(numero_dossier))) - self.logger.debug("Numéro dossier: %s", str(numero_dossier)) + numero_dossier = normalize(numero_dossier) + self.logger.debug(u"Numéro dossier: %s", numero_dossier) # save files to be forwarded to openADS.API if files: @@ -541,7 +539,7 @@ class AtrealOpenads(BaseResource, HTTPResource): FF.upload_status = 'pending' FF.save() self.logger.debug( - "Created ForwardFile '%s' for file '%s' (%s)", + u"Created ForwardFile '%s' for file '%s' (%s)", FF.id, FF.orig_filename, FF.upload_file.path @@ -554,11 +552,11 @@ class AtrealOpenads(BaseResource, HTTPResource): numero_dossier=numero_dossier, file_ids=file_ids) self.logger.debug( - "Added a job '%s' for dossier '%s' (%s) with file ids '%s'", + u"Added a job '%s' for dossier '%s' (%s) with file ids '%s'", job.id, numero_dossier, type_dossier, - ','.join([str(fid) for fid in file_ids]) + file_ids ) # respond with the 'numero_dossier' and the recepisse file @@ -594,14 +592,14 @@ class AtrealOpenads(BaseResource, HTTPResource): # response is an error if response.status_code // 100 != 2: error = self.get_response_error(response) - self.logger.warning("Request [GET] '%s' failed with error: '%s'", url, error) + self.logger.warning(u"Request [GET] '%s' failed with error: '%s'", url, error) raise APIError(error) # load the response as JSON try: result = response.json() except ValueError: - raise APIError('No JSON content returned: %r' % response.content[:1000]) + raise APIError(u'No JSON content returned: %r' % response.content[:1000]) # log the response self.log_json_payload(result, 'response') @@ -660,7 +658,7 @@ class AtrealOpenads(BaseResource, HTTPResource): try: fwd_files = [ForwardFile.objects.get(id=fichier_id)] except ForwardFile.DoesNotExist: - raise Http404("No file matches 'numero_dossier=%s' and 'id=%s'." % (numero_dossier, fichier_id)) + raise Http404(u"No file matches 'numero_dossier=%s' and 'id=%s'." % (numero_dossier, fichier_id)) # append each file to the response payload for fwd_file in fwd_files: @@ -714,7 +712,7 @@ class AtrealOpenads(BaseResource, HTTPResource): # build a summary of all files statuses for fwd_file in fwd_files: - status_msg = '[%s] %s => %s' % ( + status_msg = u'[%s] %s => %s' % ( fwd_file['id'], fwd_file['orig_filename'], fwd_file['upload_msg'] @@ -755,14 +753,14 @@ class AtrealOpenads(BaseResource, HTTPResource): # response is an error if response.status_code // 100 != 2: error = self.get_response_error(response) - self.logger.warning("Request [GET] '%s' failed with error: '%s'", url, error) + self.logger.warning(u"Request [GET] '%s' failed with error: '%s'", url, error) raise APIError(error) # load the response as JSON try: result = response.json() except ValueError: - raise APIError('No JSON content returned: %r' % response.content[:1000]) + raise APIError(u'No JSON content returned: %r' % response.content[:1000]) # log the response (filtering the file content) self.log_json_payload( @@ -796,20 +794,20 @@ class AtrealOpenads(BaseResource, HTTPResource): location = error.get('location') name = error.get('name') desc = error.get('description') - msg.append('[%s] (%s) %s' % (location, normalize(name), normalize(desc))) + msg.append(u'[%s] (%s) %s' % (location, normalize(name), normalize(desc))) # if there are messages if msg: # return a string representing the HTTP error - return "HTTP error: %s, %s" % (response.status_code, ','.join(msg)) + return u"HTTP error: %s, %s" % (response.status_code, ','.join(msg)) except ValueError: pass # TODO ask for openADS.API to *always* send JSON formatted errors, not HTML ones # return a string representing the HTTP error (filtering the HTML tags and multispaces) - return "HTTP error: %s, %s" % \ + return u"HTTP error: %s, %s" % \ (response.status_code, clean_spaces(strip_tags(response.content[:1000])) if response.content else '') @@ -822,7 +820,7 @@ class AtrealOpenads(BaseResource, HTTPResource): # for every file ids specified (in parameters of this job) for fid in file_ids: - self.logger.debug("upload_user_files() ForwardFile file_id: %s", fid) + self.logger.debug(u"upload_user_files() ForwardFile file_id: %s", fid) # get the matching forward file fwd_file = ForwardFile.objects.get(id=fid) @@ -844,7 +842,7 @@ class AtrealOpenads(BaseResource, HTTPResource): fwd_file.upload_status = 'uploading' fwd_file.upload_attempt += 1 fwd_file.upload_msg = 'attempt %s' % fwd_file.upload_attempt - self.logger.debug("upload_user_files() upload_msg: '%s'", fwd_file.upload_msg) + self.logger.debug(u"upload_user_files() upload_msg: '%s'", fwd_file.upload_msg) fwd_file.save() self.logger.debug("upload_user_files() ForwardFile saved") @@ -853,7 +851,7 @@ class AtrealOpenads(BaseResource, HTTPResource): # not found the forwarded file specified: bug else: - self.logger.warning("upload_user_files() failed to find ForwardFile file_id: %s", fid) + self.logger.warning(u"upload_user_files() failed to find ForwardFile file_id: %s", fid) # if files need to be forwarded if payload: @@ -884,10 +882,10 @@ class AtrealOpenads(BaseResource, HTTPResource): # log (warning) the error message self.logger.warning( - "upload_user_files() openADS response is not OK (code: %s) for dossier '%s' and files '%s'", + u"upload_user_files() openADS response is not OK (code: %s) for dossier '%s' and files '%s'", response.status_code, numero_dossier, - ','.join(file_ids) + file_ids ) # response is not an error @@ -903,12 +901,12 @@ class AtrealOpenads(BaseResource, HTTPResource): # update every files status as 'failed' and save the error message for fwd_file in fwd_files: fwd_file.upload_status = 'failed' - fwd_file.upload_msg = 'No JSON content returned: %r' % response.content[:1000] + fwd_file.upload_msg = u'No JSON content returned: %r' % response.content[:1000] fwd_file.save() # log (warning) the error message self.logger.warning( - "upload_user_files() openADS response is not JSON valid for dossier '%s' and files '%s'", + u"upload_user_files() openADS response is not JSON valid for dossier '%s' and files '%s'", numero_dossier, ','.join([f.id for f in fwd_files]) ) @@ -932,7 +930,7 @@ class AtrealOpenads(BaseResource, HTTPResource): # log the success message self.logger.debug( - "upload_user_files() flaging file '%s' has transfered (deleted '%s')", + u"upload_user_files() flaging file '%s' has transfered (deleted '%s')", fwd_file.id, fpath ) @@ -940,7 +938,7 @@ class AtrealOpenads(BaseResource, HTTPResource): # no file need to be forwarded else: self.logger.warning( - "upload_user_files() payload is empty for dossier '%s' and files '%s'", + u"upload_user_files() payload is empty for dossier '%s' and files '%s'", numero_dossier, ','.join(file_ids) )