- Fix a bug in ``repoze.bfg.url.static_url`` URL generation: if two

  resource specifications were used to create two separate static
  views, but they shared a common prefix, it was possible that
  ``static_url`` would generate an incorrect URL.

- Fix another bug in ``repoze.bfg.static_url`` URL generation: too
  many slashes in generated URL.

4 files changed, 39 insertions, 23 deletions

diff --git a/repoze/bfg/static.py b/repoze/bfg/static.py
index 5effd4e4b..71db132b7 100644
--- a/repoze/bfg/static.py
+++ b/repoze/bfg/static.py
@@ -99,7 +99,7 @@ class StaticURLInfo(object):
             if path.startswith(spec):
                 subpath = path[len(spec):]
                 if is_url:
-                    return urljoin(name, subpath[1:])
+                    return urljoin(name, subpath)
                 else:
                     kw['subpath'] = subpath
                     return self.route_url(name, request, **kw)
@@ -107,16 +107,32 @@ class StaticURLInfo(object):
         raise ValueError('No static URL definition matching %s' % path)
 
     def add(self, name, spec, **extra):
+        # This feature only allows for the serving of a directory and
+        # the files contained within, not of a single resource;
+        # appending a slash here if the spec doesn't have one is
+        # required for proper prefix matching done in ``generate``
+        # (``subpath = path[len(spec):]``).
+        if not spec.endswith('/'):
+            spec = spec + '/'
+
+        # we also make sure the name ends with a slash, purely as a
+        # convenience: a name that is a url is required to end in a
+        # slash, so that ``urljoin(name, subpath))`` will work above
+        # when the name is a URL, and it doesn't hurt things for it to
+        # have a name that ends in a slash if it's used as a route
+        # name instead of a URL.
+        if not name.endswith('/'):
+            # make sure it ends with a slash
+            name = name + '/'
+
         names = [ t[0] for t in self.registrations ]
+
         if name in names:
             idx = names.index(name)
             self.registrations.pop(idx)
 
         if urlparse(name)[0]:
             # it's a URL
-            if not name.endswith('/'):
-                # make sure it ends with a slash
-                name = name + '/'
             self.registrations.append((name, spec, True))
         else:
             # it's a view name
