- In the past, during traversal, the ModelGraphTraverser (the

    default traverser) always passed each URL path segment to any
    ``__getitem__`` method of a model object as a byte string (a
    ``str`` object).  Now, by default the ModelGraphTraverser attempts
    to decode the path segment to Unicode (a ``unicode`` object) using
    the UTF-8 encoding before passing it to the ``__getitem__`` method
    of a model object.  This makes it possible for model objects to be
    dumber in ``__getitem__`` when trying to resolve a subobject, as
    model objects themselves no longer need to try to divine whether
    or not to try to decode the path segment passed by the
    traverser.

    Note that since 0.5.4, URLs generated by repoze.bfg's
    ``model_url`` API will contain UTF-8 encoded path segments as
    necessary, so any URL generated by BFG itself will be decodeable
    by the traverser.  If another application generates URLs to a BFG
    application, to be resolved successully, it should generate the
    URL with UTF-8 encoded path segments to be successfully resolved.
    The decoder is not at all magical: if a non-UTF-8-decodeable path
    segment (e.g. one encoded using UTF-16 or some other insanity) is
    passed in the URL, BFG will raise a ``TypeError`` with a message
    indicating it could not decode the path segment.

    To turn on the older behavior, where path segments were not
    decoded to Unicode before being passed to model object
    ``__getitem__`` by the traverser, and were passed as a raw byte
    string, set the ``unicode_path_segments`` configuration setting to
    a false value in your BFG application's section of the paste .ini
    file, for example::

      unicode_path_segments = False

    Or start the application using the ``BFG_UNICODE_PATH_SEGMENT``
    envvar set to a false value::

      BFG_UNICODE_PATH_SEGMENTS=0

6 files changed, 151 insertions, 24 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index f7271114c..c41c1090b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,44 @@
+Next release
+
+  Backwards Incompatbilities
+
+  - In the past, during traversal, the ModelGraphTraverser (the
+    default traverser) always passed each URL path segment to any
+    ``__getitem__`` method of a model object as a byte string (a
+    ``str`` object).  Now, by default the ModelGraphTraverser attempts
+    to decode the path segment to Unicode (a ``unicode`` object) using
+    the UTF-8 encoding before passing it to the ``__getitem__`` method
+    of a model object.  This makes it possible for model objects to be
+    dumber in ``__getitem__`` when trying to resolve a subobject, as
+    model objects themselves no longer need to try to divine whether
+    or not to try to decode the path segment passed by the
+    traverser.
+
+    Note that since 0.5.4, URLs generated by repoze.bfg's
+    ``model_url`` API will contain UTF-8 encoded path segments as
+    necessary, so any URL generated by BFG itself will be decodeable
+    by the traverser.  If another application generates URLs to a BFG
+    application, to be resolved successully, it should generate the
+    URL with UTF-8 encoded path segments to be successfully resolved.
+    The decoder is not at all magical: if a non-UTF-8-decodeable path
+    segment (e.g. one encoded using UTF-16 or some other insanity) is
+    passed in the URL, BFG will raise a ``TypeError`` with a message
+    indicating it could not decode the path segment.
+
+    To turn on the older behavior, where path segments were not
+    decoded to Unicode before being passed to model object
+    ``__getitem__`` by the traverser, and were passed as a raw byte
+    string, set the ``unicode_path_segments`` configuration setting to
+    a false value in your BFG application's section of the paste .ini
+    file, for example::
+
+      unicode_path_segments = False
+
+    Or start the application using the ``BFG_UNICODE_PATH_SEGMENT``
+    envvar set to a false value::
+
+      BFG_UNICODE_PATH_SEGMENTS=0
+
 0.5.4 (12/13/2008)
 
   Backwards Incompatibilities
diff --git a/docs/narr/environment.rst b/docs/narr/environment.rst
index 3fe3457c0..69dc98e25 100644
--- a/docs/narr/environment.rst
+++ b/docs/narr/environment.rst
@@ -19,26 +19,36 @@ setting names documented in this chapter are reserved for
 :mod:`repoze.bfg` use.  You should not use them to indicate
 application-specific configuration settings.
 
