PR#4334: work around parse_qs behavior in python < 3.11 - koji

koji

#4334 work around parse_qs behavior in python < 3.11

Merged 2 months ago by tkopecek. Opened 2 months ago by mikem.

mikem/koji blank-query-string into master

avoid duplicate environ.get call

Mike McLean • 2 months ago

28b9ef7

unit test

Mike McLean • 2 months ago

abf33ce

work around parse_qs behavior in python < 3.11

Mike McLean • 2 months ago

af18c59

tests/test_www/loadwebindex.py

file modified

+1 -1

		`@@ -17,4 +17,4 @@`
		`sys.modules[INDEX_MOD] = webidx`
		`spec.loader.exec_module(webidx)`
		`else:`
		`- cli = imp.load_source(INDEX_MOD, INDEX_FILENAME)`
		`+ webidx = imp.load_source(INDEX_MOD, INDEX_FILENAME)`

tests/test_www/test_search.py

file modified

+17

		`@@ -19,6 +19,7 @@`
		`}`
		`urlencode_data = "terms=test&type=package&match=testmatch"`
		`self.fs = FieldStorageCompat({'QUERY_STRING': urlencode_data})`
		`+ self.gen_html = mock.patch.object(webidx, '_genHTML').start()`

		`def __get_server(env):`
		`env['koji.form'] = self.fs`
		`@@ -29,6 +30,18 @@`
		`def tearDown(self):`
		`mock.patch.stopall()`

		`+ def test_no_args(self):`
		`+ self.fs = FieldStorageCompat({'QUERY_STRING': ''})`
		`+`
		`+ webidx.search(self.environ)`
		`+`
		`+ self.gen_html.assert_called_once()`
		`+ # extract values`
		`+ # called as _genHTML(environ, 'search.chtml')`
		`+ args = self.gen_html.call_args_list[0][0] # no kwargs passed here`
		`+ environ = args[0]`
		`+ self.assertEqual(environ['koji.values']['terms'], '')`
		`+`
		`def test_search_exception_match(self):`
		`"""Test taskinfo function raises exception"""`
		`self.server.getBuildTarget.return_info = None`
		`@@ -37,3 +50,7 @@`
		`with self.assertRaises(koji.GenericError) as cm:`
		`webidx.search(self.environ)`
		`self.assertEqual(str(cm.exception), "No such match type: 'testmatch'")`
		`+ self.gen_html.assert_not_called()`
		`+`
		`+`
		`+ # the end`

www/lib/kojiweb/util.py

file modified

+7 -2

		`@@ -211,8 +211,13 @@`
		`"""Emulate the parts of cgi.FieldStorage that we need"""`

		`def __init__(self, environ):`
		`- data = parse_qs(environ.get('QUERY_STRING', ''), strict_parsing=True,`
		`- keep_blank_values=True)`
		`+ qs = environ.get('QUERY_STRING', '')`
		`+ if not qs:`
		`+ # for python < 3.11, parse_qs will error on a blank string`
		`+ self.data = {}`
		`+ return`
		`+`
		`+ data = parse_qs(qs, strict_parsing=True, keep_blank_values=True)`
		`# replace singleton lists with single values`
		`for arg in data:`
		`val = data[arg]`

mikem commented 2 months ago

Prior to python 3.11, parse_qs will error on blank query strings. This change simply skips the call in such a case. Also added a unit test to cover this case.

Fixes https://pagure.io/koji/issue/4332

mikem commented 2 months ago

It looks like the change in python was a side effect of a different fix for unicode handling.

https://github.com/python/cpython/pull/115771
https://github.com/python/cpython/issues/74668

The newer version of the lib (in parse_qsl, which parse_qs calls) has the same basic check we use here. I.e.