merged haufe-legacy-integration branch

doc/CHANGES.rst

@@ -14,9 +14,32 @@ Features Added
 - zExceptions.convertExceptionType:  new API, breaking out conversion of
   exception names to exception types from 'upgradeException'.
 
+- Launchpad #375322: the <environment> section within the zope.conf
+  file is now a multisection in order to provide a more modular configuration
+  support.
+
+- Launchpad #374719: introducing new ZPublisher events:
+  PubStart, PubSuccess, PubFailure, PubAfterTraversal and PubBeforeCommit.
+  
 Bugs Fixed
 ++++++++++
 
+- Launchpad #374729: Encoding cookie values to avoid issues with
+  firewalls and security proxies.
+
+- Launchpad #373583: ZODBMountPoint - fixed broken mount support and 
+  extended the test suite.
+
+- Launchpad #373621: catching and logging exceptions that could cause
+  leaking of worker threads.
+
+- Launchpad #373577: setting up standard logging earlier within the startup
+  phase for improving the analysis of startup errors.
+
+- Launchpad #373601: abort transaction before connection close in order to
+  prevent connection leaks in case of persistent changes after the main
+  transaction is closed.
+
 - Fix BBB regression which prevented setting browser ID cookies from
   browser ID managers created before the 'HTTPOnly' feature landed.
   https://bugs.launchpad.net/bugs/374816
@@ -50,11 +73,13 @@ Restructuring
 
 Features Added
 ++++++++++++++
+- zExceptions.convertExceptionType:  new API, breaking out conversion of
+  exception names to exception types from 'upgradeException'.
 
 - Extended BrowserIdManager to expose the 'HTTPOnly' attribute for its
   cookie. Also via https://bugs.launchpad.net/zope2/+bug/367393 .
 
-- Addeed support for an optional 'HTTPOnly' attribute of cookies (see
+- Added support for an optional 'HTTPOnly' attribute of cookies (see
   http://www.owasp.org/index.php/HTTPOnly).  Patch from Stephan Hofmockel,
   via https://bugs.launchpad.net/zope2/+bug/367393 .
 

src/App/ZApplication.py

@@ -80,4 +80,5 @@ class Cleanup:
         self._jar = jar
 
     def __del__(self):
+        transaction.abort()
         self._jar.close()

src/Products/ZODBMountPoint/MountedObject.py

@@ -59,9 +59,7 @@ class SimpleTrailblazer:
         factory = guarded_getattr(dispatcher, 'manage_addFolder')
         factory(id)
         o = context.restrictedTraverse(id)
-        # Commit a subtransaction to assign the new object to
-        # the correct database.
-        transaction.savepoint(optimistic=True)
+        context._p_jar.add(o.aq_base)
         return o
 
     def traverseOrConstruct(self, path, omit_final=0):

src/Products/ZODBMountPoint/tests/testMountPoint.py

@@ -83,6 +83,7 @@ class MountingTests(unittest.TestCase):
         databases = [TestDBConfig('test_main.fs', ['/']).getDB(),
                      TestDBConfig('test_mount1.fs', ['/mount1']).getDB(),
                      TestDBConfig('test_mount2.fs', ['/mount2']).getDB(),
+                     TestDBConfig('test_mount3.fs', ['/i/mount3']).getDB(),
                      ]
         mount_points = {}
         mount_factories = {}
@@ -102,13 +103,21 @@ class MountingTests(unittest.TestCase):
         root = conn.root()
         root['Application'] = app = Application()
         self.app = app
+        install_products(app, 'ZCatalog', 'PluginIndexes', 'OFSP')
+        # login
+        from AccessControl.User import system
+        from AccessControl.SecurityManagement import newSecurityManager
+        newSecurityManager(None, system)
         transaction.commit()  # Get app._p_jar set
-        manage_addMounts(app, ('/mount1', '/mount2'))
+        manage_addMounts(app, ('/mount1', '/mount2', '/i/mount3'))
         transaction.commit()  # Get the mount points ready
 
 
 
     def tearDown(self):
+        # logout
+        from AccessControl.SecurityManagement import noSecurityManager
+        noSecurityManager()
         App.config.setConfiguration(original_config)
         transaction.abort()
         self.app._p_jar.close()
@@ -120,6 +129,7 @@ class MountingTests(unittest.TestCase):
     def testRead(self):
         self.assertEqual(self.app.mount1.id, 'mount1')
         self.assertEqual(self.app.mount2.id, 'mount2')
+        self.assertEqual(self.app.i.mount3.id, 'mount3')
 
     def testWrite(self):
         app = self.app
@@ -144,6 +154,7 @@ class MountingTests(unittest.TestCase):
         self.assertEqual(getMountPoint(self.app.mount1)._path, '/mount1')
         self.assert_(getMountPoint(self.app.mount2) is not None)
         self.assertEqual(getMountPoint(self.app.mount2)._path, '/mount2')
+        self.assertEqual(getMountPoint(self.app.i.mount3)._path, '/i/mount3')
         del self.app.mount2
         self.app.mount2 = Folder()
         self.app.mount2.id = 'mount2'
@@ -160,8 +171,13 @@ class MountingTests(unittest.TestCase):
                     {'status': 'Ok',
                      'path': '/mount2',
                      'name': 'test_mount2.fs',
-                     'exists': 1}]
-        self.assertEqual(expected, status)
+                     'exists': 1},
+                    {'status': 'Ok',
+                     'path': '/i/mount3',
+                     'name': 'test_mount3.fs',
+                     'exists': 1},
+                    ]
+        self.assertEqual(sorted(expected), sorted(status))
         del self.app.mount2
         status = manage_getMountStatus(self.app)
         expected = [{'status': 'Ok',
@@ -171,8 +187,14 @@ class MountingTests(unittest.TestCase):
                     {'status': 'Ready to create',
                      'path': '/mount2',
                      'name': 'test_mount2.fs',
-                     'exists': 0}]
-        self.assertEqual(expected, status)
+                     'exists': 0},
+                    {'status': 'Ok',
+                     'path': '/i/mount3',
+                     'name': 'test_mount3.fs',
+                     'exists': 1},
+
+                    ]
+        self.assertEqual(sorted(expected), sorted(status))
         self.app.mount2 = Folder('mount2')
         status = manage_getMountStatus(self.app)
         expected = [{'status': 'Ok',
@@ -182,8 +204,13 @@ class MountingTests(unittest.TestCase):
                     {'status': '** Something is in the way **',
                      'path': '/mount2',
                      'name': 'test_mount2.fs',
-                     'exists': 1}]
-        self.assertEqual(expected, status)
+                     'exists': 1},
+                    {'status': 'Ok',
+                     'path': '/i/mount3',
+                     'name': 'test_mount3.fs',
+                     'exists': 1},
+                    ]
+        self.assertEqual(sorted(expected), sorted(status))
 
     def test_close(self):
         app = self.app
@@ -192,6 +219,7 @@ class MountingTests(unittest.TestCase):
         app.a3 = '3'
         conn1 = app.mount1._p_jar
         conn2 = app.mount2._p_jar
+        conn3 = app.i.mount3._p_jar
         transaction.abort()
         # Close the main connection
         app._p_jar.close()
@@ -199,6 +227,22 @@ class MountingTests(unittest.TestCase):
         # Check that secondary connections have been closed too
         self.assertEqual(conn1.opened, None)
         self.assertEqual(conn2.opened, None)
+        self.assertEqual(conn3.opened, None)
+
+def install_products(app, *prod):
+    """auxiliary function to install products *prod* (by names)."""
+    from OFS.Application import get_folder_permissions, get_products, install_product
+
+    folder_permissions = get_folder_permissions()
+    meta_types=[]
+    done={}
+    products = get_products()
+    for priority, product_name, index, product_dir in products:
+        if product_name not in prod or product_name in done: continue
+        done[product_name]=1
+        install_product(app, product_dir, product_name, meta_types,
+                        folder_permissions, raise_exc=True)
+        
 
 
 def test_suite():

src/ZPublisher/HTTPRequest.py

