From 34cb27f2e61964b5ff2b3d26606d0c4392125f9c Mon Sep 17 00:00:00 2001 From: Julien Muchembled <jm@nexedi.com> Date: Wed, 7 Oct 2015 17:51:27 +0200 Subject: [PATCH] Fix memory leak and DoS in ERP5Site.log() and Base.log() ERP5Site.log and Base.log are wrappers to the 'log' function from Product.ERP5Type.Log, but parameters were forwarded in a wrong way when called with a single argument: self.log(message) # Base method This was equivalent to: log(message, '') # function from Product.ERP5Type.Log And the whole message was later part of subsystem in: logger = logging.getLogger(subsystem) But because loggers are never freed, it is important that 'subsystem' does not vary too often, to avoid a memory leak. The fix is to simply forwarding parameters with catchall arguments, instead of duplicating the signature from Product.ERP5Type.Log. Of course, it remains important to call these methods correctly, otherwise memory leaks can happen again. For this reason, catchall arguments also prevents ERP5Site.log and Base.log to be called by ZPublisher. Reported-by: Kirill Smelkov <kirr@nexedi.com> Reviewed-by: Kirill Smelkov <kirr@nexedi.com> --- product/ERP5/ERP5Site.py | 10 +++++++--- product/ERP5Type/Base.py | 10 +++++++--- product/ERP5Type/Log.py | 13 +++++++++++-- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/product/ERP5/ERP5Site.py b/product/ERP5/ERP5Site.py index ed70bf8f0b..f56b662337 100644 --- a/product/ERP5/ERP5Site.py +++ b/product/ERP5/ERP5Site.py @@ -1562,12 +1562,16 @@ class ERP5Site(FolderMixIn, CMFSite, CacheCookieMixin): """ return () - def log(self, description, content='', level=INFO): - """Put a log message """ + def log(self, *args, **kw): + """Put a log message + + See the warning in Products.ERP5Type.Log.log + Catchall parameters also make this method not publishable to avoid DoS. + """ warnings.warn("The usage of ERP5Site.log is deprecated.\n" "Please use Products.ERP5Type.Log.log instead.", DeprecationWarning) - unrestrictedLog(description, content = content, level = level) + unrestrictedLog(*args, **kw) security.declarePublic('setPlacelessDefaultReindexParameters') def setPlacelessDefaultReindexParameters(self, **kw): diff --git a/product/ERP5Type/Base.py b/product/ERP5Type/Base.py index 0242c52188..af102eab8c 100644 --- a/product/ERP5Type/Base.py +++ b/product/ERP5Type/Base.py @@ -3060,12 +3060,16 @@ class Base( CopyContainer, return Error(**kw) security.declarePublic('log') - def log(self, description, content='', level=INFO): - """Put a log message """ + def log(self, *args, **kw): + """Put a log message + + See the warning in Products.ERP5Type.Log.log + Catchall parameters also make this method not publishable to avoid DoS. + """ warnings.warn("The usage of Base.log is deprecated.\n" "Please use Products.ERP5Type.Log.log instead.", DeprecationWarning) - unrestrictedLog(description, content = content, level = level) + unrestrictedLog(*args, **kw) # Dublin Core Emulation for CMF interoperatibility # CMF Dublin Core Compatibility diff --git a/product/ERP5Type/Log.py b/product/ERP5Type/Log.py index 261d2cc396..332294a315 100644 --- a/product/ERP5Type/Log.py +++ b/product/ERP5Type/Log.py @@ -43,8 +43,17 @@ from traceback import extract_stack marker_ = [] def log(description, content=marker_, level=INFO): - """Put a log message. This method is supposed to be used by - restricted environment, such as Script (Python).""" + """Put a log message + + This method is supposed to be used by restricted environment, + such as Script (Python). + + WARNING: When called with more than 1 argument, the first one is appended + to the usual information about the caller, in order to form a + subsystem string. Because a logging.Logger object is created for + each subsystem, and is never freed, you can experience memory + leaks if description is not constant. + """ if content is marker_: # allow for content only while keeping interface description, content = content, description st = extract_stack() -- 2.30.9