Commit 34cb27f2 authored by Julien Muchembled's avatar Julien Muchembled Committed by Arnaud Fontaine

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's avatarKirill Smelkov <kirr@nexedi.com>
Reviewed-by: Kirill Smelkov's avatarKirill Smelkov <kirr@nexedi.com>
parent 59c140fb
...@@ -1562,12 +1562,16 @@ class ERP5Site(FolderMixIn, CMFSite, CacheCookieMixin): ...@@ -1562,12 +1562,16 @@ class ERP5Site(FolderMixIn, CMFSite, CacheCookieMixin):
""" """
return () return ()
def log(self, description, content='', level=INFO): def log(self, *args, **kw):
"""Put a log message """ """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" warnings.warn("The usage of ERP5Site.log is deprecated.\n"
"Please use Products.ERP5Type.Log.log instead.", "Please use Products.ERP5Type.Log.log instead.",
DeprecationWarning) DeprecationWarning)
unrestrictedLog(description, content = content, level = level) unrestrictedLog(*args, **kw)
security.declarePublic('setPlacelessDefaultReindexParameters') security.declarePublic('setPlacelessDefaultReindexParameters')
def setPlacelessDefaultReindexParameters(self, **kw): def setPlacelessDefaultReindexParameters(self, **kw):
......
...@@ -3060,12 +3060,16 @@ class Base( CopyContainer, ...@@ -3060,12 +3060,16 @@ class Base( CopyContainer,
return Error(**kw) return Error(**kw)
security.declarePublic('log') security.declarePublic('log')
def log(self, description, content='', level=INFO): def log(self, *args, **kw):
"""Put a log message """ """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" warnings.warn("The usage of Base.log is deprecated.\n"
"Please use Products.ERP5Type.Log.log instead.", "Please use Products.ERP5Type.Log.log instead.",
DeprecationWarning) DeprecationWarning)
unrestrictedLog(description, content = content, level = level) unrestrictedLog(*args, **kw)
# Dublin Core Emulation for CMF interoperatibility # Dublin Core Emulation for CMF interoperatibility
# CMF Dublin Core Compatibility # CMF Dublin Core Compatibility
......
...@@ -43,8 +43,17 @@ from traceback import extract_stack ...@@ -43,8 +43,17 @@ from traceback import extract_stack
marker_ = [] marker_ = []
def log(description, content=marker_, level=INFO): def log(description, content=marker_, level=INFO):
"""Put a log message. This method is supposed to be used by """Put a log message
restricted environment, such as Script (Python)."""
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 if content is marker_: # allow for content only while keeping interface
description, content = content, description description, content = content, description
st = extract_stack() st = extract_stack()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment