From fb627c84bc174e3c63352ea73c3526eebf2e836f Mon Sep 17 00:00:00 2001 From: Wieland Hoffmann Date: Mon, 5 Oct 2015 23:18:57 +0200 Subject: [PATCH 1/6] Move all the QUrl building to util.build_qurl --- contrib/plugins/albumartist_website.py | 6 +- picard/browser/filelookup.py | 15 ++--- picard/oauth.py | 10 +-- picard/util/__init__.py | 21 ++++++ picard/webservice.py | 91 +++++++++++++++----------- 5 files changed, 87 insertions(+), 56 deletions(-) diff --git a/contrib/plugins/albumartist_website.py b/contrib/plugins/albumartist_website.py index f9273e6de..492dbed08 100644 --- a/contrib/plugins/albumartist_website.py +++ b/contrib/plugins/albumartist_website.py @@ -77,10 +77,12 @@ class AlbumArtistWebsite: if self.website_queue.append(artistId, (track, album)): host = config.setting["server_host"] port = config.setting["server_port"] - path = "/ws/2/%s/%s?inc=%s" % ('artist', artistId, 'url-rels') + path = "/ws/2/%s/%s" % ('artist', artistId) + queryargs = {"inc": "url-rels"} return album.tagger.xmlws.get(host, port, path, partial(self.website_process, artistId), - xml=True, priority=True, important=False) + xml=True, priority=True, important=False, + queryargs=queryargs) def website_process(self, artistId, response, reply, error): if error: diff --git a/picard/browser/filelookup.py b/picard/browser/filelookup.py index 398faa42f..7c91151bf 100644 --- a/picard/browser/filelookup.py +++ b/picard/browser/filelookup.py @@ -22,7 +22,7 @@ from PyQt4 import QtCore import os.path import re from picard import log -from picard.util import webbrowser2 +from picard.util import webbrowser2, build_qurl class FileLookup(object): @@ -32,11 +32,8 @@ class FileLookup(object): self.localPort = int(localPort) self.port = port - def _url(self, path, params={}, scheme='http'): - url = QtCore.QUrl() - url.setScheme(scheme if self.port != 443 else 'https') - url.setHost(self.server) - url.setPort(self.port if scheme != 'https' else 443) + def _url(self, path, params={}): + url = build_qurl(self.server, self.port) url.setPath(path) if self.localPort: params['tport'] = self.localPort @@ -44,8 +41,8 @@ class FileLookup(object): url.addQueryItem(k, unicode(v)) return url.toEncoded() - def _build_launch(self, path, params={}, scheme='http'): - return self.launch(self._url(path, params, scheme)) + def _build_launch(self, path, params={}): + return self.launch(self._url(path, params)) def launch(self, url): log.debug("webbrowser2: %s" % url) @@ -123,4 +120,4 @@ class FileLookup(object): return self._build_launch('/taglookup', params) def collectionLookup(self, userid): - return self._build_launch('/user/%s/collections' % userid, scheme='https') + return self._build_launch('/user/%s/collections' % userid) diff --git a/picard/oauth.py b/picard/oauth.py index 73a0aff5f..387615c85 100644 --- a/picard/oauth.py +++ b/picard/oauth.py @@ -28,6 +28,7 @@ from picard.const import ( MUSICBRAINZ_OAUTH_CLIENT_ID, MUSICBRAINZ_OAUTH_CLIENT_SECRET, ) +from picard.util import build_qurl class OAuthManager(object): @@ -69,14 +70,7 @@ class OAuthManager(object): def get_authorization_url(self, scopes): host, port = config.setting['server_host'], config.setting['server_port'] - url = QUrl() - if (host in MUSICBRAINZ_SERVERS and port == 80) or port == 443: - url.setScheme("https") - else: - url.setScheme("http") - if port != 80: - url.setPort(port) - url.setHost(host) + url = build_qurl(host, port) url.setPath("/oauth2/authorize") url.addQueryItem("response_type", "code") url.addQueryItem("client_id", MUSICBRAINZ_OAUTH_CLIENT_ID) diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 616f1671f..ffce7cd60 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -31,6 +31,7 @@ from PyQt4 import QtCore from string import Template # Required for compatibility with lastfmplus which imports this from here rather than loading it direct. from functools import partial +from picard.const import MUSICBRAINZ_SERVERS class LockableObject(QtCore.QObject): @@ -380,3 +381,23 @@ def album_artist_from_path(filename, album, artist): elif not artist and len(dirs) >= 2: artist = dirs[-2] return album, artist + + +def build_qurl(host, port=80, mblogin=False): + """ + Builds and returns a QUrl object from `host` and `port` and + automatically enables HTTPS if necessary. + + Setting `mblogin` to True forces HTTPS. + """ + url = QtCore.QUrl() + url.setHost(host) + url.setPort(port) + if (mblogin or # Login is required + port == 443 or # HTTPS port + (host in MUSICBRAINZ_SERVERS and port == 80)): # We know MB supports HTTPS + url.setScheme("https") + url.setPort(443) + else: + url.setScheme("http") + return url diff --git a/picard/webservice.py b/picard/webservice.py index 2e0bb20e7..0c6d3eef9 100644 --- a/picard/webservice.py +++ b/picard/webservice.py @@ -46,6 +46,7 @@ from picard.const import (ACOUSTID_KEY, CAA_PORT, MUSICBRAINZ_SERVERS) from picard.oauth import OAuthManager +from picard.util import build_qurl COUNT_REQUESTS_DELAY_MS = 250 @@ -188,15 +189,12 @@ class XmlWebService(QtCore.QObject): def _start_request_continue(self, method, host, port, path, data, handler, xml, mblogin=False, cacheloadcontrol=None, refresh=None, - access_token=None): - if (mblogin and host in MUSICBRAINZ_SERVERS and port == 80) or port == 443: - urlstring = "https://%s%s" % (host, path) - elif port is None or port == 80: - urlstring = "http://%s%s" % (host, path) - else: - urlstring = "http://%s:%d%s" % (host, port, path) - log.debug("%s %s", method, urlstring) - url = QUrl.fromEncoded(urlstring) + access_token=None, queryargs=None): + url = build_qurl(host, port) + url.setPath(path) + if queryargs is not None: + qargs_list = [(k, unicode(v)) for k, v in queryargs.iteritems()] + url.setEncodedQueryItems(qargs_list) request = QtNetwork.QNetworkRequest(url) if mblogin and access_token: request.setRawHeader("Authorization", "Bearer %s" % access_token) @@ -223,12 +221,13 @@ class XmlWebService(QtCore.QObject): self._active_requests[reply] = (request, handler, xml, refresh) def _start_request(self, method, host, port, path, data, handler, xml, - mblogin=False, cacheloadcontrol=None, refresh=None): + mblogin=False, cacheloadcontrol=None, refresh=None, + queryargs=None): def start_request_continue(access_token=None): self._start_request_continue( method, host, port, path, data, handler, xml, mblogin=mblogin, cacheloadcontrol=cacheloadcontrol, refresh=refresh, - access_token=access_token) + access_token=access_token, queryargs=queryargs) if mblogin and path != "/oauth2/token": self.oauth_manager.get_access_token(start_request_continue) else: @@ -314,22 +313,22 @@ class XmlWebService(QtCore.QObject): reply.deleteLater() def get(self, host, port, path, handler, xml=True, priority=False, - important=False, mblogin=False, cacheloadcontrol=None, refresh=False): + important=False, mblogin=False, cacheloadcontrol=None, refresh=False, queryargs=None): func = partial(self._start_request, "GET", host, port, path, None, - handler, xml, mblogin, cacheloadcontrol=cacheloadcontrol, refresh=refresh) + handler, xml, mblogin, cacheloadcontrol=cacheloadcontrol, refresh=refresh, queryargs=queryargs) return self.add_task(func, host, port, priority, important=important) - def post(self, host, port, path, data, handler, xml=True, priority=False, important=False, mblogin=True): + def post(self, host, port, path, data, handler, xml=True, priority=False, important=False, mblogin=True, queryargs=None): log.debug("POST-DATA %r", data) - func = partial(self._start_request, "POST", host, port, path, data, handler, xml, mblogin) + func = partial(self._start_request, "POST", host, port, path, data, handler, xml, mblogin, queryargs=queryargs) return self.add_task(func, host, port, priority, important=important) - def put(self, host, port, path, data, handler, priority=True, important=False, mblogin=True): - func = partial(self._start_request, "PUT", host, port, path, data, handler, False, mblogin) + def put(self, host, port, path, data, handler, priority=True, important=False, mblogin=True, queryargs=None): + func = partial(self._start_request, "PUT", host, port, path, data, handler, False, mblogin, queryargs=queryargs) return self.add_task(func, host, port, priority, important=important) - def delete(self, host, port, path, handler, priority=True, important=False, mblogin=True): - func = partial(self._start_request, "DELETE", host, port, path, None, handler, False, mblogin) + def delete(self, host, port, path, handler, priority=True, important=False, mblogin=True, queryargs=None): + func = partial(self._start_request, "DELETE", host, port, path, None, handler, False, mblogin, queryargs=queryargs) return self.add_task(func, host, port, priority, important=important) def stop(self): @@ -420,15 +419,18 @@ class XmlWebService(QtCore.QObject): except: pass - def _get_by_id(self, entitytype, entityid, handler, inc=[], params=[], + def _get_by_id(self, entitytype, entityid, handler, inc=[], queryargs=None, priority=False, important=False, mblogin=False, refresh=False): host = config.setting["server_host"] port = config.setting["server_port"] - path = "/ws/2/%s/%s?inc=%s" % (entitytype, entityid, "+".join(inc)) - if params: - path += "&" + "&".join(params) + path = "/ws/2/%s/%s" % (entitytype, entityid) + if queryargs is None: + queryargs = {} + if len(inc) > 0: + queryargs["inc"] = "+".join(inc) return self.get(host, port, path, handler, - priority=priority, important=important, mblogin=mblogin, refresh=refresh) + priority=priority, important=important, mblogin=mblogin, + refresh=refresh, queryargs=queryargs) def get_release_by_id(self, releaseid, handler, inc=[], priority=False, important=False, mblogin=False, refresh=False): @@ -442,7 +444,7 @@ class XmlWebService(QtCore.QObject): def lookup_discid(self, discid, handler, priority=True, important=True, refresh=False): inc = ['artist-credits', 'labels'] - return self._get_by_id('discid', discid, handler, inc, params=["cdstubs=no"], + return self._get_by_id('discid', discid, handler, inc, queryargs={"cdstubs": "no"}, priority=priority, important=important, refresh=refresh) def _find(self, entitytype, handler, kwargs): @@ -459,12 +461,12 @@ class XmlWebService(QtCore.QObject): query.append('%s:(%s)' % (name, value)) if query: filters.append(('query', ' '.join(query))) - params = [] + queryargs = {} for name, value in filters: value = QUrl.toPercentEncoding(unicode(value)) - params.append('%s=%s' % (str(name), value)) - path = "/ws/2/%s/?%s" % (entitytype, "&".join(params)) - return self.get(host, port, path, handler) + queryargs[str(name)] = value + path = "/ws/2/%s" % (entitytype) + return self.get(host, port, path, handler, queryargs=queryargs) def find_releases(self, handler, **kwargs): return self._find('release', handler, kwargs) @@ -475,9 +477,12 @@ class XmlWebService(QtCore.QObject): def _browse(self, entitytype, handler, kwargs, inc=[], priority=False, important=False): host = config.setting["server_host"] port = config.setting["server_port"] - params = "&".join(["%s=%s" % (k, v) for k, v in kwargs.items()]) - path = "/ws/2/%s?%s&inc=%s" % (entitytype, params, "+".join(inc)) - return self.get(host, port, path, handler, priority=priority, important=important) + path = "/ws/2/%s" % (entitytype) + queryargs = kwargs + if len(inc) > 0: + queryargs["inc"] = "+".join(inc) + return self.get(host, port, path, handler, priority=priority, + important=important, queryargs=queryargs) def browse_releases(self, handler, priority=True, important=True, **kwargs): inc = ["media", "labels"] @@ -527,10 +532,16 @@ class XmlWebService(QtCore.QObject): def get_collection(self, id, handler, limit=100, offset=0): host, port = config.setting['server_host'], config.setting['server_port'] path = "/ws/2/collection" + queryargs = None if id is not None: inc = ["releases", "artist-credits", "media"] - path += "/%s/releases?inc=%s&limit=%d&offset=%d" % (id, "+".join(inc), limit, offset) - return self.get(host, port, path, handler, priority=True, important=True, mblogin=True) + path += "/%s/releases" % (id) + queryargs = {} + queryargs["inc"] = "+".join(inc) + queryargs["limit"] = limit + queryargs["offset"] = offset + return self.get(host, port, path, handler, priority=True, important=True, + mblogin=True, queryargs=queryargs) def get_collection_list(self, handler): return self.get_collection(None, handler) @@ -539,14 +550,20 @@ class XmlWebService(QtCore.QObject): while releases: ids = ";".join(releases if len(releases) <= 400 else releases[:400]) releases = releases[400:] - yield "/ws/2/collection/%s/releases/%s?client=%s" % (id, ids, CLIENT_STRING) + yield "/ws/2/collection/%s/releases/%s" % (id, ids) + + def _get_client_queryarg(self): + return {"client": CLIENT_STRING} + def put_to_collection(self, id, releases, handler): host, port = config.setting['server_host'], config.setting['server_port'] for path in self._collection_request(id, releases): - self.put(host, port, path, "", handler) + self.put(host, port, path, "", handler, + queryargs=self._get_client_queryarg()) def delete_from_collection(self, id, releases, handler): host, port = config.setting['server_host'], config.setting['server_port'] for path in self._collection_request(id, releases): - self.delete(host, port, path, handler) + self.delete(host, port, path, handler, + queryargs=self._get_client_queryarg) From 8ebe83b69e0bd83773dc10df2afaa6d38da3361e Mon Sep 17 00:00:00 2001 From: Wieland Hoffmann Date: Tue, 6 Oct 2015 10:35:46 +0200 Subject: [PATCH 2/6] Move adding query arguments and the path to build_qurl --- picard/browser/filelookup.py | 5 +---- picard/oauth.py | 11 +++++------ picard/util/__init__.py | 13 +++++++++++-- picard/webservice.py | 7 ++----- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/picard/browser/filelookup.py b/picard/browser/filelookup.py index 7c91151bf..1403d3406 100644 --- a/picard/browser/filelookup.py +++ b/picard/browser/filelookup.py @@ -33,12 +33,9 @@ class FileLookup(object): self.port = port def _url(self, path, params={}): - url = build_qurl(self.server, self.port) - url.setPath(path) if self.localPort: params['tport'] = self.localPort - for k, v in params.iteritems(): - url.addQueryItem(k, unicode(v)) + url = build_qurl(self.server, self.port, path=path, queryargs=params) return url.toEncoded() def _build_launch(self, path, params={}): diff --git a/picard/oauth.py b/picard/oauth.py index 387615c85..2ec151200 100644 --- a/picard/oauth.py +++ b/picard/oauth.py @@ -70,12 +70,11 @@ class OAuthManager(object): def get_authorization_url(self, scopes): host, port = config.setting['server_host'], config.setting['server_port'] - url = build_qurl(host, port) - url.setPath("/oauth2/authorize") - url.addQueryItem("response_type", "code") - url.addQueryItem("client_id", MUSICBRAINZ_OAUTH_CLIENT_ID) - url.addQueryItem("redirect_uri", "urn:ietf:wg:oauth:2.0:oob") - url.addQueryItem("scope", scopes) + params = {"response_type": "code", "client_id": + MUSICBRAINZ_OAUTH_CLIENT_ID, "redirect_uri": + "urn:ietf:wg:oauth:2.0:oob", "scope": scopes} + url = build_qurl(host, port, path="/oauth2/authorize", + queryargs=params) return str(url.toEncoded()) def set_refresh_token(self, refresh_token, scopes): diff --git a/picard/util/__init__.py b/picard/util/__init__.py index ffce7cd60..5f968cea7 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -383,12 +383,15 @@ def album_artist_from_path(filename, album, artist): return album, artist -def build_qurl(host, port=80, mblogin=False): +def build_qurl(host, port=80, path=None, mblogin=False, queryargs=None): """ - Builds and returns a QUrl object from `host` and `port` and + Builds and returns a QUrl object from `host`, `port` and `path` and automatically enables HTTPS if necessary. Setting `mblogin` to True forces HTTPS. + + Encoded query arguments can be provided in `queryargs`, a + dictionary mapping field names to values. """ url = QtCore.QUrl() url.setHost(host) @@ -400,4 +403,10 @@ def build_qurl(host, port=80, mblogin=False): url.setPort(443) else: url.setScheme("http") + + if path is not None: + url.setPath(path) + if queryargs is not None: + for k, v in queryargs.iteritems(): + url.addEncodedQueryItem(k, unicode(v)) return url diff --git a/picard/webservice.py b/picard/webservice.py index 0c6d3eef9..5ae6decd9 100644 --- a/picard/webservice.py +++ b/picard/webservice.py @@ -190,11 +190,8 @@ class XmlWebService(QtCore.QObject): def _start_request_continue(self, method, host, port, path, data, handler, xml, mblogin=False, cacheloadcontrol=None, refresh=None, access_token=None, queryargs=None): - url = build_qurl(host, port) - url.setPath(path) - if queryargs is not None: - qargs_list = [(k, unicode(v)) for k, v in queryargs.iteritems()] - url.setEncodedQueryItems(qargs_list) + url = build_qurl(host, port, path=path, mblogin=mblogin, + queryargs=queryargs) request = QtNetwork.QNetworkRequest(url) if mblogin and access_token: request.setRawHeader("Authorization", "Bearer %s" % access_token) From 25390410dac79c5b2789d9244ed3e859952979e9 Mon Sep 17 00:00:00 2001 From: Wieland Hoffmann Date: Tue, 6 Oct 2015 10:57:47 +0200 Subject: [PATCH 3/6] Port the plugin downloading process to build_qurl as well --- picard/ui/options/plugins.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/picard/ui/options/plugins.py b/picard/ui/options/plugins.py index 9e918fffb..d88eea423 100644 --- a/picard/ui/options/plugins.py +++ b/picard/ui/options/plugins.py @@ -314,11 +314,12 @@ class PluginsOptionsPage(OptionsPage): self.tagger.xmlws.get( PLUGINS_API['host'], PLUGINS_API['port'], - PLUGINS_API['endpoint']['download'] + "?id=" + plugin.module_name, + PLUGINS_API['endpoint']['download'], partial(self.download_handler, plugin=plugin), xml=False, priority=True, - important=True + important=True, + queryargs={"id": plugin.module_name} ) def download_handler(self, response, reply, error, plugin): From 78430c250e24f592de94ce75920c4a03a3ef4f88 Mon Sep 17 00:00:00 2001 From: Wieland Hoffmann Date: Tue, 6 Oct 2015 12:34:58 +0200 Subject: [PATCH 4/6] Don't force HTTPS for every call to MB --- picard/oauth.py | 2 +- picard/util/__init__.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/picard/oauth.py b/picard/oauth.py index 2ec151200..f88a2534a 100644 --- a/picard/oauth.py +++ b/picard/oauth.py @@ -74,7 +74,7 @@ class OAuthManager(object): MUSICBRAINZ_OAUTH_CLIENT_ID, "redirect_uri": "urn:ietf:wg:oauth:2.0:oob", "scope": scopes} url = build_qurl(host, port, path="/oauth2/authorize", - queryargs=params) + queryargs=params, mblogin=True) return str(url.toEncoded()) def set_refresh_token(self, refresh_token, scopes): diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 5f968cea7..042e5c153 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -396,9 +396,10 @@ def build_qurl(host, port=80, path=None, mblogin=False, queryargs=None): url = QtCore.QUrl() url.setHost(host) url.setPort(port) - if (mblogin or # Login is required - port == 443 or # HTTPS port - (host in MUSICBRAINZ_SERVERS and port == 80)): # We know MB supports HTTPS + if (# Login is required and we're contacting an MB server + (mblogin and host in MUSICBRAINZ_SERVERS and port == 80) or + # Or we're contacting some other server via HTTPS. + port == 443): url.setScheme("https") url.setPort(443) else: From 77482ae6cb3c1d687a533eeaaf0732e66dc69c9f Mon Sep 17 00:00:00 2001 From: Wieland Hoffmann Date: Tue, 6 Oct 2015 12:39:40 +0200 Subject: [PATCH 5/6] Clarify the effect of mb_login --- picard/util/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/picard/util/__init__.py b/picard/util/__init__.py index 042e5c153..a0f7a3613 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -388,7 +388,7 @@ def build_qurl(host, port=80, path=None, mblogin=False, queryargs=None): Builds and returns a QUrl object from `host`, `port` and `path` and automatically enables HTTPS if necessary. - Setting `mblogin` to True forces HTTPS. + Setting `mblogin` to True forces HTTPS on MusicBrainz' servers. Encoded query arguments can be provided in `queryargs`, a dictionary mapping field names to values. From 1645e1c5c799fab2035e410761f19a8f733d01e8 Mon Sep 17 00:00:00 2001 From: Wieland Hoffmann Date: Tue, 6 Oct 2015 12:57:30 +0200 Subject: [PATCH 6/6] =?UTF-8?q?if=20len(obj)=20>=200=20=E2=86=92=20if=20ob?= =?UTF-8?q?j?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- picard/webservice.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/picard/webservice.py b/picard/webservice.py index 5ae6decd9..0d22d73e2 100644 --- a/picard/webservice.py +++ b/picard/webservice.py @@ -423,7 +423,7 @@ class XmlWebService(QtCore.QObject): path = "/ws/2/%s/%s" % (entitytype, entityid) if queryargs is None: queryargs = {} - if len(inc) > 0: + if inc: queryargs["inc"] = "+".join(inc) return self.get(host, port, path, handler, priority=priority, important=important, mblogin=mblogin, @@ -476,7 +476,7 @@ class XmlWebService(QtCore.QObject): port = config.setting["server_port"] path = "/ws/2/%s" % (entitytype) queryargs = kwargs - if len(inc) > 0: + if inc: queryargs["inc"] = "+".join(inc) return self.get(host, port, path, handler, priority=priority, important=important, queryargs=queryargs)