Use https and http appropriately when connecting to MusicBrainz

Fixed some bugs:
- MusicBrainz submit URL always has https as protocol: hardcoded, even when
inappropriate. It's just a graphical issue.
- Whipper appears to always communicate with MusicBrainz using musicbrainzngs
over http. The musicbrainzngs.set_hostname(server).
- `musicbrainzngs.set_hostname(server)` always defaults to http. Since musicbrainzngs
version 0.7 the method `set_hostname` takes an optional argument named `use_https`
(defaults to False) which whipper never passes.

Changed behaviour of `server` option (`musicbrainz` section of whipper's configuration file).
Now it expects an URL with a valid scheme (scheme must be `http` or `http`, empty scheme isn't allowed anymore).
Only the scheme and netloc parts of the URL are taken into account.

Fixes #437.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>

README.md

@@ -240,15 +240,17 @@ options:
 
 ```INI
 [main]
-path_filter_fat = True		; replace FAT file system unsafe characters in filenames with _
-path_filter_special = False	; replace special characters in filenames with _
+path_filter_fat = True			; replace FAT file system unsafe characters in filenames with _
+path_filter_special = False		; replace special characters in filenames with _
 
 [musicbrainz]
-server = musicbrainz.org:80	; use MusicBrainz server at host[:port]
+server = https://musicbrainz.org	; use MusicBrainz server at host[:port]
+# use http as scheme if connecting to a plain http server. Example below:
+# server = http://example.com:8080
 
 [drive:HL-20]
-defeats_cache = True		; whether the drive is capable of defeating the audio cache
-read_offset = 6			; drive read offset in positive/negative frames (no leading +)
+defeats_cache = True			; whether the drive is capable of defeating the audio cache
+read_offset = 6				; drive read offset in positive/negative frames (no leading +)
 # do not edit the values 'vendor', 'model', and 'release'; they are used by whipper to match the drive
 
 # command line defaults for `whipper cd rip`

whipper/command/main.py

@@ -20,7 +20,13 @@ logger = logging.getLogger(__name__)
 
 def main():
     server = config.Config().get_musicbrainz_server()
-    musicbrainzngs.set_hostname(server)
+    https_enabled = server['scheme'] == 'https'
+    try:
+        musicbrainzngs.set_hostname(server['netloc'], https_enabled)
+    # Parameter 'use_https' is missing in versions of musicbrainzngs < 0.7
+    except TypeError as e:
+        logger.warning(e)
+        musicbrainzngs.set_hostname(server['netloc'])
 
     # Find whipper's plugins paths (local paths have higher priority)
     plugins_p = [directory.data_path('plugins')]  # local path (in $HOME)

whipper/common/config.py

@@ -75,11 +75,12 @@ class Config:
     # musicbrainz section
 
     def get_musicbrainz_server(self):
-        server = self.get('musicbrainz', 'server') or 'musicbrainz.org'
-        server_url = urlparse('//' + server)
-        if server_url.scheme != '' or server_url.path != '':
-            raise KeyError('Invalid MusicBrainz server: %s' % server)
-        return server
+        conf = self.get('musicbrainz', 'server') or 'https://musicbrainz.org'
+        if not conf.startswith(('http://', 'https://')):
+            raise KeyError('Invalid MusicBrainz server: unsupported '
+                           'or missing scheme')
+        scheme, netloc, _, _, _, _ = urlparse(conf)
+        return {'scheme': scheme, 'netloc': netloc}
 
     # drive sections
 

whipper/image/table.py

@@ -351,7 +351,7 @@ class Table:
         return disc.id
 
     def getMusicBrainzSubmitURL(self):
-        host = config.Config().get_musicbrainz_server()
+        serv = config.Config().get_musicbrainz_server()
 
         discid = self.getMusicBrainzDiscId()
         values = self._getMusicBrainzValues()
@@ -363,7 +363,7 @@ class Table:
         ])
 
         return urlunparse((
-            'https', host, '/cdtoc/attach', '', query, ''))
+            serv['scheme'], serv['netloc'], '/cdtoc/attach', '', query, ''))
 
     def getFrameLength(self, data=False):
         """

whipper/test/test_common_config.py

@@ -69,25 +69,25 @@ class ConfigTestCase(tcommon.TestCase):
 
     def test_get_musicbrainz_server(self):
         self.assertEqual(self._config.get_musicbrainz_server(),
-                         'musicbrainz.org',
+                         {'scheme': 'https', 'netloc': 'musicbrainz.org'},
                          msg='Default value is correct')
 
         self._config._parser.add_section('musicbrainz')
 
         self._config._parser.set('musicbrainz', 'server',
-                                 '192.168.2.141:5000')
+                                 'http://192.168.2.141:5000')
         self._config.write()
         self.assertEqual(self._config.get_musicbrainz_server(),
-                         '192.168.2.141:5000',
+                         {'scheme': 'http', 'netloc': '192.168.2.141:5000'},
                          msg='Correctly returns user-set value')
 
-        self._config._parser.set('musicbrainz', 'server',
-                                 '192.168.2.141:5000/hello/world')
+        # Test for unsupported scheme
+        self._config._parser.set('musicbrainz', 'server', 'ftp://example.com')
         self._config.write()
         self.assertRaises(KeyError, self._config.get_musicbrainz_server)
 
-        self._config._parser.set('musicbrainz', 'server',
-                                 'http://192.168.2.141:5000')
+        # Test for absent scheme
+        self._config._parser.set('musicbrainz', 'server', 'example.com')
         self._config.write()
         self.assertRaises(KeyError, self._config.get_musicbrainz_server)