Merge pull request #5 from michael-lazar/url_redirect_quuery_params

Fix HTTP redirects when the URL has query params
michael-lazar · Apr 10, 2019 · 46a3691 · 46a3691
2 parents d4f56f7 + 20eade7
commit 46a3691
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+### UNRELEASED
+
+- Add a helper method to generate a gopher menu from a directory.
+- Fixed bug when redirecting to HTTP pages that was causing query parameters
+  to get lost.
+
 ### v2.1.0 (2019-01-08)
 
 - Added support for establishing TLS connections with gopher clients. See the

diff --git a/flask_gopher/flask_gopher.py b/flask_gopher/flask_gopher.py
@@ -20,12 +20,11 @@
 from flask.helpers import safe_join, send_file
 from flask.sessions import SecureCookieSessionInterface, SecureCookieSession
 from werkzeug.local import LocalProxy
-from werkzeug.urls import url_quote
 from werkzeug.serving import WSGIRequestHandler, can_fork
 from werkzeug.serving import BaseWSGIServer, ThreadingMixIn, ForkingMixIn
 from werkzeug.serving import generate_adhoc_ssl_context, load_ssl_context
-
 from werkzeug.exceptions import HTTPException, BadRequest
+from jinja2.filters import escape
 from itsdangerous import URLSafeSerializer, BadSignature
 
 from .__version__ import __version__
@@ -433,7 +432,10 @@ def _add_gopher_url_redirect(self, app):
         """
         @app.route('/URL:<path:url>')
         def gopher_url_redirect(url):
-            url = url_quote(url)
+            # Use the full_path because it keeps any query params intact
+            url = request.full_path.split(':', 1)[1]  # Drop the "/URL:"
+            url = url.rstrip('?')  # Flask adds an ? even if there are no params
+            url = escape(url)
             return self.URL_REDIRECT_TEMPLATE.format(url=url).strip()
 
     def _add_gopher_error_handler(self, app):

diff --git a/tests/test_flask_gopher.py b/tests/test_flask_gopher.py
@@ -251,6 +251,19 @@ def test_url_redirect(self):
         href = b'<A HREF="https://gopher.floodgap.com">https://gopher.floodgap.com</A>'
         self.assertIn(href, resp)
 
+    def test_url_redirect_with_query_params(self):
+        """
+        Selectors starting with URL:<path> should preserve the URL query params.
+        """
+        url = 'https://gopher.floodgap.com?foo=b"r&foz=baz'
+        escaped_url = 'https://gopher.floodgap.com?foo=b&#34;r&amp;foz=baz'
+
+        resp = self.send_data('/URL:{}\r\n'.format(url).encode())
+        self.assertTrue(resp.startswith(b'<HTML>'))
+        self.assertTrue(resp.endswith(b'</HTML>'))
+
+        self.assertIn('<A HREF="{0}">{0}</A>'.format(escaped_url).encode(), resp)
+
     def test_http_get(self):
         """
         Regular HTTP requests should still work and headers should be passed