-+-----------------------------+--------------------------+-------------------------------------+
-| Environment Variable Name   | Config File Setting Name |       Further Information           |
-+=============================+==========================+=====================================+
-| ``BFG_RELOAD_TEMPLATES``    |  ``reload_templates``    |  Reload templates without restart   |
-|                             |                          |  when true                          |
-|                             |                          |  See also:                          |
-|                             |                          |  :ref:`reload_templates_section`    |
-+-----------------------------+--------------------------+-------------------------------------+
-| ``BFG_DEBUG_AUTHORIZATION`` |  ``debug_authorization`` |  Print view authorization failure & |
-|                             |                          |  success info to stderr when true   |
-|                             |                          |  See also:                          |
-|                             |                          |  :ref:`debug_authorization_section` |
-+-----------------------------+--------------------------+-------------------------------------+
-| ``BFG_DEBUG_NOTFOUND``      |  ``debug_notfound``      |  Print view-related NotFound debug  |
-|                             |                          |  messages to stderr when true       |
-|                             |                          |  See also:                          |
-|                             |                          |  :ref:`debug_notfound_section`      |
-+-----------------------------+--------------------------+-------------------------------------+
-| ``BFG_DEBUG_ALL``           |  ``debug_all``           |  Turns all debug_* settings on.     |
-+-----------------------------+--------------------------+-------------------------------------+
++---------------------------------+-----------------------------+----------------------------------------+
+| Environment Variable Name       | Config File Setting Name    |       Further Information              |
++=================================+=============================+========================================+
+| ``BFG_RELOAD_TEMPLATES``        |  ``reload_templates``       |  Reload templates without restart      |
+|                                 |                             |  when true                             |
+|                                 |                             |  See also:                             |
+|                                 |                             |  :ref:`reload_templates_section`       |
++---------------------------------+-----------------------------+----------------------------------------+
+| ``BFG_DEBUG_AUTHORIZATION``     |  ``debug_authorization``    |  Print view authorization failure &    |
+|                                 |                             |  success info to stderr when true      |
+|                                 |                             |  See also:                             |
+|                                 |                             |  :ref:`debug_authorization_section`    | 
++---------------------------------+-----------------------------+----------------------------------------+
+| ``BFG_DEBUG_NOTFOUND``          |  ``debug_notfound``         |  Print view-related NotFound debug     |
+|                                 |                             |  messages to stderr when true          |
+|                                 |                             |  See also:                             |
+|                                 |                             |  :ref:`debug_notfound_section`         |
++---------------------------------+-----------------------------+----------------------------------------+
+| ``BFG_DEBUG_ALL``               |  ``debug_all``              |  Turns all debug_* settings on.        |
++---------------------------------+-----------------------------+----------------------------------------+
+| ``BFG_UNICODE_PATH_SEGMENTS``   |  ``unicode_path_segments``  |  Defaults to ``true``.  When ``true``, |
+|                                 |                             |  URL path segment names will be passed |
+|                                 |                             |  to model object ``__getitem__``       |
+|                                 |                             |  methods by the BFG model graph        |
+|                                 |                             |  traverser as ``unicode`` types rather |
+|                                 |                             |  than as ``str`` types; path segments  |
+|                                 |                             |  will be assumed to be UTF-8 encoded.  |
+|                                 |                             |  When ``false``, pass path segments    |
+|                                 |                             |  as undecoded ``str`` types.           |
++---------------------------------+-----------------------------+----------------------------------------+
 
 Examples
 --------
diff --git a/repoze/bfg/registry.py b/repoze/bfg/registry.py
index b76453f36..5dda3f7a6 100644
--- a/repoze/bfg/registry.py
+++ b/repoze/bfg/registry.py
@@ -74,6 +74,7 @@ class Settings(object):
     reload_templates = False
     debug_notfound = False
     debug_authorization = False
+    unicode_path_segments = True
     def __init__(self, options):
         self.__dict__.update(options)
 
@@ -102,13 +103,17 @@ def get_options(kw, environ=os.environ):
     config_debug_notfound = kw.get('debug_notfound', '')
     effective_debug_notfound = asbool(eget('BFG_DEBUG_NOTFOUND',
                                            config_debug_notfound))
