From c1f7071d338a72684d7c99f9d2e40207ef8f7639 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 1 Feb 2024 21:14:32 +0100 Subject: [PATCH] Remove all path quoting, rely on makeurl() --- osc/build.py | 8 ++++---- osc/commandline.py | 36 ++++++++++++++++++------------------ osc/core.py | 43 +++++++++++++++++++++---------------------- osc/fetch.py | 9 ++++----- tests/test_update.py | 4 ++-- 5 files changed, 49 insertions(+), 51 deletions(-) diff --git a/osc/build.py b/osc/build.py index 2002cb1da8..b77b8ff2b7 100644 --- a/osc/build.py +++ b/osc/build.py @@ -21,7 +21,7 @@ from . import connection from . import core from . import oscerr -from .core import get_buildinfo, meta_exists, quote_plus, get_buildconfig, dgst +from .core import get_buildinfo, meta_exists, get_buildconfig, dgst from .core import get_binarylist, get_binary_file, run_external, return_external, raw_input from .fetch import Fetcher, OscFileGrabber, verify_pacs from .meter import create_text_meter @@ -1021,13 +1021,13 @@ def main(apiurl, store, opts, argv): except HTTPError as e: if e.code == 404: # check what caused the 404 - if meta_exists(metatype='prj', path_args=(quote_plus(prj), ), + if meta_exists(metatype='prj', path_args=(prj, ), template_args=None, create_new=False, apiurl=apiurl): pkg_meta_e = None try: # take care, not to run into double trouble. - pkg_meta_e = meta_exists(metatype='pkg', path_args=(quote_plus(prj), - quote_plus(pac)), template_args=None, create_new=False, + pkg_meta_e = meta_exists(metatype='pkg', path_args=(prj, pac), + template_args=None, create_new=False, apiurl=apiurl) except: pass diff --git a/osc/commandline.py b/osc/commandline.py index b4ce3f431e..b7f7228440 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -1949,7 +1949,7 @@ def do_meta(self, subcmd, opts, *args): edit=True, force=opts.force, remove_linking_repositories=opts.remove_linking_repositories, - path_args=quote_plus(project), + path_args=(project, ), apiurl=apiurl, msg=opts.message, template_args=({ @@ -1958,7 +1958,7 @@ def do_meta(self, subcmd, opts, *args): elif cmd == 'pkg': edit_meta(metatype='pkg', edit=True, - path_args=(quote_plus(project), quote_plus(package)), + path_args=(project, package), apiurl=apiurl, template_args=({ 'name': package, @@ -1966,20 +1966,20 @@ def do_meta(self, subcmd, opts, *args): elif cmd == 'prjconf': edit_meta(metatype='prjconf', edit=True, - path_args=quote_plus(project), + path_args=(project, ), apiurl=apiurl, msg=opts.message, template_args=None) elif cmd == 'user': edit_meta(metatype='user', edit=True, - path_args=(quote_plus(user)), + path_args=(user, ), apiurl=apiurl, template_args=({'user': user})) elif cmd == 'group': edit_meta(metatype='group', edit=True, - path_args=(quote_plus(group)), + path_args=(group, ), apiurl=apiurl, template_args=({'group': group})) elif cmd == 'pattern': @@ -1992,7 +1992,7 @@ def do_meta(self, subcmd, opts, *args): edit_meta( metatype='attribute', edit=True, - path_args=(quote_plus(project), quote_plus(opts.attribute)), + path_args=(project, opts.attribute), apiurl=apiurl, # PUT is not supported method="POST", @@ -2061,32 +2061,32 @@ def do_meta(self, subcmd, opts, *args): remove_linking_repositories=opts.remove_linking_repositories, apiurl=apiurl, msg=opts.message, - path_args=quote_plus(project)) + path_args=(project, )) elif cmd == 'pkg': edit_meta(metatype='pkg', data=f, edit=opts.edit, apiurl=apiurl, - path_args=(quote_plus(project), quote_plus(package))) + path_args=(project, package)) elif cmd == 'prjconf': edit_meta(metatype='prjconf', data=f, edit=opts.edit, apiurl=apiurl, msg=opts.message, - path_args=quote_plus(project)) + path_args=(project, )) elif cmd == 'user': edit_meta(metatype='user', data=f, edit=opts.edit, apiurl=apiurl, - path_args=(quote_plus(user))) + path_args=(user, )) elif cmd == 'group': edit_meta(metatype='group', data=f, edit=opts.edit, apiurl=apiurl, - path_args=(quote_plus(group))) + path_args=(group, )) elif cmd == 'pattern': edit_meta(metatype='pattern', data=f, @@ -6276,7 +6276,7 @@ def do_buildlog(self, subcmd, opts, *args): query['last'] = 1 if opts.lastsucceeded: query['lastsucceeded'] = 1 - u = makeurl(self.get_api_url(), ['build', quote_plus(project), quote_plus(repository), quote_plus(arch), quote_plus(package), '_log'], query=query) + u = makeurl(self.get_api_url(), ['build', project, repository, arch, package, '_log'], query=query) f = http_GET(u) root = ET.parse(f).getroot() offset = int(root.find('entry').get('size')) @@ -6289,7 +6289,7 @@ def do_buildlog(self, subcmd, opts, *args): elif opts.offset: offset = int(opts.offset) strip_time = opts.strip_time or conf.config['buildlog_strip_time'] - print_buildlog(apiurl, quote_plus(project), quote_plus(package), quote_plus(repository), quote_plus(arch), offset, strip_time, opts.last, opts.lastsucceeded) + print_buildlog(apiurl, project, package, repository, arch, offset, strip_time, opts.last, opts.lastsucceeded) def print_repos(self, repos_only=False, exc_class=oscerr.WrongArgs, exc_msg='Missing arguments', project=None): wd = Path.cwd() @@ -6370,7 +6370,7 @@ def do_remotebuildlog(self, subcmd, opts, *args): query['last'] = 1 if opts.lastsucceeded: query['lastsucceeded'] = 1 - u = makeurl(self.get_api_url(), ['build', quote_plus(project), quote_plus(repository), quote_plus(arch), quote_plus(package), '_log'], query=query) + u = makeurl(self.get_api_url(), ['build', project, repository, arch, package, '_log'], query=query) f = http_GET(u) root = ET.parse(f).getroot() offset = int(root.find('entry').get('size')) @@ -6383,7 +6383,7 @@ def do_remotebuildlog(self, subcmd, opts, *args): elif opts.offset: offset = int(opts.offset) strip_time = opts.strip_time or conf.config['buildlog_strip_time'] - print_buildlog(apiurl, quote_plus(project), quote_plus(package), quote_plus(repository), quote_plus(arch), offset, strip_time, opts.last, opts.lastsucceeded) + print_buildlog(apiurl, project, package, repository, arch, offset, strip_time, opts.last, opts.lastsucceeded) def _find_last_repo_arch(self, repo=None, fatal=True): files = glob.glob(os.path.join(Path.cwd(), store, "_buildinfo-*")) @@ -8709,7 +8709,7 @@ def do_importsrcpkg(self, subcmd, opts): apiurl = osc_store.Store(project_dir).apiurl user = conf.get_apiurl_usr(apiurl) data = meta_exists(metatype='pkg', - path_args=(quote_plus(project), quote_plus(pac)), + path_args=(project, pac), template_args=({ 'name': pac, 'user': user}), apiurl=apiurl) @@ -8723,7 +8723,7 @@ def do_importsrcpkg(self, subcmd, opts): print('error - cannot get meta data', file=sys.stderr) sys.exit(1) edit_meta(metatype='pkg', - path_args=(quote_plus(project), quote_plus(pac)), + path_args=(project, pac), data=data, apiurl=apiurl) Package.init_package(apiurl, project, pac, os.path.join(project_dir, pac)) else: @@ -9254,7 +9254,7 @@ def download(self, name, md5, dir, destfile): o = open(destfile, 'wb') if md5 != '': query = {'rev': dir['srcmd5']} - u = makeurl(dir['apiurl'], ['source', dir['project'], dir['package'], pathname2url(name)], query=query) + u = makeurl(dir['apiurl'], ['source', dir['project'], dir['package'], name], query=query) for buf in streamfile(u, http_GET, BUFSIZE): o.write(buf) o.close() diff --git a/osc/core.py b/osc/core.py index e7d82ffe8b..597e0a0f0e 100644 --- a/osc/core.py +++ b/osc/core.py @@ -36,9 +36,8 @@ from io import StringIO from pathlib import Path from typing import Optional, Dict, Union, List, Iterable -from urllib.parse import urlsplit, urlunsplit, urlparse, quote, quote_plus, urlencode, unquote +from urllib.parse import urlsplit, urlunsplit, urlparse, quote, urlencode, unquote from urllib.error import HTTPError -from urllib.request import pathname2url from xml.etree import ElementTree as ET try: @@ -1116,7 +1115,7 @@ def commitNewPackage(self, pac, msg='', files=None, verbose=False, skip_local_se else: user = conf.get_apiurl_usr(self.apiurl) edit_meta(metatype='pkg', - path_args=(quote_plus(self.name), quote_plus(pac)), + path_args=(self.name, pac), template_args=({ 'name': pac, 'user': user}), @@ -1171,11 +1170,11 @@ def commitExtPackage(self, pac, msg, files=None, verbose=False, skip_local_servi package = store_read_package(pac_path) apiurl = store.apiurl if not meta_exists(metatype='pkg', - path_args=(quote_plus(project), quote_plus(package)), + path_args=(project, package), template_args=None, create_new=False, apiurl=apiurl): user = conf.get_apiurl_usr(self.apiurl) edit_meta(metatype='pkg', - path_args=(quote_plus(project), quote_plus(package)), + path_args=(project, package), template_args=({'name': pac, 'user': user}), apiurl=apiurl) p = Package(pac_path) p.todo = files @@ -1542,7 +1541,7 @@ def delete_source_file(self, n): def delete_remote_source_file(self, n): """delete a remote source file (e.g. from the server)""" query = 'rev=upload' - u = makeurl(self.apiurl, ['source', self.prjname, self.name, pathname2url(n)], query=query) + u = makeurl(self.apiurl, ['source', self.prjname, self.name, n], query=query) http_DELETE(u) def put_source_file(self, n, tdir, copy_only=False): @@ -1552,7 +1551,7 @@ def put_source_file(self, n, tdir, copy_only=False): # escaping '+' in the URL path (note: not in the URL query string) is # only a workaround for ruby on rails, which swallows it otherwise if not copy_only: - u = makeurl(self.apiurl, ['source', self.prjname, self.name, pathname2url(n)], query=query) + u = makeurl(self.apiurl, ['source', self.prjname, self.name, n], query=query) http_PUT(u, file=tfilename) if n in self.to_be_added: self.to_be_added.remove(n) @@ -5181,7 +5180,7 @@ def get_group(apiurl: str, group: str): def get_group_meta(apiurl: str, group: str): - u = makeurl(apiurl, ['group', quote_plus(group)]) + u = makeurl(apiurl, ['group', group]) try: f = http_GET(u) return b''.join(f.readlines()) @@ -5191,7 +5190,7 @@ def get_group_meta(apiurl: str, group: str): def get_user_meta(apiurl: str, user: str): - u = makeurl(apiurl, ['person', quote_plus(user)]) + u = makeurl(apiurl, ['person', user]) try: f = http_GET(u) return b''.join(f.readlines()) @@ -5272,7 +5271,7 @@ def get_source_file( query['rev'] = revision u = makeurl( apiurl, - ["source", prj, package, pathname2url(filename.encode(locale.getpreferredencoding(), "replace"))], + ["source", prj, package, filename.encode(locale.getpreferredencoding(), "replace")], query=query, ) download(u, targetfilename, progress_obj, mtime) @@ -5703,7 +5702,7 @@ def checkout_package( # before we create directories and stuff, check if the package actually # exists - meta_data = b''.join(show_package_meta(apiurl, quote_plus(project), quote_plus(package))) + meta_data = b''.join(show_package_meta(apiurl, project, package)) root = ET.fromstring(meta_data) scmsync_element = root.find("scmsync") if scmsync_element is not None and scmsync_element.text is not None: @@ -5823,7 +5822,7 @@ def link_pac( apiurl = conf.config['apiurl'] try: dst_meta = meta_exists(metatype='pkg', - path_args=(quote_plus(dst_project), quote_plus(dst_package)), + path_args=(dst_project, dst_package), template_args=None, create_new=False, apiurl=apiurl) root = ET.fromstring(parse_meta_to_string(dst_meta)) @@ -5953,7 +5952,7 @@ def aggregate_pac( try: dst_meta = meta_exists(metatype='pkg', - path_args=(quote_plus(dst_project), quote_plus(dst_package_meta)), + path_args=(dst_project, dst_package_meta), template_args=None, create_new=False, apiurl=apiurl) root = ET.fromstring(parse_meta_to_string(dst_meta)) @@ -6247,7 +6246,7 @@ def copy_pac( src_meta = replace_pkg_meta(src_meta, dst_package, dst_project, keep_maintainers, dst_userid, keep_develproject) - url = make_meta_url('pkg', (quote_plus(dst_project),) + (quote_plus(dst_package),), dst_apiurl) + url = make_meta_url('pkg', (dst_project, dst_package), dst_apiurl) found = None try: found = http_GET(url).readlines() @@ -6296,7 +6295,7 @@ def copy_pac( with tempfile.NamedTemporaryFile(prefix='osc-copypac') as f: get_source_file(src_apiurl, src_project, src_package, filename, targetfilename=f.name, revision=revision) - path = ['source', dst_project, dst_package, pathname2url(filename)] + path = ['source', dst_project, dst_package, filename] u = makeurl(dst_apiurl, path, query={'rev': 'repository'}) http_PUT(u, file=f.name) tfilelist = Package.commit_filelist(dst_apiurl, dst_project, dst_package, @@ -7876,10 +7875,10 @@ def addMaintainer(apiurl: str, prj: str, pac: str, user: str): def addPerson(apiurl: str, prj: str, pac: str, user: str, role="maintainer"): """ add a new person to a package or project """ - path = quote_plus(prj), + path = (prj, ) kind = 'prj' if pac: - path = path + (quote_plus(pac),) + path = path + (pac ,) kind = 'pkg' data = meta_exists(metatype=kind, path_args=path, @@ -7912,10 +7911,10 @@ def delMaintainer(apiurl: str, prj: str, pac: str, user: str): def delPerson(apiurl: str, prj: str, pac: str, user: str, role="maintainer"): """ delete a person from a package or project """ - path = quote_plus(prj), + path = (prj, ) kind = 'prj' if pac: - path = path + (quote_plus(pac), ) + path = path + (pac, ) kind = 'pkg' data = meta_exists(metatype=kind, path_args=path, @@ -7941,10 +7940,10 @@ def delPerson(apiurl: str, prj: str, pac: str, user: str, role="maintainer"): def setBugowner(apiurl: str, prj: str, pac: str, user=None, group=None): """ delete all bugowners (user and group entries) and set one new one in a package or project """ - path = quote_plus(prj), + path = (prj, ) kind = 'prj' if pac: - path = path + (quote_plus(pac), ) + path = path + (pac, ) kind = 'pkg' data = meta_exists(metatype=kind, path_args=path, @@ -7974,7 +7973,7 @@ def setBugowner(apiurl: str, prj: str, pac: str, user=None, group=None): def setDevelProject(apiurl, prj, pac, dprj, dpkg=None): """ set the element to package metadata""" - path = (quote_plus(prj),) + (quote_plus(pac),) + path = (prj, pac) data = meta_exists(metatype='pkg', path_args=path, template_args=None, diff --git a/osc/fetch.py b/osc/fetch.py index 8ef1a79961..8ed75a58c7 100644 --- a/osc/fetch.py +++ b/osc/fetch.py @@ -10,7 +10,6 @@ import subprocess import sys import tempfile -from urllib.parse import quote_plus from urllib.request import HTTPError from . import checker as osc_checker @@ -51,10 +50,10 @@ def __add_cpio(self, pac): def __download_cpio_archive(self, apiurl, project, repo, arch, package, **pkgs): if not pkgs: return - query = [f'binary={quote_plus(i)}' for i in pkgs] - query.append('view=cpio') - for module in self.modules: - query.append(f"module={module}") + query = {} + query["binary"] = pkgs + query["view"] = "cpio" + query["module"] = self.modules try: url = makeurl(apiurl, ['build', project, repo, arch, package], query=query) sys.stdout.write("preparing download ...\r") diff --git a/tests/test_update.py b/tests/test_update.py index 846e50ae93..1c64e3308d 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -193,8 +193,8 @@ def testUpdateLimitSizeAddDelete(self): @GET('http://localhost/source/osctest/services?rev=latest', file='testUpdateServiceFilesAddDelete_filesremote') @GET('http://localhost/source/osctest/services/bigfile?rev=2', file='testUpdateServiceFilesAddDelete_bigfile') - @GET('http://localhost/source/osctest/services/_service%3Abar?rev=2', file='testUpdateServiceFilesAddDelete__service:bar') - @GET('http://localhost/source/osctest/services/_service%3Afoo?rev=2', file='testUpdateServiceFilesAddDelete__service:foo') + @GET('http://localhost/source/osctest/services/_service:bar?rev=2', file='testUpdateServiceFilesAddDelete__service:bar') + @GET('http://localhost/source/osctest/services/_service:foo?rev=2', file='testUpdateServiceFilesAddDelete__service:foo') @GET('http://localhost/source/osctest/services/_meta', file='meta.xml') def testUpdateAddDeleteServiceFiles(self): """update package with _service:* files"""