repoquery: Properly sanitize queryformat strings

[gotmax23]

Previously, dnf repoquery --qf allowed looking up arbitrary attributes of the dnf.package.Package objects in the query and e.g. the current Base via the .base attribute. This checks that %{FOO} is a valid query string as per dnf repoquery --querytags. If it isn't, rpm2py_format will leave a literal "%{FOO}".

Before:

$ dnf repoquery dnf --qf='%{base}' --latest=1 --arch=noarch -q
<dnf.cli.cli.BaseCli object at ...>

After:

$ dnf repoquery dnf --qf='%{base}' --latest=1 --arch=noarch -q
%{base}

= changelog =
msg: repoqury: Properly sanitize queryformat strings
type: bugfix
resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2140884

[m-blaha]

Now the list is in two places - ALLOWED_QUERY_TAGS and QUERY_TAGS. Could one of them be somehow generated from the other?
What do you think?

[gotmax23]

Definitely. Something like (

diff --git a/dnf/cli/commands/repoquery.py b/dnf/cli/commands/repoquery.py
index 098ee836..83023aca 100644
--- a/dnf/cli/commands/repoquery.py
+++ b/dnf/cli/commands/repoquery.py
@@ -443,7 +443,7 @@ class RepoQueryCommand(commands.Command):
 
     def run(self):
         if self.opts.querytags:
-            print(QUERY_TAGS)
+            print("\n".join(ALLOWED_QUERY_TAGS))
             return
 
         self.cli._populate_update_security_filter(self.opts)

) would work. I just noticed that QUERY_TAGS was formatted in a rather specific way and thought it might better to keep it that way. Are you okay with the above solution?

[m-blaha]

It prints almost 30 lines of output which seems too much to me. Maybe wrapping it using textwrap module? It creates almost identical output like original code:

print('\n'.join(textwrap.wrap(', '.join(ALLOWED_QUERY_TAGS))))

Or generate it the other way round? ALLOWED_QUERY_TAGS = QUERY_TAGS.replace('\n', ' ').split(', ')

[gotmax23]

I think printing each on its own line is nice. It mirrors that behavior of rpm --querytags and makes it easy to grep for a field name. Otherwise, I think your first solution is better than generating it the other way around.

[m-blaha]

OK then, let's go with "rpm compatible" solution then.

[gotmax23]

See bb69114

EDIT: I force pushed to fix a commit message typo and thus there's a new commit hash

[m-blaha]

Also the failing unit test test_illegal_attr() (see

dnf/tests/test_repoquery.py

Lines 130 to 137 in 45a06f9

	def test_illegal_attr(self):
	pkg = PkgStub()
	with self.assertRaises(AttributeError) as ctx:
	dnf.cli.commands.repoquery.rpm2py_format('%{notfound}').format(pkg)
	self.assertEqual(str(ctx.exception),
	"'PkgStub' object has no attribute 'notfound'")

) needs to be adjusted. Query format does not raise AttributeError exception any more.

[gotmax23]

I've improved the unit tests in d2e3593

[m-blaha]

Thank you for the patch. The test of repoquery --querytags has failed which is expected. Here is PR with adjusted test: rpm-software-management/ci-dnf-stack#1184

[jlebon]

Follow-up to this in #1922.