-    config_reload_templates = kw.get('reload_templates')
+    config_reload_templates = kw.get('reload_templates', '')
     effective_reload_templates = asbool(eget('BFG_RELOAD_TEMPLATES',
                                         config_reload_templates))
+    config_unicode_path_segments = kw.get('unicode_path_segments', '')
+    effective_unicode_path_segments = asbool(eget('BFG_UNICODE_PATH_SEGMENTS',
+                                                  config_unicode_path_segments))
     return {
         'debug_authorization': effective_debug_all or effective_debug_auth,
         'debug_notfound': effective_debug_all or effective_debug_notfound,
         'reload_templates': effective_reload_templates,
+        'unicode_path_segments': effective_unicode_path_segments,
         }
 
 from zope.testing.cleanup import addCleanUp
diff --git a/repoze/bfg/tests/test_registry.py b/repoze/bfg/tests/test_registry.py
index 0b7eda586..f10fe6a52 100644
--- a/repoze/bfg/tests/test_registry.py
+++ b/repoze/bfg/tests/test_registry.py
@@ -117,6 +117,22 @@ class TestGetOptions(unittest.TestCase):
         self.assertEqual(result['debug_notfound'], True)
         self.assertEqual(result['debug_authorization'], True)
 
+    def test_unicode_path_segments(self):
+        get_options = self._getFUT()
+        result = get_options({})
+        self.assertEqual(result['unicode_path_segments'], False)
+        result = get_options({'unicode_path_segments':'false'})
+        self.assertEqual(result['unicode_path_segments'], False)
+        result = get_options({'unicode_path_segments':'t'})
+        self.assertEqual(result['unicode_path_segments'], True)
+        result = get_options({'unicode_path_segments':'1'})
+        self.assertEqual(result['unicode_path_segments'], True)
+        result = get_options({}, {'BFG_UNICODE_PATH_SEGMENTS':'1'})
+        self.assertEqual(result['unicode_path_segments'], True)
+        result = get_options({'unicode_path_segments':'false'},
+                             {'BFG_UNICODE_PATH_SEGMENTS':'1'})
+        self.assertEqual(result['unicode_path_segments'], True)
+
 class TestSettings(unittest.TestCase):
     def _getTargetClass(self):
         from repoze.bfg.registry import Settings
@@ -131,12 +147,14 @@ class TestSettings(unittest.TestCase):
         self.assertEqual(settings.reload_templates, False)
         self.assertEqual(settings.debug_notfound, False)
         self.assertEqual(settings.debug_authorization, False)
+        self.assertEqual(settings.unicode_path_segments, True)
 
     def test_with_option(self):
         settings = self._makeOne(reload_templates=True)
         self.assertEqual(settings.reload_templates, True)
         self.assertEqual(settings.debug_notfound, False)
         self.assertEqual(settings.debug_authorization, False)
+        self.assertEqual(settings.unicode_path_segments, True)
 
 class TestThreadLocalRegistryManager(unittest.TestCase, PlacelessSetup):
     def setUp(self):
diff --git a/repoze/bfg/tests/test_traversal.py b/repoze/bfg/tests/test_traversal.py
index 32be1ebbb..21e2b6180 100644
--- a/repoze/bfg/tests/test_traversal.py
+++ b/repoze/bfg/tests/test_traversal.py
@@ -162,6 +162,42 @@ class ModelGraphTraverserTests(unittest.TestCase, PlacelessSetup):
         self.assertEqual(ctx.__parent__, bar)
         self.failIf(isProxy(ctx.__parent__))
 