@@ -1684,7 +1684,7 @@ def parse_cookie(text,
         release()
 
     if not already_have(name):
-        result[name] = value
+        result[name] = unquote(value)
 
     return apply(parse_cookie,(text[l:],result))
 

src/ZPublisher/HTTPResponse.py

@@ -23,6 +23,7 @@ from zExceptions import Unauthorized, Redirect
 from zExceptions.ExceptionFormatter import format_exception
 from ZPublisher import BadRequest, InternalError, NotFound
 from cgi import escape
+from urllib import quote
 
 nl2sp = maketrans('\n',' ')
 
@@ -842,7 +843,7 @@ class HTTPResponse(BaseResponse):
             # quoted cookie attr values, so only the value part
             # of name=value pairs may be quoted.
 
-            cookie = 'Set-Cookie: %s="%s"' % (name, attrs['value'])
+            cookie = 'Set-Cookie: %s="%s"' % (name, quote(attrs['value']))
             for name, v in attrs.items():
                 name = name.lower()
                 if name == 'expires':

src/ZPublisher/Publish.py

@@ -24,6 +24,10 @@ from zExceptions import Redirect
 from zope.publisher.interfaces import ISkinnable
 from zope.publisher.skinnable import setDefaultSkin
 from zope.security.management import newInteraction, endInteraction
+from zope.event import notify
+
+from pubevents import PubStart, PubSuccess, PubFailure, \
+     PubBeforeCommit, PubAfterTraversal
 
 class Retry(Exception):
     """Raise this to retry a request
@@ -76,6 +80,7 @@ def publish(request, module_name, after_list, debug=0,
     response=None
 
     try:
+        notify(PubStart(request))
         # TODO pass request here once BaseRequest implements IParticipation
         newInteraction()
 
@@ -110,6 +115,8 @@ def publish(request, module_name, after_list, debug=0,
 
         object=request.traverse(path, validated_hook=validated_hook)
 
+        notify(PubAfterTraversal(request))
+
         if transactions_manager:
             transactions_manager.recordMetaData(object, request)
 
@@ -122,12 +129,18 @@ def publish(request, module_name, after_list, debug=0,
         if result is not response:
             response.setBody(result)
 
+        notify(PubBeforeCommit(request))
+
         if transactions_manager:
             transactions_manager.commit()
         endInteraction()
 
+        notify(PubSuccess(request))
+
         return response
     except:
+        # save in order to give 'PubFailure' the original exception info
+        exc_info = sys.exc_info()
         # DM: provide nicer error message for FTP
         sm = None
         if response is not None:
@@ -141,6 +154,7 @@ def publish(request, module_name, after_list, debug=0,
                 debug_mode and compact_traceback()[-1] or ''))
 
         if err_hook is not None:
+            retry = False
             if parents:
                 parents=parents[0]
             try:
@@ -157,10 +171,15 @@ def publish(request, module_name, after_list, debug=0,
                                         sys.exc_info()[1],
                                         sys.exc_info()[2],
                                         )
+                    retry = True
             finally:
-                if transactions_manager:
-                    transactions_manager.abort()
-                endInteraction()
+                # Note: 'abort's can fail. Nevertheless, we want end request handling
+                try: 
+                    if transactions_manager:
+                        transactions_manager.abort()
+                finally:
+                    endInteraction()
+                    notify(PubFailure(request, exc_info, retry))
 
             # Only reachable if Retry is raised and request supports retry.
             newrequest=request.retry()
@@ -175,9 +194,13 @@ def publish(request, module_name, after_list, debug=0,
                 newrequest.close()
 
         else:
-            if transactions_manager:
-                transactions_manager.abort()
-            endInteraction()
+            # Note: 'abort's can fail. Nevertheless, we want end request handling
+            try:
+                if transactions_manager:
+                    transactions_manager.abort()
+            finally:
+                endInteraction()
+                notify(PubFailure(request, exc_info, False))
             raise
 
 

src/ZPublisher/interfaces.py

+from zope.interface import Interface, Attribute
+
+#############################################################################
+# Publication events
+#  These are events notified in 'ZPublisher.Publish.publish'.
+
+class IPubEvent(Interface):
+    '''Base class for publication events.
+
+    Publication events are notified in 'ZPublisher.Publish.publish' to
+    inform about publications (aka requests) and their fate.
+    '''
+    request = Attribute('The request being affected')
+
+class IPubStart(IPubEvent):
+    '''Event notified at the beginning of 'ZPublisher.Publish.publish'.'''
+
+class IPubEnd(IPubEvent):
+    '''Event notified after request processing.
+
+    Note that a retried request ends before the retrieal, the retrial
+    itself is considered a new event.
+    '''
+
+class IPubSuccess(IPubEnd):
+    '''A successful request processing.'''
+
+class IPubFailure(IPubEnd):
+    '''A failed request processing.
+
+    Note: If a subscriber to 'IPubSuccess' raises an exception,
+    then 'IPubFailure' may be notified in addtion to 'IPubSuccess'.
+    '''
+    exc_info = Attribute('''The exception info as returned by 'sys.exc_info()'.''')
+    retry = Attribute('Whether the request will be retried')
+
+
+class IPubAfterTraversal(IPubEvent):
+    """notified after traversal and an (optional) authentication."""
+
+
+class IPubBeforeCommit(IPubEvent):
+    """notified immediately before the transaction commit (i.e. after the main
+    request processing is finished.
+    """

src/ZPublisher/pubevents.py

+'''Publication events.
+
+They are notified in 'ZPublisher.Publish.publish' and 
+inform about publications and their fate.
+
+Subscriptions can be used for all kinds of request supervision,
+e.g. request and error rate determination, writing high resolution logfiles
+for detailed time related analysis, inline request monitoring.
+'''
+from zope.interface import implements
+
+from interfaces import IPubStart, IPubSuccess, IPubFailure, \
+     IPubAfterTraversal, IPubBeforeCommit
+
+class _Base(object):
+    """PubEvent base class."""
+
+    def __init__(self, request):
+        self.request = request
+
+class PubStart(_Base):
+    '''notified at the beginning of 'ZPublisher.Publish.publish'.'''
+    implements(IPubStart)
+
+class PubSuccess(_Base):
+    '''notified at successful request end.'''
+    implements(IPubSuccess)
+
+class PubFailure(object):
+    '''notified at failed request end.'''
+    implements(IPubFailure)
+
+    def __init__(self, request, exc_info, retry):
+        self.request, self.exc_info, self.retry = request, exc_info, retry
+
+
+class PubAfterTraversal(_Base):
+    """notified after traversal and an (optional) authentication."""
+    implements(IPubAfterTraversal)
+
+
+class PubBeforeCommit(_Base):
+    """notified immediately before the commit."""
+    implements(IPubBeforeCommit)

src/ZPublisher/tests/testpubevents.py

+from sys import modules, exc_info
+from unittest import TestCase, TestSuite, makeSuite, main
+
+from ZODB.POSException import ConflictError
+from zope.interface.verify import verifyObject
+from zope.event import subscribers
+
+from ZPublisher.Publish import publish, Retry
+from ZPublisher.BaseRequest import BaseRequest
+from ZPublisher.pubevents import PubStart, PubSuccess, PubFailure, \
+     PubAfterTraversal, PubBeforeCommit
+from ZPublisher.interfaces import \
+     IPubStart, IPubEnd, IPubSuccess, IPubFailure, \
+     IPubAfterTraversal, IPubBeforeCommit
+
+PUBMODULE = 'TEST_testpubevents'
+
+_g=globals()
+
+class TestInterface(TestCase):
+    def testPubStart(self):
+        verifyObject(IPubStart, PubStart(_Request()))
+
+    def testPubSuccess(self):
+        e = PubSuccess(_Request())
+        verifyObject(IPubSuccess, e)
+        verifyObject(IPubEnd, e)
+
+    def testPubFailure(self):
+        # get some exc info
+        try: raise ValueError()
+        except: exc = exc_info()
+        e = PubFailure(_Request(), exc, False)
+        verifyObject(IPubFailure, e)
+        verifyObject(IPubEnd, e)
+
+    def testAfterTraversal(self):
+        e = PubAfterTraversal(_Request())
+        verifyObject(IPubAfterTraversal, e)
+
+    def testBeforeCommit(self):
+        e = PubBeforeCommit(_Request())
+        verifyObject(IPubBeforeCommit, e)
+
+        
+class TestPubEvents(TestCase):
+    def setUp(self):
+        self._saved_subscribers = subscribers[:]
+        self.reporter = r = _Reporter()
+        subscribers[:] = [r]
+        modules[PUBMODULE] = __import__(__name__, _g, _g, ('__doc__', ))
+        self.request = _Request()
+
+    def tearDown(self):
+        if PUBMODULE in modules: del modules[PUBMODULE]
+        subscribers[:] = self._saved_subscribers
+
+    def testSuccess(self):
+        r = self.request; r.action = 'succeed'
+        publish(r, PUBMODULE, [None])
+        events = self.reporter.events
+        self.assertEqual(len(events), 4)
+        self.assert_(isinstance(events[0], PubStart))
+        self.assertEqual(events[0].request, r)
+        self.assert_(isinstance(events[-1], PubSuccess))
+        self.assertEqual(events[-1].request, r)
+        # test AfterTraversal and BeforeCommit as well
+        self.assert_(isinstance(events[1], PubAfterTraversal))
+        self.assertEqual(events[1].request, r)
+        self.assert_(isinstance(events[2], PubBeforeCommit))
+        self.assertEqual(events[2].request, r)
+
+    def testFailureReturn(self):
+        r = self.request; r.action = 'fail_return'
+        publish(r, PUBMODULE, [None])
+        events = self.reporter.events
+        self.assertEqual(len(events), 2)
+        self.assert_(isinstance(events[0], PubStart))
+        self.assertEqual(events[0].request, r)
+        self.assert_(isinstance(events[1], PubFailure))
+        self.assertEqual(events[1].request, r)
+        self.assertEqual(events[1].retry, False)
+        self.assertEqual(len(events[1].exc_info), 3)
+
+    def testFailureException(self):
+        r = self.request; r.action = 'fail_exception'
+        self.assertRaises(Exception, publish, r, PUBMODULE, [None])
+        events = self.reporter.events
+        self.assertEqual(len(events), 2)
+        self.assert_(isinstance(events[0], PubStart))
+        self.assertEqual(events[0].request, r)
+        self.assert_(isinstance(events[1], PubFailure))
+        self.assertEqual(events[1].request, r)
+        self.assertEqual(events[1].retry, False)
+        self.assertEqual(len(events[1].exc_info), 3)
+
+    def testFailureConflict(self):
+        r = self.request; r.action = 'conflict'
+        publish(r, PUBMODULE, [None])
+        events = self.reporter.events
+        self.assertEqual(len(events), 6)
+        self.assert_(isinstance(events[0], PubStart))
+        self.assertEqual(events[0].request, r)
+        self.assert_(isinstance(events[1], PubFailure))
+        self.assertEqual(events[1].request, r)
+        self.assertEqual(events[1].retry, True)
+        self.assertEqual(len(events[1].exc_info), 3)
+        self.assert_(isinstance(events[1].exc_info[1], ConflictError))
+        self.assert_(isinstance(events[2], PubStart))
+        self.assert_(isinstance(events[5], PubSuccess))
+
+# Auxiliaries
+def _succeed():
+    ''' '''
+    return 'success'
+
+class _Application(object): pass
+
+class _Reporter(object):
+    def __init__(self): self.events = []
+    def __call__(self, event): self.events.append(event)
+
+class _Response(object):
+    def setBody(*unused): pass
+
+
+class _Request(BaseRequest):
+    response = _Response()
+    _hacked_path = False
+    args = ()
+
+    def __init__(self, *args, **kw):
+        BaseRequest.__init__(self, *args, **kw)
+        self['PATH_INFO'] = self['URL'] = ''
+        self.steps = []
+
+    def supports_retry(self): return True
+    def retry(self):
+        r = self.__class__()
+        r.action = 'succeed'
+        return r
+
+    def traverse(self, *unused, **unused_kw):
+        action = self.action
+        if action.startswith('fail'): raise Exception(action)
+        if action == 'conflict': raise ConflictError()
+        if action == 'succeed': return _succeed
+        else: raise ValueError('unknown action: %s' % action)
+
+    # override to get rid of the 'EndRequestEvent' notification
+    def close(self): pass
+    
+# define things necessary for publication
+bobo_application = _Application()
+def zpublisher_exception_hook(parent, request, *unused):
+    action = request.action
+    if action == 'fail_return': return 0
+    if action == 'fail_exception': raise Exception()
+    if action == 'conflict': raise Retry()
+    raise ValueError('unknown action: %s' % action)
+
+def test_suite():
+    return TestSuite((makeSuite(c) for c in (TestPubEvents, TestInterface)))
+
+        
+

src/ZServer/PubCore/ZServerPublisher.py

@@ -11,11 +11,17 @@
 #
 ##############################################################################
 
+import logging
+
+LOG = logging.getLogger('ZServerPublisher')
+
 class ZServerPublisher:
     def __init__(self, accept):
+        from sys import exc_info
         from ZPublisher import publish_module
         from ZPublisher.WSGIPublisher import publish_module as publish_wsgi
         while 1:
+          try:
             name, a, b=accept()
             if name == "Zope2":
                 try:
@@ -36,3 +42,5 @@ class ZServerPublisher:
                     # TODO: Support keeping connections open.
                     a['wsgi.output']._close = 1
                     a['wsgi.output'].close()
+          except:
+            LOG.error('exception caught', exc_info=True)

src/Zope2/Startup/__init__.py

@@ -86,6 +86,7 @@ class ZopeStarter:
         self.setupServers()
         # drop privileges after setting up servers
         self.dropPrivileges()
+        self.setupFinalLogging()
         self.makeLockFile()
         self.makePidFile()
         self.setupInterpreter()
@@ -100,7 +101,6 @@ class ZopeStarter:
         # after it has emitted ZServer messages.
         
         logger.info('Ready to handle requests')
-        self.setupFinalLogging()
         self.sendEvents()
 
     def run(self):

src/Zope2/Startup/handlers.py

@@ -184,8 +184,10 @@ def root_handler(config):
     """
 
     # Set environment variables
-    for k,v in config.environment.items():
-        os.environ[k] = v
+    d = {}
+    for s in config.environment: 
+        d.update(s)
+    os.environ.update(d)
 
     # Add directories to the pythonpath
     instancelib = os.path.join(config.instancehome, 'lib', 'python')

src/Zope2/Startup/tests/test_schema.py

@@ -101,9 +101,23 @@ class StartupTestCase(unittest.TestCase):
               NSYNC doesnt
             </environment>
             """)
-        items = conf.environment.items()
+        items = conf.environment[0].items()
         items.sort()
         self.assertEqual(items, [("FEARFACTORY", "rocks"), ("NSYNC","doesnt")])
+        conf, handler = self.load_config_text("""\
+            # instancehome is here since it's required
+            instancehome <<INSTANCE_HOME>>
+            <environment>
+              FEARFACTORY rocks
+            </environment>
+            <environment>
+              NSYNC doesnt
+            </environment>
+            """)
+        self.assertEqual(len(conf.environment), 2)
+        # in principle, we should test the handler as well
+        #  But this would have vast side effects
+        #  Thus, we test with a non regression test
 
     def test_ms_author_via(self):
         import webdav

src/Zope2/Startup/zopeschema.xml

@@ -292,14 +292,14 @@
     </description>
   </multisection>
 
-  <section type="environment" attribute="environment" name="*">
+  <multisection type="environment" attribute="environment" name="*">
     <description>
      A section which allows a user to define arbitrary key-value pairs for
      use as environment variables during Zope's run cycle.  It
      is not recommended to set system-related environment variables such as
      PYTHONPATH within this section.
     </description>
-  </section>
+  </multisection>
 
   <key name="instancehome" datatype="existing-directory"
        required="yes">