Skip to content

Commit

Permalink
Remove all path quoting, rely on makeurl()
Browse files Browse the repository at this point in the history
  • Loading branch information
@dmach
dmach committed Feb 1, 2024
1 parent 7f7bfa8 commit c1f7071
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 51 deletions.
8 changes: 4 additions & 4 deletions osc/build.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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, ),

Check warning on line 1024 in osc/build.py

View check run for this annotation

Codecov / codecov/patch

osc/build.py#L1024 

Added line #L1024 was not covered by tests
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),

Check warning on line 1029 in osc/build.py

View check run for this annotation

Codecov / codecov/patch

osc/build.py#L1029 

Added line #L1029 was not covered by tests
template_args=None, create_new=False,
apiurl=apiurl)
except:
pass
Expand Down
36 changes: 18 additions & 18 deletions osc/commandline.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=({
Expand All @@ -1958,28 +1958,28 @@ 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,
'user': conf.get_apiurl_usr(apiurl)}))
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':
Expand All @@ -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",
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)

Check warning on line 6279 in osc/commandline.py

View check run for this annotation

Codecov / codecov/patch

osc/commandline.py#L6279 

Added line #L6279 was not covered by tests
f = http_GET(u)
root = ET.parse(f).getroot()
offset = int(root.find('entry').get('size'))
Expand All @@ -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)

Check warning on line 6292 in osc/commandline.py

View check run for this annotation

Codecov / codecov/patch

osc/commandline.py#L6292 

Added line #L6292 was not covered by tests

def print_repos(self, repos_only=False, exc_class=oscerr.WrongArgs, exc_msg='Missing arguments', project=None):
wd = Path.cwd()
Expand Down Expand Up @@ -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)

Check warning on line 6373 in osc/commandline.py

View check run for this annotation

Codecov / codecov/patch

osc/commandline.py#L6373 

Added line #L6373 was not covered by tests
f = http_GET(u)
root = ET.parse(f).getroot()
offset = int(root.find('entry').get('size'))
Expand All @@ -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)

Check warning on line 6386 in osc/commandline.py

View check run for this annotation

Codecov / codecov/patch

osc/commandline.py#L6386 

Added line #L6386 was not covered by tests

def _find_last_repo_arch(self, repo=None, fatal=True):
files = glob.glob(os.path.join(Path.cwd(), store, "_buildinfo-*"))
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand Down Expand Up @@ -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)

Check warning on line 9257 in osc/commandline.py

View check run for this annotation

Codecov / codecov/patch

osc/commandline.py#L9257 

Added line #L9257 was not covered by tests
for buf in streamfile(u, http_GET, BUFSIZE):
o.write(buf)
o.close()
Expand Down
43 changes: 21 additions & 22 deletions osc/core.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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}),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Check warning on line 1544 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L1544 

Added line #L1544 was not covered by tests
http_DELETE(u)

def put_source_file(self, n, tdir, copy_only=False):
Expand All @@ -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)
Expand Down Expand Up @@ -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])

Check warning on line 5183 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L5183 

Added line #L5183 was not covered by tests
try:
f = http_GET(u)
return b''.join(f.readlines())
Expand All @@ -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())
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))

Check warning on line 5705 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L5705 

Added line #L5705 was not covered by tests
root = ET.fromstring(meta_data)
scmsync_element = root.find("scmsync")
if scmsync_element is not None and scmsync_element.text is not None:
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)

Check warning on line 6249 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L6249 

Added line #L6249 was not covered by tests
found = None
try:
found = http_GET(url).readlines()
Expand Down Expand Up @@ -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]

Check warning on line 6298 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L6298 

Added line #L6298 was not covered by tests
u = makeurl(dst_apiurl, path, query={'rev': 'repository'})
http_PUT(u, file=f.name)
tfilelist = Package.commit_filelist(dst_apiurl, dst_project, dst_package,
Expand Down Expand Up @@ -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, )

Check warning on line 7878 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L7878 

Added line #L7878 was not covered by tests
kind = 'prj'
if pac:
path = path + (quote_plus(pac),)
path = path + (pac ,)

Check warning on line 7881 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L7881 

Added line #L7881 was not covered by tests
kind = 'pkg'
data = meta_exists(metatype=kind,
path_args=path,
Expand Down Expand Up @@ -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, )

Check warning on line 7914 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L7914 

Added line #L7914 was not covered by tests
kind = 'prj'
if pac:
path = path + (quote_plus(pac), )
path = path + (pac, )

Check warning on line 7917 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L7917 

Added line #L7917 was not covered by tests
kind = 'pkg'
data = meta_exists(metatype=kind,
path_args=path,
Expand All @@ -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, )

Check warning on line 7943 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L7943 

Added line #L7943 was not covered by tests
kind = 'prj'
if pac:
path = path + (quote_plus(pac), )
path = path + (pac, )

Check warning on line 7946 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L7946 

Added line #L7946 was not covered by tests
kind = 'pkg'
data = meta_exists(metatype=kind,
path_args=path,
Expand Down Expand Up @@ -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 <devel project="..."> element to package metadata"""
path = (quote_plus(prj),) + (quote_plus(pac),)
path = (prj, pac)

Check warning on line 7976 in osc/core.py

View check run for this annotation

Codecov / codecov/patch

osc/core.py#L7976 

Added line #L7976 was not covered by tests
data = meta_exists(metatype='pkg',
path_args=path,
template_args=None,
Expand Down
9 changes: 4 additions & 5 deletions osc/fetch.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Check warning on line 56 in osc/fetch.py

View check run for this annotation

Codecov / codecov/patch

osc/fetch.py#L53-L56 

Added lines #L53 - L56 were not covered by tests
try:
url = makeurl(apiurl, ['build', project, repo, arch, package], query=query)
sys.stdout.write("preparing download ...\r")
Expand Down
4 changes: 2 additions & 2 deletions tests/test_update.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down

0 comments on commit c1f7071

Please sign in to comment.