launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+476250@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad:revert-escape-query-params into launchpad:master.

Commit message:
revert https://code.launchpad.net/~goulinkh/launchpad/+git/launchpad/+merge/476203

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/476250

The change caused ~50 build failures, which would block any current development, see http://lpbuildbot.canonical.com/builders/lp-devel-focal/builds/697/steps/shell/logs/summary
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:revert-escape-query-params into launchpad:master.

diff --git a/lib/lp/services/webapp/publisher.py b/lib/lp/services/webapp/publisher.py
index 64892eb..18aca49 100644
--- a/lib/lp/services/webapp/publisher.py
+++ b/lib/lp/services/webapp/publisher.py
@@ -306,7 +306,6 @@ class LaunchpadView(UserAttributeCache):
         cache = self._get_json_cache()
         if cache is None:
             return
-        self.form = request.form_ng
         related_features = cache.setdefault("related_features", {})
         related_features.update(self.related_feature_info)
 
diff --git a/lib/lp/services/webapp/servers.py b/lib/lp/services/webapp/servers.py
index ec7f94d..1bed9ee 100644
--- a/lib/lp/services/webapp/servers.py
+++ b/lib/lp/services/webapp/servers.py
@@ -68,7 +68,6 @@ from lp.services.webapp.authorization import (
     LAUNCHPAD_SECURITY_POLICY_CACHE_UNAUTH_KEY,
 )
 from lp.services.webapp.errorlog import ErrorReportRequest
-from lp.services.webapp.escaping import html_escape
 from lp.services.webapp.interaction import get_interaction_extras
 from lp.services.webapp.interfaces import (
     IBasicLaunchpadRequest,
@@ -749,10 +748,6 @@ class BrowserFormNG:
     def __init__(self, form):
         """Create a new BrowserFormNG that wraps a dict of form data."""
         self.form = form
-        # sanitize the form data
-        for key, value in list(self.form.items()):
-            if isinstance(value, str):
-                self.form[key] = html_escape(value)
 
     def __contains__(self, name):
         """See IBrowserFormNG."""
diff --git a/lib/lp/services/webapp/tests/test_publisher.py b/lib/lp/services/webapp/tests/test_publisher.py
index 79530a4..8e6cc93 100644
--- a/lib/lp/services/webapp/tests/test_publisher.py
+++ b/lib/lp/services/webapp/tests/test_publisher.py
@@ -525,51 +525,6 @@ class TestLaunchpadView(TestCaseWithFactory):
         ]
         self.assertEqual(expected_beta_features, view.beta_features)
 
-    def test_request_form_sanitizes_html(self):
-        """Test that HTML in form parameters is properly escaped."""
-        request = LaunchpadTestRequest(
-            form={"resize_frame": "<script>alert(1)</script>"}
-        )
-        LaunchpadView(object(), request)
-        self.assertEqual(
-            request.form["resize_frame"],
-            "&lt;script&gt;alert(1)&lt;/script&gt;",
-        )
-
-    def test_request_form_preserves_safe_values(self):
-        """Test that safe form values are not modified."""
-        request = LaunchpadTestRequest(
-            form={"resize_frame": "normal123-value.text"}
-        )
-        LaunchpadView(object(), request)
-        self.assertEqual(request.form["resize_frame"], "normal123-value.text")
-
-    def test_request_form_sanitizes_multiple_values(self):
-        """Test that multiple form values containing HTML are escaped."""
-        request = LaunchpadTestRequest(
-            form={
-                "field1": "<p>test</p>",
-                "field2": "'); test //",
-            }
-        )
-        LaunchpadView(object(), request)
-        self.assertEqual(request.form["field1"], "&lt;p&gt;test&lt;/p&gt;")
-        self.assertEqual(request.form["field2"], "&#x27;); test //")
-
-    def test_request_form_handles_non_string_values(self):
-        """Test that non-string form values are not modified."""
-        request = LaunchpadTestRequest(form={"number": 123, "boolean": True})
-        LaunchpadView(object(), request)
-        self.assertEqual(request.form["number"], 123)
-        self.assertEqual(request.form["boolean"], True)
-
-    def test_request_form_handles_empty_values(self):
-        """Test that empty or None form values are handled properly."""
-        request = LaunchpadTestRequest(form={"empty": "", "none": None})
-        LaunchpadView(object(), request)
-        self.assertEqual(request.form["empty"], "")
-        self.assertIsNone(request.form["none"])
-
 
 class TestRedirectionView(TestCase):
     layer = DatabaseFunctionalLayer