[PATCH V3] [Wok 0/5] Fix issue #142
Changes in V3: Added patch 5. Other patches are the same already reviewed. Lucio Correia (5): Add option to get untranslated message text Issue #142 - Translate request log at reading-time Fix issue with converting message to unicode Update tests Fix ascii_dict and utf8_dict src/wok/control/base.py | 13 ++++++------ src/wok/message.py | 32 ++++++++++++++++------------- src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ src/wok/stringutils.py | 6 ++---- tests/test_api.py | 2 +- 6 files changed, 77 insertions(+), 41 deletions(-) -- 1.9.1
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/message.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/wok/message.py b/src/wok/message.py index 4c35747..35f40be 100644 --- a/src/wok/message.py +++ b/src/wok/message.py @@ -46,7 +46,7 @@ class WokMessage(object): self.args = args self.plugin = plugin - def _get_translation(self): + def _get_text(self, translate): wok_app = cherrypy.tree.apps.get('', None) # get app from plugin path if specified @@ -68,20 +68,23 @@ class WokMessage(object): app = wok_app text = app.root.messages.get(self.code, self.code) - # do translation - domain = app.root.domain - paths = app.root.paths - lang = validate_language(get_lang()) + if translate: + # do translation + domain = app.root.domain + paths = app.root.paths + lang = validate_language(get_lang()) - try: - translation = gettext.translation(domain, paths.mo_dir, [lang]) - except: - translation = gettext + try: + translation = gettext.translation(domain, paths.mo_dir, [lang]) + except: + translation = gettext + + return translation.gettext(text) - return translation.gettext(text) + return gettext.gettext(text) - def get_text(self, prepend_code=True): - msg = self._get_translation() + def get_text(self, prepend_code=True, translate=True): + msg = self._get_text(translate) msg = unicode(msg, 'utf-8') % self.args if prepend_code: -- 1.9.1
* Add info needed for translation to user request log entries (without unsafe parameters like passwwords) * Log message untranslated to base file (wok-req.log). * Keep compatibility to prior format of base log file, which does not have needed information to do translation. Old log entry format: [logrecord] >>> [translated message] New log entry format: [logrecord] >>> [message params] >>> [untranslated message] Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 13 ++++++------ src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/src/wok/control/base.py b/src/wok/control/base.py index 6dfc977..f563aed 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -31,7 +31,6 @@ from wok.control.utils import get_class_name, internal_redirect, model_fn from wok.control.utils import parse_request, validate_method from wok.control.utils import validate_params from wok.exception import InvalidOperation, UnauthorizedError, WokException -from wok.message import WokMessage from wok.reqlogger import RequestRecord from wok.stringutils import encode_value, utf8_dict from wok.utils import get_plugin_from_request, wok_log @@ -156,10 +155,10 @@ class Resource(object): # log request code = self.getRequestMessage(method, action_name) reqParams = utf8_dict(self.log_args, request) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -216,10 +215,10 @@ class Resource(object): # log request if method not in LOG_DISABLED_METHODS: code = self.getRequestMessage(method) - msg = WokMessage(code, self.log_args) RequestRecord( - msg.get_text(prepend_code=False), + self.log_args, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -455,10 +454,10 @@ class Collection(object): # log request code = self.getRequestMessage(method) reqParams = utf8_dict(self.log_args, params) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index fd02382..f82ae6b 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -30,6 +30,7 @@ from tempfile import NamedTemporaryFile from wok.config import config, get_log_download_path from wok.exception import InvalidParameter, OperationFailed +from wok.message import WokMessage from wok.stringutils import ascii_dict from wok.utils import remove_old_files @@ -55,6 +56,7 @@ SECONDS_PER_HOUR = 360 TS_DATE_FORMAT = "%Y-%m-%d" TS_TIME_FORMAT = "%H:%M:%S" TS_ZONE_FORMAT = "%Z" +UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd'] # Log handler setup REQUEST_LOG_FILE = "wok-req.log" @@ -112,6 +114,16 @@ class RequestParser(object): return LOG_DOWNLOAD_URI % os.path.basename(fd.name) + def getTranslatedMessage(self, record, params): + code = record.get('msgCode', '') + app = record.get('app', 'wok') + plugin = None + if app != 'wok': + plugin = "/plugins/%s" % app + + msg = WokMessage(code, params, plugin) + return msg.get_text(prepend_code=False, translate=True) + def getRecords(self): records = self.getRecordsFromFile(self.baseFile) @@ -140,7 +152,17 @@ class RequestParser(object): data = line.split(">>>") if len(data) > 1: record = json.JSONDecoder().decode(data[0]) - record['message'] = data[1].strip() + + if len(data) > 2: + # new log format: translate message on the fly + params = json.JSONDecoder().decode(data[1]) + msg = self.getTranslatedMessage(record, params) + else: + # make it compatible with v2.2 log files, which + # messages are already translated + msg = data[1].strip() + + record['message'] = msg records.append(record) line = f.readline() @@ -181,19 +203,32 @@ class RequestParser(object): class RequestRecord(object): - def __init__(self, message, **kwargs): - self.message = message - self.kwargs = kwargs + def __init__(self, msgParams, **kwargs): + # log record data + self.logData = kwargs + + # data for message translation + self.code = self.logData['msgCode'] + self.params = self.getSafeReqParams(msgParams) # register timestamp in local time timestamp = time.localtime() - self.kwargs['date'] = time.strftime(TS_DATE_FORMAT, timestamp) - self.kwargs['time'] = time.strftime(TS_TIME_FORMAT, timestamp) - self.kwargs['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp) + self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp) + self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + + def getSafeReqParams(self, params): + result = params.copy() + for param in UNSAFE_REQUEST_PARAMETERS: + result.pop(param, None) + return result def __str__(self): - info = json.JSONEncoder().encode(self.kwargs) - return '%s >>> %s' % (info, self.message) + msg = WokMessage(self.code, self.params) + msgText = msg.get_text(prepend_code=False, translate=False) + logData = json.JSONEncoder().encode(self.logData) + msgParams = json.JSONEncoder().encode(self.params) + return '%s >>> %s >>> %s' % (logData, msgParams, msgText) def log(self): reqLogger = logging.getLogger(WOK_REQUEST_LOGGER) diff --git a/src/wok/root.py b/src/wok/root.py index 8601b71..08f4981 100644 --- a/src/wok/root.py +++ b/src/wok/root.py @@ -32,7 +32,6 @@ from wok.control import sub_nodes from wok.control.base import Resource from wok.control.utils import parse_request from wok.exception import MissingParameter -from wok.message import WokMessage from wok.reqlogger import RequestRecord @@ -161,13 +160,13 @@ class WokRoot(Root): try: params = parse_request() - msg = WokMessage(code, params).get_text(prepend_code=False) username = params['username'] password = params['password'] except KeyError, item: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=400, user='N/A', @@ -185,8 +184,9 @@ class WokRoot(Root): raise finally: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=status, user='N/A', @@ -200,14 +200,14 @@ class WokRoot(Root): method = 'POST' code = self.getRequestMessage(method, 'logout') params = {'username': cherrypy.session.get(auth.USER_NAME, 'N/A')} - msg = WokMessage(code, params).get_text(prepend_code=False) ip = cherrypy.request.remote.ip auth.logout() RequestRecord( - msg, + params, app='wok', + msgCode=code, req=method, status=200, user=params['username'], -- 1.9.1
On 08/09/2016 11:42 AM, Lucio Correia wrote:
* Add info needed for translation to user request log entries (without unsafe parameters like passwwords) * Log message untranslated to base file (wok-req.log). * Keep compatibility to prior format of base log file, which does not have needed information to do translation.
Old log entry format: [logrecord] >>> [translated message]
New log entry format: [logrecord] >>> [message params] >>> [untranslated message]
Why don't include the message params to the JSON? Why do we need to have the untranslated message there? This log file is only used as input for the user log activity feature. It is not done to be read as debug or so. For debug, we have the official logs under /var/log/wok.
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 13 ++++++------ src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ 3 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/src/wok/control/base.py b/src/wok/control/base.py index 6dfc977..f563aed 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -31,7 +31,6 @@ from wok.control.utils import get_class_name, internal_redirect, model_fn from wok.control.utils import parse_request, validate_method from wok.control.utils import validate_params from wok.exception import InvalidOperation, UnauthorizedError, WokException -from wok.message import WokMessage from wok.reqlogger import RequestRecord from wok.stringutils import encode_value, utf8_dict from wok.utils import get_plugin_from_request, wok_log @@ -156,10 +155,10 @@ class Resource(object): # log request code = self.getRequestMessage(method, action_name) reqParams = utf8_dict(self.log_args, request) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -216,10 +215,10 @@ class Resource(object): # log request if method not in LOG_DISABLED_METHODS: code = self.getRequestMessage(method) - msg = WokMessage(code, self.log_args) RequestRecord( - msg.get_text(prepend_code=False), + self.log_args, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -455,10 +454,10 @@ class Collection(object): # log request code = self.getRequestMessage(method) reqParams = utf8_dict(self.log_args, params) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index fd02382..f82ae6b 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -30,6 +30,7 @@ from tempfile import NamedTemporaryFile
from wok.config import config, get_log_download_path from wok.exception import InvalidParameter, OperationFailed +from wok.message import WokMessage from wok.stringutils import ascii_dict from wok.utils import remove_old_files
@@ -55,6 +56,7 @@ SECONDS_PER_HOUR = 360 TS_DATE_FORMAT = "%Y-%m-%d" TS_TIME_FORMAT = "%H:%M:%S" TS_ZONE_FORMAT = "%Z" +UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd']
# Log handler setup REQUEST_LOG_FILE = "wok-req.log" @@ -112,6 +114,16 @@ class RequestParser(object):
return LOG_DOWNLOAD_URI % os.path.basename(fd.name)
+ def getTranslatedMessage(self, record, params): + code = record.get('msgCode', '') + app = record.get('app', 'wok') + plugin = None + if app != 'wok': + plugin = "/plugins/%s" % app +
You should not assume the plugin URI is something starting with '/plugins/' This is a plugin configuration and can be changed as the user wants.
+ msg = WokMessage(code, params, plugin) + return msg.get_text(prepend_code=False, translate=True) + def getRecords(self): records = self.getRecordsFromFile(self.baseFile)
@@ -140,7 +152,17 @@ class RequestParser(object): data = line.split(">>>") if len(data) > 1: record = json.JSONDecoder().decode(data[0]) - record['message'] = data[1].strip() + + if len(data) > 2: + # new log format: translate message on the fly + params = json.JSONDecoder().decode(data[1]) + msg = self.getTranslatedMessage(record, params) + else: + # make it compatible with v2.2 log files, which + # messages are already translated + msg = data[1].strip() + + record['message'] = msg records.append(record)
line = f.readline() @@ -181,19 +203,32 @@ class RequestParser(object):
class RequestRecord(object): - def __init__(self, message, **kwargs): - self.message = message - self.kwargs = kwargs + def __init__(self, msgParams, **kwargs): + # log record data + self.logData = kwargs + + # data for message translation + self.code = self.logData['msgCode'] + self.params = self.getSafeReqParams(msgParams)
# register timestamp in local time timestamp = time.localtime() - self.kwargs['date'] = time.strftime(TS_DATE_FORMAT, timestamp) - self.kwargs['time'] = time.strftime(TS_TIME_FORMAT, timestamp) - self.kwargs['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp) + self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp) + self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + + def getSafeReqParams(self, params): + result = params.copy() + for param in UNSAFE_REQUEST_PARAMETERS: + result.pop(param, None) + return result
def __str__(self): - info = json.JSONEncoder().encode(self.kwargs) - return '%s >>> %s' % (info, self.message) + msg = WokMessage(self.code, self.params) + msgText = msg.get_text(prepend_code=False, translate=False) + logData = json.JSONEncoder().encode(self.logData) + msgParams = json.JSONEncoder().encode(self.params) + return '%s >>> %s >>> %s' % (logData, msgParams, msgText)
def log(self): reqLogger = logging.getLogger(WOK_REQUEST_LOGGER) diff --git a/src/wok/root.py b/src/wok/root.py index 8601b71..08f4981 100644 --- a/src/wok/root.py +++ b/src/wok/root.py @@ -32,7 +32,6 @@ from wok.control import sub_nodes from wok.control.base import Resource from wok.control.utils import parse_request from wok.exception import MissingParameter -from wok.message import WokMessage from wok.reqlogger import RequestRecord
@@ -161,13 +160,13 @@ class WokRoot(Root):
try: params = parse_request() - msg = WokMessage(code, params).get_text(prepend_code=False) username = params['username'] password = params['password'] except KeyError, item: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=400, user='N/A', @@ -185,8 +184,9 @@ class WokRoot(Root): raise finally: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=status, user='N/A', @@ -200,14 +200,14 @@ class WokRoot(Root): method = 'POST' code = self.getRequestMessage(method, 'logout') params = {'username': cherrypy.session.get(auth.USER_NAME, 'N/A')} - msg = WokMessage(code, params).get_text(prepend_code=False) ip = cherrypy.request.remote.ip
auth.logout()
RequestRecord( - msg, + params, app='wok', + msgCode=code, req=method, status=200, user=params['username'],
On 22-08-2016 11:05, Aline Manera wrote:
On 08/09/2016 11:42 AM, Lucio Correia wrote:
* Add info needed for translation to user request log entries (without unsafe parameters like passwwords) * Log message untranslated to base file (wok-req.log). * Keep compatibility to prior format of base log file, which does not have needed information to do translation.
Old log entry format: [logrecord] >>> [translated message]
New log entry format: [logrecord] >>> [message params] >>> [untranslated message]
Why don't include the message params to the JSON?
That's because [logrecord] is the dict with all key=values passed to the request, and we should not mix the parameters to the error message there, because it can overwrite a request parameter (independently of key name I choose).
Why do we need to have the untranslated message there? This log file is only used as input for the user log activity feature. It is not done to be read as debug or so. For debug, we have the official logs under /var/log/wok.
We don't need it, but why not? Since the beginning, I've designed it to be also a log file, with useful info (JSON was chosen because of that). Remember that prior versions wok-req.log files contain the translated message there. Now that this has changed, I decided to keep its English version. Let me know if you want a patch to remove it. Thanks,
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 13 ++++++------ src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ 3 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/src/wok/control/base.py b/src/wok/control/base.py index 6dfc977..f563aed 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -31,7 +31,6 @@ from wok.control.utils import get_class_name, internal_redirect, model_fn from wok.control.utils import parse_request, validate_method from wok.control.utils import validate_params from wok.exception import InvalidOperation, UnauthorizedError, WokException -from wok.message import WokMessage from wok.reqlogger import RequestRecord from wok.stringutils import encode_value, utf8_dict from wok.utils import get_plugin_from_request, wok_log @@ -156,10 +155,10 @@ class Resource(object): # log request code = self.getRequestMessage(method, action_name) reqParams = utf8_dict(self.log_args, request) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -216,10 +215,10 @@ class Resource(object): # log request if method not in LOG_DISABLED_METHODS: code = self.getRequestMessage(method) - msg = WokMessage(code, self.log_args) RequestRecord( - msg.get_text(prepend_code=False), + self.log_args, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -455,10 +454,10 @@ class Collection(object): # log request code = self.getRequestMessage(method) reqParams = utf8_dict(self.log_args, params) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index fd02382..f82ae6b 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -30,6 +30,7 @@ from tempfile import NamedTemporaryFile
from wok.config import config, get_log_download_path from wok.exception import InvalidParameter, OperationFailed +from wok.message import WokMessage from wok.stringutils import ascii_dict from wok.utils import remove_old_files
@@ -55,6 +56,7 @@ SECONDS_PER_HOUR = 360 TS_DATE_FORMAT = "%Y-%m-%d" TS_TIME_FORMAT = "%H:%M:%S" TS_ZONE_FORMAT = "%Z" +UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd']
# Log handler setup REQUEST_LOG_FILE = "wok-req.log" @@ -112,6 +114,16 @@ class RequestParser(object):
return LOG_DOWNLOAD_URI % os.path.basename(fd.name)
+ def getTranslatedMessage(self, record, params): + code = record.get('msgCode', '') + app = record.get('app', 'wok') + plugin = None + if app != 'wok': + plugin = "/plugins/%s" % app +
You should not assume the plugin URI is something starting with '/plugins/' This is a plugin configuration and can be changed as the user wants.
OK, I will send a patch to fix this. Thanks for the good catch.
+ msg = WokMessage(code, params, plugin) + return msg.get_text(prepend_code=False, translate=True) + def getRecords(self): records = self.getRecordsFromFile(self.baseFile)
@@ -140,7 +152,17 @@ class RequestParser(object): data = line.split(">>>") if len(data) > 1: record = json.JSONDecoder().decode(data[0]) - record['message'] = data[1].strip() + + if len(data) > 2: + # new log format: translate message on the fly + params = json.JSONDecoder().decode(data[1]) + msg = self.getTranslatedMessage(record, params) + else: + # make it compatible with v2.2 log files, which + # messages are already translated + msg = data[1].strip() + + record['message'] = msg records.append(record)
line = f.readline() @@ -181,19 +203,32 @@ class RequestParser(object):
class RequestRecord(object): - def __init__(self, message, **kwargs): - self.message = message - self.kwargs = kwargs + def __init__(self, msgParams, **kwargs): + # log record data + self.logData = kwargs + + # data for message translation + self.code = self.logData['msgCode'] + self.params = self.getSafeReqParams(msgParams)
# register timestamp in local time timestamp = time.localtime() - self.kwargs['date'] = time.strftime(TS_DATE_FORMAT, timestamp) - self.kwargs['time'] = time.strftime(TS_TIME_FORMAT, timestamp) - self.kwargs['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp) + self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp) + self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + + def getSafeReqParams(self, params): + result = params.copy() + for param in UNSAFE_REQUEST_PARAMETERS: + result.pop(param, None) + return result
def __str__(self): - info = json.JSONEncoder().encode(self.kwargs) - return '%s >>> %s' % (info, self.message) + msg = WokMessage(self.code, self.params) + msgText = msg.get_text(prepend_code=False, translate=False) + logData = json.JSONEncoder().encode(self.logData) + msgParams = json.JSONEncoder().encode(self.params) + return '%s >>> %s >>> %s' % (logData, msgParams, msgText)
def log(self): reqLogger = logging.getLogger(WOK_REQUEST_LOGGER) diff --git a/src/wok/root.py b/src/wok/root.py index 8601b71..08f4981 100644 --- a/src/wok/root.py +++ b/src/wok/root.py @@ -32,7 +32,6 @@ from wok.control import sub_nodes from wok.control.base import Resource from wok.control.utils import parse_request from wok.exception import MissingParameter -from wok.message import WokMessage from wok.reqlogger import RequestRecord
@@ -161,13 +160,13 @@ class WokRoot(Root):
try: params = parse_request() - msg = WokMessage(code, params).get_text(prepend_code=False) username = params['username'] password = params['password'] except KeyError, item: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=400, user='N/A', @@ -185,8 +184,9 @@ class WokRoot(Root): raise finally: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=status, user='N/A', @@ -200,14 +200,14 @@ class WokRoot(Root): method = 'POST' code = self.getRequestMessage(method, 'logout') params = {'username': cherrypy.session.get(auth.USER_NAME, 'N/A')} - msg = WokMessage(code, params).get_text(prepend_code=False) ip = cherrypy.request.remote.ip
auth.logout()
RequestRecord( - msg, + params, app='wok', + msgCode=code, req=method, status=200, user=params['username'],
-- Lucio Correia Software Engineer IBM LTC Brazil
On 08/22/2016 02:56 PM, Lucio Correia wrote:
On 22-08-2016 11:05, Aline Manera wrote:
On 08/09/2016 11:42 AM, Lucio Correia wrote:
* Add info needed for translation to user request log entries (without unsafe parameters like passwwords) * Log message untranslated to base file (wok-req.log). * Keep compatibility to prior format of base log file, which does not have needed information to do translation.
Old log entry format: [logrecord] >>> [translated message]
New log entry format: [logrecord] >>> [message params] >>> [untranslated message]
Why don't include the message params to the JSON?
That's because [logrecord] is the dict with all key=values passed to the request, and we should not mix the parameters to the error message there, because it can overwrite a request parameter (independently of key name I choose).
I am not sure I got your point. Why can't we add the params to the JSON? For example: error: {code: XXX, params: {<msg params>}}
Why do we need to have the untranslated message there? This log file is only used as input for the user log activity feature. It is not done to be read as debug or so. For debug, we have the official logs under /var/log/wok.
We don't need it, but why not?
Since the beginning, I've designed it to be also a log file, with useful info (JSON was chosen because of that).
Remember that prior versions wok-req.log files contain the translated message there. Now that this has changed, I decided to keep its English version.
Let me know if you want a patch to remove it.
Yeap! I'd say to remove it and make it a pure JSON file. So we identify it as an input file for a feature.
Thanks,
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/control/base.py | 13 ++++++------ src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ 3 files changed, 56 insertions(+), 22 deletions(-)
diff --git a/src/wok/control/base.py b/src/wok/control/base.py index 6dfc977..f563aed 100644 --- a/src/wok/control/base.py +++ b/src/wok/control/base.py @@ -31,7 +31,6 @@ from wok.control.utils import get_class_name, internal_redirect, model_fn from wok.control.utils import parse_request, validate_method from wok.control.utils import validate_params from wok.exception import InvalidOperation, UnauthorizedError, WokException -from wok.message import WokMessage from wok.reqlogger import RequestRecord from wok.stringutils import encode_value, utf8_dict from wok.utils import get_plugin_from_request, wok_log @@ -156,10 +155,10 @@ class Resource(object): # log request code = self.getRequestMessage(method, action_name) reqParams = utf8_dict(self.log_args, request) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -216,10 +215,10 @@ class Resource(object): # log request if method not in LOG_DISABLED_METHODS: code = self.getRequestMessage(method) - msg = WokMessage(code, self.log_args) RequestRecord( - msg.get_text(prepend_code=False), + self.log_args, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), @@ -455,10 +454,10 @@ class Collection(object): # log request code = self.getRequestMessage(method) reqParams = utf8_dict(self.log_args, params) - msg = WokMessage(code, reqParams).get_text(prepend_code=False) RequestRecord( - msg, + reqParams, app=get_plugin_from_request(), + msgCode=code, req=method, status=status, user=cherrypy.session.get(USER_NAME, 'N/A'), diff --git a/src/wok/reqlogger.py b/src/wok/reqlogger.py index fd02382..f82ae6b 100644 --- a/src/wok/reqlogger.py +++ b/src/wok/reqlogger.py @@ -30,6 +30,7 @@ from tempfile import NamedTemporaryFile
from wok.config import config, get_log_download_path from wok.exception import InvalidParameter, OperationFailed +from wok.message import WokMessage from wok.stringutils import ascii_dict from wok.utils import remove_old_files
@@ -55,6 +56,7 @@ SECONDS_PER_HOUR = 360 TS_DATE_FORMAT = "%Y-%m-%d" TS_TIME_FORMAT = "%H:%M:%S" TS_ZONE_FORMAT = "%Z" +UNSAFE_REQUEST_PARAMETERS = ['password', 'passwd']
# Log handler setup REQUEST_LOG_FILE = "wok-req.log" @@ -112,6 +114,16 @@ class RequestParser(object):
return LOG_DOWNLOAD_URI % os.path.basename(fd.name)
+ def getTranslatedMessage(self, record, params): + code = record.get('msgCode', '') + app = record.get('app', 'wok') + plugin = None + if app != 'wok': + plugin = "/plugins/%s" % app +
You should not assume the plugin URI is something starting with '/plugins/' This is a plugin configuration and can be changed as the user wants.
OK, I will send a patch to fix this.
Thanks for the good catch.
+ msg = WokMessage(code, params, plugin) + return msg.get_text(prepend_code=False, translate=True) + def getRecords(self): records = self.getRecordsFromFile(self.baseFile)
@@ -140,7 +152,17 @@ class RequestParser(object): data = line.split(">>>") if len(data) > 1: record = json.JSONDecoder().decode(data[0]) - record['message'] = data[1].strip() + + if len(data) > 2: + # new log format: translate message on the fly + params = json.JSONDecoder().decode(data[1]) + msg = self.getTranslatedMessage(record, params) + else: + # make it compatible with v2.2 log files, which + # messages are already translated + msg = data[1].strip() + + record['message'] = msg records.append(record)
line = f.readline() @@ -181,19 +203,32 @@ class RequestParser(object):
class RequestRecord(object): - def __init__(self, message, **kwargs): - self.message = message - self.kwargs = kwargs + def __init__(self, msgParams, **kwargs): + # log record data + self.logData = kwargs + + # data for message translation + self.code = self.logData['msgCode'] + self.params = self.getSafeReqParams(msgParams)
# register timestamp in local time timestamp = time.localtime() - self.kwargs['date'] = time.strftime(TS_DATE_FORMAT, timestamp) - self.kwargs['time'] = time.strftime(TS_TIME_FORMAT, timestamp) - self.kwargs['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + self.logData['date'] = time.strftime(TS_DATE_FORMAT, timestamp) + self.logData['time'] = time.strftime(TS_TIME_FORMAT, timestamp) + self.logData['zone'] = time.strftime(TS_ZONE_FORMAT, timestamp) + + def getSafeReqParams(self, params): + result = params.copy() + for param in UNSAFE_REQUEST_PARAMETERS: + result.pop(param, None) + return result
def __str__(self): - info = json.JSONEncoder().encode(self.kwargs) - return '%s >>> %s' % (info, self.message) + msg = WokMessage(self.code, self.params) + msgText = msg.get_text(prepend_code=False, translate=False) + logData = json.JSONEncoder().encode(self.logData) + msgParams = json.JSONEncoder().encode(self.params) + return '%s >>> %s >>> %s' % (logData, msgParams, msgText)
def log(self): reqLogger = logging.getLogger(WOK_REQUEST_LOGGER) diff --git a/src/wok/root.py b/src/wok/root.py index 8601b71..08f4981 100644 --- a/src/wok/root.py +++ b/src/wok/root.py @@ -32,7 +32,6 @@ from wok.control import sub_nodes from wok.control.base import Resource from wok.control.utils import parse_request from wok.exception import MissingParameter -from wok.message import WokMessage from wok.reqlogger import RequestRecord
@@ -161,13 +160,13 @@ class WokRoot(Root):
try: params = parse_request() - msg = WokMessage(code, params).get_text(prepend_code=False) username = params['username'] password = params['password'] except KeyError, item: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=400, user='N/A', @@ -185,8 +184,9 @@ class WokRoot(Root): raise finally: RequestRecord( - msg, + params, app=app, + msgCode=code, req=method, status=status, user='N/A', @@ -200,14 +200,14 @@ class WokRoot(Root): method = 'POST' code = self.getRequestMessage(method, 'logout') params = {'username': cherrypy.session.get(auth.USER_NAME, 'N/A')} - msg = WokMessage(code, params).get_text(prepend_code=False) ip = cherrypy.request.remote.ip
auth.logout()
RequestRecord( - msg, + params, app='wok', + msgCode=code, req=method, status=200, user=params['username'],
On 22-08-2016 15:36, Aline Manera wrote:
On 08/22/2016 02:56 PM, Lucio Correia wrote:
On 22-08-2016 11:05, Aline Manera wrote:
On 08/09/2016 11:42 AM, Lucio Correia wrote:
* Add info needed for translation to user request log entries (without unsafe parameters like passwwords) * Log message untranslated to base file (wok-req.log). * Keep compatibility to prior format of base log file, which does not have needed information to do translation.
Old log entry format: [logrecord] >>> [translated message]
New log entry format: [logrecord] >>> [message params] >>> [untranslated message]
Why don't include the message params to the JSON?
That's because [logrecord] is the dict with all key=values passed to the request, and we should not mix the parameters to the error message there, because it can overwrite a request parameter (independently of key name I choose).
I am not sure I got your point.
Why can't we add the params to the JSON?
For example:
error: {code: XXX, params: {<msg params>}}
Because if the request has a parameter 'error' (even passed by mistake or malicious intention) it would be overwritten. We cannot guarantee that, that's why it's separated.
Why do we need to have the untranslated message there? This log file is only used as input for the user log activity feature. It is not done to be read as debug or so. For debug, we have the official logs under /var/log/wok.
We don't need it, but why not?
Since the beginning, I've designed it to be also a log file, with useful info (JSON was chosen because of that).
Remember that prior versions wok-req.log files contain the translated message there. Now that this has changed, I decided to keep its English version.
Let me know if you want a patch to remove it.
Yeap! I'd say to remove it and make it a pure JSON file. So we identify it as an input file for a feature.
It cannot be a pure JSON file due to problem above.
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/message.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wok/message.py b/src/wok/message.py index 35f40be..ff4cbc9 100644 --- a/src/wok/message.py +++ b/src/wok/message.py @@ -22,6 +22,7 @@ import cherrypy import gettext +from wok.stringutils import decode_value from wok.template import get_lang, validate_language @@ -36,7 +37,7 @@ class WokMessage(object): try: # In case the value formats itself to an ascii string. - args[key] = unicode(str(value), 'utf-8') + args[key] = decode_value(value) except UnicodeEncodeError: # In case the value is a WokException or it formats # itself to a unicode string. @@ -85,7 +86,7 @@ class WokMessage(object): def get_text(self, prepend_code=True, translate=True): msg = self._get_text(translate) - msg = unicode(msg, 'utf-8') % self.args + msg = decode_value(msg) % self.args if prepend_code: return "%s: %s" % (self.code, msg) -- 1.9.1
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index bcf34cb..c4ca755 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -84,7 +84,7 @@ class APITests(unittest.TestCase): self.assertGreaterEqual(records, 1) for record in records: keys = [u'zone', u'ip', u'app', u'req', u'user', u'time', u'date', - u'message', u'status'] + u'message', u'msgCode', u'status'] self.assertEquals(sorted(keys), sorted(record.keys())) # Test search by app -- 1.9.1
Signed-off-by: Lucio Correia <luciojhc@linux.vnet.ibm.com> --- src/wok/stringutils.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/wok/stringutils.py b/src/wok/stringutils.py index 8f0160b..b50222b 100644 --- a/src/wok/stringutils.py +++ b/src/wok/stringutils.py @@ -26,8 +26,7 @@ def ascii_dict(base, overlay=None): result.update(overlay or {}) for key, value in result.iteritems(): - if isinstance(value, unicode): - result[key] = str(value.decode('utf-8')) + result[key] = encode_value(value) return result @@ -37,8 +36,7 @@ def utf8_dict(base, overlay=None): result.update(overlay or {}) for key, value in result.iteritems(): - if isinstance(value, unicode): - result[key] = value.encode('utf-8') + result[key] = decode_value(value) return result -- 1.9.1
Reviewed-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> Tested-by: Daniel Barboza <danielhb@linux.vnet.ibm.com> On 08/09/2016 11:42 AM, Lucio Correia wrote:
Changes in V3:
Added patch 5. Other patches are the same already reviewed.
Lucio Correia (5): Add option to get untranslated message text Issue #142 - Translate request log at reading-time Fix issue with converting message to unicode Update tests Fix ascii_dict and utf8_dict
src/wok/control/base.py | 13 ++++++------ src/wok/message.py | 32 ++++++++++++++++------------- src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ src/wok/stringutils.py | 6 ++---- tests/test_api.py | 2 +- 6 files changed, 77 insertions(+), 41 deletions(-)
Applied to master. Thanks! On 08/09/2016 11:42 AM, Lucio Correia wrote:
Changes in V3:
Added patch 5. Other patches are the same already reviewed.
Lucio Correia (5): Add option to get untranslated message text Issue #142 - Translate request log at reading-time Fix issue with converting message to unicode Update tests Fix ascii_dict and utf8_dict
src/wok/control/base.py | 13 ++++++------ src/wok/message.py | 32 ++++++++++++++++------------- src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ src/wok/stringutils.py | 6 ++---- tests/test_api.py | 2 +- 6 files changed, 77 insertions(+), 41 deletions(-)
Hi Lucio, I will reopen the issue according to my comments so we can discuss more around that. Regards, Aline Manera On 08/09/2016 11:42 AM, Lucio Correia wrote:
Changes in V3:
Added patch 5. Other patches are the same already reviewed.
Lucio Correia (5): Add option to get untranslated message text Issue #142 - Translate request log at reading-time Fix issue with converting message to unicode Update tests Fix ascii_dict and utf8_dict
src/wok/control/base.py | 13 ++++++------ src/wok/message.py | 32 ++++++++++++++++------------- src/wok/reqlogger.py | 53 ++++++++++++++++++++++++++++++++++++++++--------- src/wok/root.py | 12 +++++------ src/wok/stringutils.py | 6 ++---- tests/test_api.py | 2 +- 6 files changed, 77 insertions(+), 41 deletions(-)
participants (3)
-
Aline Manera -
Daniel Henrique Barboza -
Lucio Correia