From 9e740c11a4051d25fea5d95db40fdfcc743eb518 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Sun, 30 Jan 2022 15:31:09 +0100 Subject: [PATCH] Omit standard ports in URLs constructed by build_qurl --- picard/util/__init__.py | 7 ++++--- test/test_browser.py | 3 ++- test/test_disc.py | 4 ++-- test/test_util_mbserver.py | 12 ++++++------ test/test_utils.py | 29 +++++++++++++++++++++++++++++ 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/picard/util/__init__.py b/picard/util/__init__.py index b6c59bf7c..a37efa584 100644 --- a/picard/util/__init__.py +++ b/picard/util/__init__.py @@ -580,13 +580,14 @@ def build_qurl(host, port=80, path=None, queryargs=None): """ url = QtCore.QUrl() url.setHost(host) - url.setPort(port) - if host in MUSICBRAINZ_SERVERS or port == 443: + if port == 443 or host in MUSICBRAINZ_SERVERS: url.setScheme("https") - url.setPort(443) + elif port == 80: + url.setScheme("http") else: url.setScheme("http") + url.setPort(port) if path is not None: url.setPath(path) diff --git a/test/test_browser.py b/test/test_browser.py index d8dd7f965..1e5336930 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -50,8 +50,9 @@ class BrowserLookupTest(PicardTestCase): def assert_mb_url_matches(self, url, path, query_args=None): parsed_url = urlparse(url) - expected_host = "%s:%s" % (SERVER, PORT) + expected_host = SERVER self.assertEqual(expected_host, parsed_url.netloc, '"%s" hostname does not match "%s"' % (url, expected_host)) + self.assertEqual('https' if PORT == 443 else 'http', parsed_url.scheme) self.assertEqual(path, parsed_url.path, '"%s" path does not match "%s"' % (url, path)) if query_args is not None: actual_query_args = {k: v[0] for k, v in parse_qs(parsed_url.query).items()} diff --git a/test/test_disc.py b/test/test_disc.py index c01d0af83..b99dd9bd3 100644 --- a/test/test_disc.py +++ b/test/test_disc.py @@ -39,7 +39,7 @@ class MockDisc: mcn = '5029343070452' tracks = list(range(0, 11)) toc_string = ' '.join(str(i) for i in test_toc) - submission_url = 'https://musicbrainz.org:443/cdtoc/attach?id=lSOVc5h6IXSuzcamJS1Gp4_tRuA-&tracks=11&toc=1+11+242457+150+44942+61305+72755+96360+130485+147315+164275+190702+205412+220437' + submission_url = 'https://musicbrainz.org/cdtoc/attach?id=lSOVc5h6IXSuzcamJS1Gp4_tRuA-&tracks=11&toc=1+11+242457+150+44942+61305+72755+96360+130485+147315+164275+190702+205412+220437' class DiscTest(PicardTestCase): @@ -112,6 +112,6 @@ class DiscTest(PicardTestCase): disc = picard.disc.Disc() disc.read() self.assertEqual( - 'http://test.musicbrainz.org:80/cdtoc/attach?id=lSOVc5h6IXSuzcamJS1Gp4_tRuA-&tracks=11&toc=1+11+242457+150+44942+61305+72755+96360+130485+147315+164275+190702+205412+220437', + 'http://test.musicbrainz.org/cdtoc/attach?id=lSOVc5h6IXSuzcamJS1Gp4_tRuA-&tracks=11&toc=1+11+242457+150+44942+61305+72755+96360+130485+147315+164275+190702+205412+220437', disc.submission_url ) diff --git a/test/test_util_mbserver.py b/test/test_util_mbserver.py index e0f193585..91574737b 100644 --- a/test/test_util_mbserver.py +++ b/test/test_util_mbserver.py @@ -90,9 +90,9 @@ class BuildSubmissionUrlTest(PicardTestCase): 'server_port': 80, 'use_server_for_submission': False, }) - self.assertEqual('https://%s:443' % host, build_submission_url()) - self.assertEqual('https://%s:443/' % host, build_submission_url('/')) - self.assertEqual('https://%s:443/some/path?foo=1&bar=baz' % host, + self.assertEqual('https://%s' % host, build_submission_url()) + self.assertEqual('https://%s/' % host, build_submission_url('/')) + self.assertEqual('https://%s/some/path?foo=1&bar=baz' % host, build_submission_url('/some/path', {'foo': 1, 'bar': 'baz'})) def test_use_unofficial(self): @@ -112,7 +112,7 @@ class BuildSubmissionUrlTest(PicardTestCase): 'server_port': 80, 'use_server_for_submission': False, }) - self.assertEqual('https://musicbrainz.org:443', build_submission_url()) - self.assertEqual('https://musicbrainz.org:443/', build_submission_url('/')) - self.assertEqual('https://musicbrainz.org:443/some/path?foo=1&bar=baz', + self.assertEqual('https://musicbrainz.org', build_submission_url()) + self.assertEqual('https://musicbrainz.org/', build_submission_url('/')) + self.assertEqual('https://musicbrainz.org/some/path?foo=1&bar=baz', build_submission_url('/some/path', {'foo': 1, 'bar': 'baz'})) diff --git a/test/test_utils.py b/test/test_utils.py index 0c861b7b0..fbc1e1256 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -42,12 +42,14 @@ from unittest.mock import Mock from test.picardtestcase import PicardTestCase from picard import util +from picard.const import MUSICBRAINZ_SERVERS from picard.const.sys import ( IS_MACOS, IS_WIN, ) from picard.util import ( album_artist_from_path, + build_qurl, extract_year_from_date, find_best_match, is_absolute_path, @@ -671,3 +673,30 @@ class WildcardsToRegexPatternTest(PicardTestCase): regex = wildcards_to_regex_pattern(pattern) self.assertEqual(re.escape(pattern), regex) re.compile(regex) + + +class BuildQUrlTest(PicardTestCase): + + def test_path_and_querystring(self): + query = {'foo': 'x', 'bar': 'y'} + self.assertEqual('http://example.com/', build_qurl('example.com', path='/').toDisplayString()) + self.assertEqual('http://example.com/foo/bar', build_qurl('example.com', path='/foo/bar').toDisplayString()) + self.assertEqual('http://example.com/foo/bar?foo=x&bar=y', build_qurl('example.com', path='/foo/bar', queryargs=query).toDisplayString()) + self.assertEqual('http://example.com?foo=x&bar=y', build_qurl('example.com', queryargs=query).toDisplayString()) + + def test_standard_ports(self): + self.assertEqual('http://example.com', build_qurl('example.com').toDisplayString()) + self.assertEqual('http://example.com', build_qurl('example.com', port=80).toDisplayString()) + self.assertEqual('https://example.com', build_qurl('example.com', port=443).toDisplayString()) + + def test_custom_port(self): + self.assertEqual('http://example.com:8080', build_qurl('example.com', port=8080).toDisplayString()) + self.assertEqual('http://example.com:8080/', build_qurl('example.com', port=8080, path="/").toDisplayString()) + self.assertEqual('http://example.com:8080?foo=x', build_qurl('example.com', port=8080, queryargs={'foo': 'x'}).toDisplayString()) + + def test_mb_server(self): + for host in MUSICBRAINZ_SERVERS: + expected = 'https://' + host + self.assertEqual(expected, build_qurl(host, port=80).toDisplayString()) + self.assertEqual(expected, build_qurl(host, port=443).toDisplayString()) + self.assertEqual(expected, build_qurl(host, port=8080).toDisplayString())