Re: [Kimchi-devel] [PATCH] [Wok 2/5] Issue #142 - Translate request log at reading-time

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(a)linux.vnet.ibm.com&gt; >> --- >> 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'], >