@@ -126,7 +142,7 @@ class StaticURLInfo(object):
             # register a route using this view
             self.config.add_route(
                 name,
-                "%s/*subpath" % name,
+                "%s*subpath" % name, # name already ends with slash
                 view=view,
                 view_for=self.__class__,
                 factory=lambda *x: self,
diff --git a/repoze/bfg/tests/test_configuration.py b/repoze/bfg/tests/test_configuration.py
index 5c677fdcd..232cbd6bb 100644
--- a/repoze/bfg/tests/test_configuration.py
+++ b/repoze/bfg/tests/test_configuration.py
@@ -1969,8 +1969,8 @@ class ConfiguratorTests(unittest.TestCase):
         from repoze.bfg.interfaces import IViewClassifier
         config = self._makeOne()
         config.add_static_view('static', 'fixtures/static')
-        request_type = self._getRouteRequestIface(config, 'static')
-        route = self._assertRoute(config, 'static', 'static/*subpath')
+        request_type = self._getRouteRequestIface(config, 'static/')
+        route = self._assertRoute(config, 'static/', 'static/*subpath')
         self.assertEqual(route.factory.__class__, type(lambda x: x))
         iface = implementedBy(StaticURLInfo)
         wrapped = config.registry.adapters.lookup(
diff --git a/repoze/bfg/tests/test_static.py b/repoze/bfg/tests/test_static.py
index d9893ab2f..449860883 100644
--- a/repoze/bfg/tests/test_static.py
+++ b/repoze/bfg/tests/test_static.py
@@ -256,25 +256,25 @@ class TestStaticURLInfo(unittest.TestCase):
 
     def test_generate_slash_in_name1(self):
         inst = self._makeOne(None)
-        inst.registrations = [('http://example.com/foo/', 'package:path', True)]
+        inst.registrations = [('http://example.com/foo/', 'package:path/',True)]
         request = DummyRequest()
         result = inst.generate('package:path/abc', request)
         self.assertEqual(result, 'http://example.com/foo/abc')
 
     def test_generate_slash_in_name2(self):
         inst = self._makeOne(None)
-        inst.registrations = [('http://example.com/foo/', 'package:path', True)]
+        inst.registrations = [('http://example.com/foo/', 'package:path/',True)]
         request = DummyRequest()
-        result = inst.generate('package:path', request)
+        result = inst.generate('package:path/', request)
         self.assertEqual(result, 'http://example.com/foo/')
 
     def test_generate_route_url(self):
         inst = self._makeOne(None)
-        inst.registrations = [('viewname', 'package:path', False)]
+        inst.registrations = [('viewname/', 'package:path/', False)]
         def route_url(n, r, **kw):
-            self.assertEqual(n, 'viewname')
+            self.assertEqual(n, 'viewname/')
             self.assertEqual(r, request)
-            self.assertEqual(kw, {'subpath':'/abc', 'a':1})
+            self.assertEqual(kw, {'subpath':'abc', 'a':1})
             return 'url'
         request = DummyRequest()
         inst.route_url = route_url
@@ -283,21 +283,21 @@ class TestStaticURLInfo(unittest.TestCase):
 
     def test_add_already_exists(self):
         inst = self._makeOne(None)
-        inst.registrations = [('http://example.com/', 'package:path')]
-        inst.add('http://example.com/', 'anotherpackage:path')
-        expected = [('http://example.com/', 'anotherpackage:path', True)]
+        inst.registrations = [('http://example.com/', 'package:path/', True)]
+        inst.add('http://example.com', 'anotherpackage:path')
+        expected = [('http://example.com/',  'anotherpackage:path/', True)]
         self.assertEqual(inst.registrations, expected)
-        
+
     def test_add_url_withendslash(self):
         inst = self._makeOne(None)
         inst.add('http://example.com/', 'anotherpackage:path')
-        expected = [('http://example.com/', 'anotherpackage:path', True)]
+        expected = [('http://example.com/', 'anotherpackage:path/', True)]
         self.assertEqual(inst.registrations, expected)
 
     def test_add_url_noendslash(self):
         inst = self._makeOne(None)
         inst.add('http://example.com', 'anotherpackage:path')
-        expected = [('http://example.com/', 'anotherpackage:path', True)]
+        expected = [('http://example.com/', 'anotherpackage:path/', True)]
         self.assertEqual(inst.registrations, expected)
 
     def test_add_viewname(self):
@@ -309,9 +309,9 @@ class TestStaticURLInfo(unittest.TestCase):
         config = Config()
         inst = self._makeOne(config)
         inst.add('view', 'anotherpackage:path', cache_max_age=1)
-        expected = [('view', 'anotherpackage:path', False)]
+        expected = [('view/', 'anotherpackage:path/', False)]
         self.assertEqual(inst.registrations, expected)
-        self.assertEqual(config.arg, ('view', 'view/*subpath'))
+        self.assertEqual(config.arg, ('view/', 'view/*subpath'))
         self.assertEqual(config.kw['_info'], None)
         self.assertEqual(config.kw['view_for'], self._getTargetClass())
         self.assertEqual(config.kw['factory'](), inst)
diff --git a/repoze/bfg/tests/test_zcml.py b/repoze/bfg/tests/test_zcml.py
index 799826d1b..a01b52840 100644
--- a/repoze/bfg/tests/test_zcml.py
+++ b/repoze/bfg/tests/test_zcml.py
@@ -707,14 +707,14 @@ class TestStaticDirective(unittest.TestCase):
         routes = mapper.get_routes()
         self.assertEqual(len(routes), 1)
         self.assertEqual(routes[0].path, 'name/*subpath')
-        self.assertEqual(routes[0].name, 'name')
+        self.assertEqual(routes[0].name, 'name/')
 
         view_action = actions[1]
         discriminator = view_action['discriminator']
         self.assertEqual(discriminator[:3], ('view', StaticURLInfo, ''))
         self.assertEqual(discriminator[4], IView)
         iface = implementedBy(StaticURLInfo)
-        request_type = reg.getUtility(IRouteRequest, 'name')
+        request_type = reg.getUtility(IRouteRequest, 'name/')
         view = reg.adapters.lookup(
             (IViewClassifier, request_type, iface), IView, name='')
         request = DummyRequest()