+    def test_non_utf8_path_segment_unicode_path_segments_fails(self):
+        foo = DummyContext()
+        root = DummyContext(foo)
+        policy = self._makeOne(root)
+        segment = unicode('LaPe\xc3\xb1a', 'utf-8').encode('utf-16')
+        environ = self._getEnviron(PATH_INFO='/%s' % segment)
+        self.assertRaises(TypeError, policy, environ)
+
+    def test_non_utf8_path_segment_settings_unicode_path_segments_fails(self):
+        defaultkw = {'unicode_path_segments':True}
+        settings = DummySettings(**defaultkw)
+        from repoze.bfg.interfaces import ISettings
+        import zope.component
+        gsm = zope.component.getGlobalSiteManager()
+        gsm.registerUtility(settings, ISettings)
+        foo = DummyContext()
+        root = DummyContext(foo)
+        policy = self._makeOne(root)
+        segment = unicode('LaPe\xc3\xb1a', 'utf-8').encode('utf-16')
+        environ = self._getEnviron(PATH_INFO='/%s' % segment)
+        self.assertRaises(TypeError, policy, environ)
+
+    def test_non_utf8_path_segment_str_path_segments_succeeds(self):
+        defaultkw = {'unicode_path_segments':False}
+        settings = DummySettings(**defaultkw)
+        from repoze.bfg.interfaces import ISettings
+        import zope.component
+        gsm = zope.component.getGlobalSiteManager()
+        gsm.registerUtility(settings, ISettings)
+        foo = DummyContext()
+        root = DummyContext(foo)
+        policy = self._makeOne(root)
+        segment = unicode('LaPe\xc3\xb1a', 'utf-8').encode('utf-16')
+        environ = self._getEnviron(PATH_INFO='/%s' % segment)
+        ctx, name, subpath = policy(environ) # test is: this doesn't fail
+
 class FindInterfaceTests(unittest.TestCase):
     def _callFUT(self, context, iface):
         from repoze.bfg.traversal import find_interface
@@ -422,3 +458,6 @@ class DummyContext(object):
 class DummyRequest:
     application_url = 'http://example.com:5432/'
 
+class DummySettings:
+    def __init__(self, **kw):
+        self.__dict__.update(kw)
diff --git a/repoze/bfg/traversal.py b/repoze/bfg/traversal.py
index e599ee6c1..89495a223 100644
--- a/repoze/bfg/traversal.py
+++ b/repoze/bfg/traversal.py
@@ -1,5 +1,7 @@
 import urllib
 import urlparse
+
+from zope.component import queryUtility
    
 from zope.interface import classProvides
 from zope.interface import implements
@@ -9,6 +11,7 @@ from repoze.bfg.location import lineage
 from repoze.bfg.interfaces import ILocation
 from repoze.bfg.interfaces import ITraverser
 from repoze.bfg.interfaces import ITraverserFactory
+from repoze.bfg.interfaces import ISettings
 
 def split_path(path):
     while path.startswith('/'):
@@ -26,7 +29,13 @@ def split_path(path):
             clean.append(segment)
     return clean
 
-def step(ob, name, default):
+def step(ob, name, default, as_unicode=True):
+    if as_unicode:
+        try:
+            name = name.decode('utf-8')
+        except UnicodeDecodeError:
+            raise TypeError('Could not decode path segment "%s" using the '
+                            'UTF-8 decoding scheme' % name)
     if name.startswith('@@'):
         return name[2:], default
     if not hasattr(ob, '__getitem__'):
@@ -36,7 +45,7 @@ def step(ob, name, default):
     except KeyError:
         return name, default
 
-_marker = ()
+_marker = []
 
 class ModelGraphTraverser(object):
     classProvides(ITraverserFactory)
@@ -44,8 +53,13 @@ class ModelGraphTraverser(object):
     def __init__(self, root):
         self.root = root
         self.locatable = ILocation.providedBy(root)
+        self.unicode_path_segments = True
+        settings = queryUtility(ISettings)
+        if settings is not None:
+            self.unicode_path_segments = settings.unicode_path_segments
 
     def __call__(self, environ):
+        unicode_path_segments = self.unicode_path_segments
         path = environ.get('PATH_INFO', '/')
         path = split_path(path)
         ob = self.root
@@ -54,7 +68,7 @@ class ModelGraphTraverser(object):
 
         while path:
             segment = path.pop(0)
-            segment, next = step(ob, segment, _marker)
+            segment, next = step(ob, segment, _marker, unicode_path_segments)
             if next is _marker:
                 name = segment
                 break