Repository: pallets/werkzeug
Pull request #1141 (2017-06-24 23:21:55)

remove lighttpd version check in CGIRootFix
According to #266, CGIRootFix is not only for lighttpd now.
So, SERVER_SOFTWARE should not be assumed to start with `'lighttpd/'`, as this value can be `'Apache/x.x.x'` (less than `'lighttpd/1.4.28'`) or `'nginx/x.x.x'` (greater), for example.

davidism: @untitaker maybe that whole if statement can be removed?
ThiefMaster: :+1: on removing it. people should not use this kind of middleware unconditionally but either use a custom wsgi file applying it or add a config option to determine whether to use it or not
untitaker: I don't understand what this code does.

On Sat, Jun 24, 2017 at 05:19:24PM -0700, David Lord wrote:
> @untitaker maybe that whole if statement can be removed?
> 
> -- 
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub:
> https://github.com/pallets/werkzeug/pull/1141#issuecomment-310873808

davidism: I'm removing the entire check. It's addressing servers before Lighttpd 1.4.28, which was released in 2010. We can move on.

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index a79962b5..e5778f82 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -110,6 +110,10 @@ Unreleased
 -   If a nested ``ImportError`` occurs from :func:`~utils.import_string`
     the traceback mentions the nested import. Removes an untested code
     path for handling "modules not yet set up by the parent." (`#735`_)
+-   ``CGIRootFix`` no longer modifies ``PATH_INFO`` for very old
+    versions of Lighttpd. ``LighttpdCGIRootFix`` was renamed to
+    ``CGIRootFix`` in 0.9. The old name emits a deprecation warning and
+    will be removed in the next version. (`#1141`_)
 
 .. _`#209`: https://github.com/pallets/werkzeug/pull/209
 .. _`#609`: https://github.com/pallets/werkzeug/pull/609
@@ -123,6 +127,7 @@ Unreleased
 .. _`#795`: https://github.com/pallets/werkzeug/pull/795
 .. _`#1019`: https://github.com/pallets/werkzeug/issues/1019
 .. _`#1023`: https://github.com/pallets/werkzeug/issues/1023
+.. _`#1141`: https://github.com/pallets/werkzeug/issues/1141
 .. _`#1231`: https://github.com/pallets/werkzeug/issues/1231
 .. _`#1233`: https://github.com/pallets/werkzeug/pull/1233
 .. _`#1237`: https://github.com/pallets/werkzeug/pull/1237
diff --git a/werkzeug/contrib/__init__.py b/werkzeug/contrib/__init__.py
index 8edee7fd..7934f740 100644
--- a/werkzeug/contrib/__init__.py
+++ b/werkzeug/contrib/__init__.py
@@ -21,4 +21,4 @@ class WerkzeugContribDeprecationWarning(DeprecationWarning):
     pass
 
 
-warnings.simplefilter("module", WerkzeugContribDeprecationWarning)
+warnings.simplefilter("default", WerkzeugContribDeprecationWarning)
diff --git a/werkzeug/contrib/fixers.py b/werkzeug/contrib/fixers.py
index 4c4cec35..9a304405 100644
--- a/werkzeug/contrib/fixers.py
+++ b/werkzeug/contrib/fixers.py
@@ -18,6 +18,8 @@
 """
 import warnings
 
+from werkzeug.contrib import WerkzeugContribDeprecationWarning
+
 try:
     from urllib import unquote
 except ImportError:
@@ -30,38 +32,37 @@ from werkzeug.datastructures import Headers, ResponseCacheControl
 
 
 class CGIRootFix(object):
-
-    """Wrap the application in this middleware if you are using FastCGI or CGI
-    and you have problems with your app root being set to the cgi script's path
-    instead of the path users are going to visit
+    """Wrap the application in this middleware if you are using FastCGI
+    or CGI and you have problems with your app root being set to the CGI
+    script's path instead of the path users are going to visit.
 
     .. versionchanged:: 0.9
-       Added `app_root` parameter and renamed from `LighttpdCGIRootFix`.
+        Added `app_root` parameter and renamed from
+        ``LighttpdCGIRootFix``.
 
     :param app: the WSGI application
-    :param app_root: Defaulting to ``'/'``, you can set this to something else
-        if your app is mounted somewhere else.
+    :param app_root: Defaulting to ``'/'``, you can set this to
+        something else if your app is mounted somewhere else.
     """
 
     def __init__(self, app, app_root='/'):
         self.app = app
-        self.app_root = app_root
+        self.app_root = app_root.strip("/")
 
     def __call__(self, environ, start_response):
-        # only set PATH_INFO for older versions of Lighty or if no
-        # server software is provided.  That's because the test was
-        # added in newer Werkzeug versions and we don't want to break
-        # people's code if they are using this fixer in a test that
-        # does not set the SERVER_SOFTWARE key.
-        if 'SERVER_SOFTWARE' not in environ or \
-           environ['SERVER_SOFTWARE'] < 'lighttpd/1.4.28':
-            environ['PATH_INFO'] = environ.get('SCRIPT_NAME', '') + \
-                environ.get('PATH_INFO', '')
-        environ['SCRIPT_NAME'] = self.app_root.strip('/')
+        environ['SCRIPT_NAME'] = self.app_root
         return self.app(environ, start_response)
 
-# backwards compatibility
-LighttpdCGIRootFix = CGIRootFix
+
+class LighttpdCGIRootFix(CGIRootFix):
+    def __init__(self, *args, **kwargs):
+        warnings.warn(
+            "LighttpdCGIRootFix is renamed CGIRootFix and will be"
+            " removed in 1.0.",
+            WerkzeugContribDeprecationWarning,
+            stacklevel=3,
+        )
+        super(LighttpdCGIRootFix, self).__init__(*args, **kwargs)
 
 
 class PathInfoFromRequestUriFix(object):


Test patch:
diff --git a/tests/contrib/test_fixers.py b/tests/contrib/test_fixers.py
index 1edf69a9..70f0ff30 100644
--- a/tests/contrib/test_fixers.py
+++ b/tests/contrib/test_fixers.py
@@ -35,18 +35,17 @@ class TestServerFixer(object):
             app,
             dict(create_environ(),
                  SCRIPT_NAME='/foo',
-                 PATH_INFO='/bar',
-                 SERVER_SOFTWARE='lighttpd/1.4.27'))
-        assert response.get_data() == b'PATH_INFO: /foo/bar\nSCRIPT_NAME: '
+                 PATH_INFO='/bar'))
+        assert response.get_data() == b'PATH_INFO: /bar\nSCRIPT_NAME: '
 
     def test_cgi_root_fix_custom_app_root(self):
-        app = fixers.CGIRootFix(path_check_app, app_root='/baz/poop/')
+        app = fixers.CGIRootFix(path_check_app, app_root='/baz/')
         response = Response.from_app(
             app,
             dict(create_environ(),
                  SCRIPT_NAME='/foo',
                  PATH_INFO='/bar'))
-        assert response.get_data() == b'PATH_INFO: /foo/bar\nSCRIPT_NAME: baz/poop'
+        assert response.get_data() == b'PATH_INFO: /bar\nSCRIPT_NAME: baz'
 
     def test_path_info_from_request_uri_fix(self):
         app = fixers.PathInfoFromRequestUriFix(path_check_app)