PrefixMiddleware shouldn't strip prefix from PATH_INFO

[catlee]

I don't think that PrefixMiddleware? should be stripping the prefix from environPATH_INFO?.

I've run into this a few times when trying to deploy simple single-controller applications. One example is an application called 'signon' that manages single-signon for our intranet.

The app is deployed behind an apache reverse proxy which maps /signon on the apache server to the paste server. The signon application expects urls relative to its root to be actions like "signon", "signoff". So the client-visible url would be "/signon/signon".

PrefixMiddleware? currently strips the prefix from PATH_INFO, so "/signon/signon" gets passed to the application in PATH_INFO as "/" whereas "/signon/signoff" would get passed in as "/signoff". Other strange effects include "/signon/signon123" being passed in as "123"

I'm not sure if there is (or was) a use case for the current behaviour, in which case maybe an option to PrefixMiddleware? would be appropriate. Otherwise, the patch below should suffice.

tests/test_prefix_middleware.py

@@ +1 @@
+from paste.deploy.config import PrefixMiddleware
+from paste.fixture import TestApp
+from py.test import raises
+
+class Bug(Exception): pass
+
+def app(environ, start_response):
+    start_response('200 OK', [('Content-type', 'text/html')])
+    retval = "%s %s" % (environ['PATH_INFO'], environ['SCRIPT_NAME'])
+    return iter(retval)
+
+def test_prefix():
+    wrapped = PrefixMiddleware(app, {'test': 1}, "/signon")
+    test_app = TestApp(wrapped)
+    assert "/ /signon" == test_app.get("/").normal_body
+    assert "/foo /signon" == test_app.get("/foo").normal_body
+    assert "/signoff /signon" == test_app.get("/signoff").normal_body
+    assert "/signon /signon" == test_app.get("/signon").normal_body

paste/deploy/config.py

@@ -252 +252 @@
     
     def __call__(self, environ, start_response):
         url = environ['PATH_INFO']
-        url = re.sub(self.regprefix, r'\1', url)
         if not url: url = '/'
         environ['PATH_INFO'] = url
         environ['SCRIPT_NAME'] = self.prefix

[catlee]

The following patch makes the stripping behaviour configurable, and updates the docstrings to help the user determine when the prefix should be stripped, and when it shouldn't be stripped.

I also slightly changed how the prefix regexp is created and used. re.escape is just in case somebody tries to use a prefix that contains regular expression meta characters.

tests/test_prefix_middleware.py

@@ +1 @@
+from paste.deploy.config import PrefixMiddleware
+from paste.fixture import TestApp
+from py.test import raises
+
+class Bug(Exception): pass
+
+def app(environ, start_response):
+    start_response('200 OK', [('Content-type', 'text/html')])
+    retval = "%s %s" % (environ['PATH_INFO'], environ['SCRIPT_NAME'])
+    return iter(retval)
+
+def test_prefix():
+    wrapped = PrefixMiddleware(app, {"test": "1"}, "/signon")
+    test_app = TestApp(wrapped)
+    assert "/ /signon" == test_app.get("/").normal_body
+    assert "/foo /signon" == test_app.get("/foo").normal_body
+    assert "/signoff /signon" == test_app.get("/signoff").normal_body
+    assert "/ /signon" == test_app.get("/signon").normal_body
+
+def test_prefix2():
+    wrapped = PrefixMiddleware(app, {"test": "1"}, "/signon", strip_prefix=False)
+    test_app = TestApp(wrapped)
+    assert "/ /signon" == test_app.get("/").normal_body
+    assert "/foo /signon" == test_app.get("/foo").normal_body
+    assert "/signoff /signon" == test_app.get("/signoff").normal_body
+    assert "/signon /signon" == test_app.get("/signon").normal_body

paste/deploy/config.py

@@ -205 +205 @@
     is accessed via a reverse proxy with a prefix. The application is accessed
     through the reverse proxy via the the URL prefix '/james', whereas the
     reverse proxy forwards those requests to the application at the prefix '/'.
+    In this case you should also disable ``strip_prefix`` since the reverse
+    proxy is already removing the prefix from the request to the application.
 
     The reverse proxy, being an entirely separate web server, has no way of
     specifying the SCRIPT_NAME variable; it must be manually set by a
@@ -239 +241 @@
 
     You can also use ``scheme`` to explicitly set the scheme (like
     ``scheme = https``).
+
+    If ``strip_prefix`` is set, ``prefix`` will be stripped from
+    PATH_INFO when it appears as the first part of the requested url.
+
+    In some configurations, the reverse proxy will strip the prefix before
+    sending the request to your application, in which case PrefixMiddleware
+    should not try and strip the prefix again.  In configurations like these,
+    set ``strip_prefix`` to False
     """
     def __init__(self, app, global_conf=None, prefix='/',
                  translate_forwarded_server=True,
-                 force_port=None, scheme=None):
+                 force_port=None, scheme=None,
+                 strip_prefix=True):
         self.app = app
         self.prefix = prefix.rstrip('/')
         self.translate_forwarded_server = translate_forwarded_server
-        self.regprefix = re.compile("^%s(.*)$" % self.prefix)
+        self.regprefix = re.compile("^%s(.*)$" % re.escape(self.prefix))
         self.force_port = force_port
         self.scheme = scheme
+        self.strip_prefix = strip_prefix
     
     def __call__(self, environ, start_response):
         url = environ['PATH_INFO']
-        url = re.sub(self.regprefix, r'\1', url)
+        if self.strip_prefix:
+            url = self.regprefix.sub(r'\1', url)
         if not url: url = '/'
         environ['PATH_INFO'] = url
         environ['SCRIPT_NAME'] = self.prefix
@@ -280 +293 @@
 def make_prefix_middleware(
     app, global_conf, prefix='/',
     translate_forwarded_server=True,
-    force_port=None, scheme=None):
+    force_port=None, scheme=None, strip_prefix=True):
     from paste.deploy.converters import asbool
     translate_forwarded_server = asbool(translate_forwarded_server)
+    strip_prefix = asbool(strip_prefix)
     return PrefixMiddleware(
         app, prefix=prefix,
         translate_forwarded_server=translate_forwarded_server,
-        force_port=force_port, scheme=scheme)
+        force_port=force_port, scheme=scheme, strip_prefix=strip_prefix)
 
 make_prefix_middleware.__doc__ = PrefixMiddleware.__doc__