rename clean_url method to examine_url. the method doesn't realy clean anything, it throws a ValueError if it finds a problem

Author: rouilj

roundup/cgi/actions.py

@@ -40,7 +40,7 @@ def execute(self):
         self.permission()
         return self.handle()
 
-    def clean_url(self, url):
+    def examine_url(self, url):
         '''Return URL validated to be under self.base and properly escaped
 
         If url not properly escaped or validation fails raise ValueError.
@@ -822,7 +822,7 @@ def handle(self):
         # Allow an option to stay on the page to create new things
         if '__redirect_to' in self.form:
             raise exceptions.Redirect('%s&@ok_message=%s'%(
-                self.clean_url(self.form['__redirect_to'].value),
+                self.examine_url(self.form['__redirect_to'].value),
                 urllib_.quote(messages)))
 
         # otherwise redirect to the new item's page
@@ -1141,7 +1141,7 @@ def handle(self):
             #    This will be redefined if there is a login error to include
             #      a new error message
 
-            clean_url = self.clean_url(self.form['__came_from'].value)
+            clean_url = self.examine_url(self.form['__came_from'].value)
             redirect_url_tuple = urllib_.urlparse(clean_url)
             # now I have a tuple form for the __came_from url
             try:

test/test_cgi.py

@@ -1173,83 +1173,70 @@ def testrenderContext(self):
         self.assertNotEqual(-1, result.index('ok message'))
         # print result
 
-    def testclean_url(self):
-        ''' test the clean_url function '''
+    def testexamine_url(self):
+        ''' test the examine_url function '''
+
+        def te(url, exception, raises=ValueError):
+            with self.assertRaises(raises) as cm:
+                examine_url(url)
+            self.assertEqual(cm.exception.message, exception)
+
+
         action = actions.Action(self.client)
-        clean_url = action.clean_url
+        examine_url = action.examine_url
 
         # Christmas tree url: test of every component that passes
         self.assertEqual(
-            clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
+            examine_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
             'http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue')
 
         # allow replacing http with https if base is http
         self.assertEqual(
-            clean_url("https://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
+            examine_url("https://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
             'https://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue')
 
 
         # change base to use https and make sure we don't redirect to http
         saved_base = action.base
         action.base = "https://tracker.example/cgi-bin/roundup.cgi/bugs/"
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue")
-        self.assertEqual(cm.exception.message, 'Base url https://tracker.example/cgi-bin/roundup.cgi/bugs/ requires https. Redirect url http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue uses http.')
+        te("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue",
+           'Base url https://tracker.example/cgi-bin/roundup.cgi/bugs/ requires https. Redirect url http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue uses http.')
         action.base = saved_base
 
         # url doesn't have to be valid to roundup, just has to be contained
         # inside of roundup. No zoik class is defined
-        self.assertEqual(clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/zoik7;parm=bar?@template=foo&parm=(zot)#issue"), "http://tracker.example/cgi-bin/roundup.cgi/bugs/zoik7;parm=bar?@template=foo&parm=(zot)#issue")
+        self.assertEqual(examine_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/zoik7;parm=bar?@template=foo&parm=(zot)#issue"), "http://tracker.example/cgi-bin/roundup.cgi/bugs/zoik7;parm=bar?@template=foo&parm=(zot)#issue")
 
         # test with wonky schemes
-        with self.assertRaises(ValueError) as cm:
-            clean_url("email://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Unrecognized scheme in email://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue')
+        te("email://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue",
+        'Unrecognized scheme in email://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue')
 
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http%3a//tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Unrecognized scheme in http%3a//tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue')
+        te("http%3a//tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue", 'Unrecognized scheme in http%3a//tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue')
 
         # test different netloc/path prefix
         # assert port
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example:1025/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Net location in http://tracker.example:1025/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
+        te("http://tracker.example:1025/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue",'Net location in http://tracker.example:1025/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
 
         #assert user
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Net location in http://[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
+        te("http://[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue", 'Net location in http://[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
 
         #assert user:password
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://user:[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Net location in http://user:[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
+        te("http://user:[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue", 'Net location in http://user:[email protected]/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
 
         # try localhost http scheme
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://localhost/cgi-bin/roundup.cgi/bugs/user3")
-        self.assertEqual(cm.exception.message, 'Net location in http://localhost/cgi-bin/roundup.cgi/bugs/user3 does not match base: tracker.example')
+        te("http://localhost/cgi-bin/roundup.cgi/bugs/user3", 'Net location in http://localhost/cgi-bin/roundup.cgi/bugs/user3 does not match base: tracker.example')
 
         # try localhost https scheme
-        with self.assertRaises(ValueError) as cm:
-            clean_url("https://localhost/cgi-bin/roundup.cgi/bugs/user3")
-        self.assertEqual(cm.exception.message, 'Net location in https://localhost/cgi-bin/roundup.cgi/bugs/user3 does not match base: tracker.example')
+        te("https://localhost/cgi-bin/roundup.cgi/bugs/user3", 'Net location in https://localhost/cgi-bin/roundup.cgi/bugs/user3 does not match base: tracker.example')
 
         # try different host
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://bad.guys.are.us/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Net location in http://bad.guys.are.us/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
+        te("http://bad.guys.are.us/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue", 'Net location in http://bad.guys.are.us/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#issue does not match base: tracker.example')
 
         # change the base path to .../bug from .../bugs
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example/cgi-bin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Base path /cgi-bin/roundup.cgi/bugs/ is not a prefix for url http://tracker.example/cgi-bin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue')
+        te("http://tracker.example/cgi-bin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue", 'Base path /cgi-bin/roundup.cgi/bugs/ is not a prefix for url http://tracker.example/cgi-bin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue')
 
         # change the base path eliminate - in cgi-bin
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example/cgibin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Base path /cgi-bin/roundup.cgi/bugs/ is not a prefix for url http://tracker.example/cgibin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue')
+        te("http://tracker.example/cgibin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue",'Base path /cgi-bin/roundup.cgi/bugs/ is not a prefix for url http://tracker.example/cgibin/roundup.cgi/bug/user3;parm=bar?@template=foo&parm=(zot)#issue')
 
 
         # scan for unencoded characters
@@ -1258,24 +1245,16 @@ def testclean_url(self):
         #
         # break components with unescaped character '<'
         # path component
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/<user3;parm=bar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Path component (/cgi-bin/roundup.cgi/bugs/<user3) in http://tracker.example/cgi-bin/roundup.cgi/bugs/<user3;parm=bar?@template=foo&parm=(zot)#issue is not properly escaped')
+        te("http://tracker.example/cgi-bin/roundup.cgi/bugs/<user3;parm=bar?@template=foo&parm=(zot)#issue", 'Path component (/cgi-bin/roundup.cgi/bugs/<user3) in http://tracker.example/cgi-bin/roundup.cgi/bugs/<user3;parm=bar?@template=foo&parm=(zot)#issue is not properly escaped')
 
         # params component
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=b<ar?@template=foo&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Params component (parm=b<ar) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=b<ar?@template=foo&parm=(zot)#issue is not properly escaped')
+        te("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=b<ar?@template=foo&parm=(zot)#issue", 'Params component (parm=b<ar) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=b<ar?@template=foo&parm=(zot)#issue is not properly escaped')
 
         # query component
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=<foo>&parm=(zot)#issue"),
-        self.assertEqual(cm.exception.message, 'Query component (@template=<foo>&parm=(zot)) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=<foo>&parm=(zot)#issue is not properly escaped')
+        te("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=<foo>&parm=(zot)#issue", 'Query component (@template=<foo>&parm=(zot)) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=<foo>&parm=(zot)#issue is not properly escaped')
 
         # fragment component
-        with self.assertRaises(ValueError) as cm:
-            clean_url("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#iss<ue"),
-        self.assertEqual(cm.exception.message, 'Fragment component (iss<ue) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#iss<ue is not properly escaped')
+        te("http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#iss<ue", 'Fragment component (iss<ue) in http://tracker.example/cgi-bin/roundup.cgi/bugs/user3;parm=bar?@template=foo&parm=(zot)#iss<ue is not properly escaped')
 
 class TemplateTestCase(unittest.TestCase):
     ''' Test the template resolving code, i.e. what can be given to @template