Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check python version and enable https where it's possible #2307

Merged
merged 22 commits into from
Jan 3, 2017
Merged

Check python version and enable https where it's possible #2307

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
43fcb3d
Check python version and enable https where it's possible
tigranl Dec 5, 2016
6ba5099
Python version check for lyrics.py
tigranl Dec 6, 2016
0fb19cf
Merge branch 'master' of github.com:beetbox/beets into https_fix
tigranl Dec 6, 2016
f60c911
Add SNI_SUPPORTED variable for https check
tigranl Dec 6, 2016
73a7a4f
Add SNI_SUPPORTED variable for https check
tigranl Dec 6, 2016
d065b33
Add SNI_SUPPORTED variable for https check
tigranl Dec 6, 2016
5ae1376
Add SNI_SUPPORTED variable for https check
tigranl Dec 6, 2016
25ebf89
Revert "Add SNI_SUPPORTED variable for https check"
tigranl Dec 6, 2016
91819b2
Add SNI_SUPPORTED
tigranl Dec 6, 2016
efa9041
Add SNI_SUPPORTED
tigranl Dec 6, 2016
420c451
Add SNI_SUPPORTED
tigranl Dec 6, 2016
9bba178
Add tests for https
tigranl Dec 6, 2016
21208b8
Add SNI_SUPPORTED
tigranl Dec 8, 2016
b65a7da
Add SNI_SUPPORTED
tigranl Dec 8, 2016
68b4a03
Add tests for https
tigranl Dec 10, 2016
0868299
PEP8 corrections
tigranl Dec 10, 2016
471f875
Fix typo
tigranl Dec 10, 2016
5ca664e
Fix typos
tigranl Dec 10, 2016
dd115b1
Add ui import
tigranl Dec 10, 2016
1c5c74f
Fix a typo
tigranl Dec 31, 2016
f941fd4
Always use SSL on servers that don't require SNI
sampsyo Jan 3, 2017
33a8e81
Simplify test changes
sampsyo Jan 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion beets/autotag/mb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,14 @@
from beets import util
from beets import config
import six
import sys

VARIOUS_ARTISTS_ID = '89ad4ac3-39f7-470e-963a-56509c546377'
BASE_URL = 'http://musicbrainz.org/'

if util.SNI_SUPPORTED:
BASE_URL = 'https://musicbrainz.org/'
else:
BASE_URL = 'http://musicbrainz.org/'

musicbrainzngs.set_useragent('beets', beets.__version__,
'http://beets.io/')
Expand Down
1 change: 1 addition & 0 deletions beets/util/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

MAX_FILENAME_LENGTH = 200
WINDOWS_MAGIC_PREFIX = u'\\\\?\\'
SNI_SUPPORTED = sys.version_info >= (2, 7, 9)


class HumanReadableException(Exception):
Expand Down
5 changes: 4 additions & 1 deletion beets/util/artresizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@
IMAGEMAGICK = 2
WEBPROXY = 3

PROXY_URL = 'http://images.weserv.nl/'
if util.SNI_SUPPORTED:
PROXY_URL = 'https://images.weserv.nl/'
else:
PROXY_URL = 'http://images.weserv.nl/'

log = logging.getLogger('beets')

Expand Down
32 changes: 24 additions & 8 deletions beetsplug/fetchart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,12 @@ def fetch_image(self, candidate, extra):
class CoverArtArchive(RemoteArtSource):
NAME = u"Cover Art Archive"

URL = 'http://coverartarchive.org/release/{mbid}/front'
GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front'
if util.SNI_SUPPORTED:
URL = 'https://coverartarchive.org/release/{mbid}/front'
GROUP_URL = 'https://coverartarchive.org/release-group/{mbid}/front'
else:
URL = 'http://coverartarchive.org/release/{mbid}/front'
GROUP_URL = 'http://coverartarchive.org/release-group/{mbid}/front'

def get(self, album, extra):
"""Return the Cover Art Archive and Cover Art Archive release group URLs
Expand All @@ -310,7 +314,10 @@ def get(self, album, extra):

class Amazon(RemoteArtSource):
NAME = u"Amazon"
URL = 'http://images.amazon.com/images/P/%s.%02i.LZZZZZZZ.jpg'
if util.SNI_SUPPORTED:
URL = 'https://images.amazon.com/images/P/%s.%02i.LZZZZZZZ.jpg'
else:
URL = 'http://images.amazon.com/images/P/%s.%02i.LZZZZZZZ.jpg'
INDICES = (1, 2)

def get(self, album, extra):
Expand All @@ -324,7 +331,10 @@ def get(self, album, extra):

class AlbumArtOrg(RemoteArtSource):
NAME = u"AlbumArt.org scraper"
URL = 'http://www.albumart.org/index_detail.php'
if util.SNI_SUPPORTED:
URL = 'https://www.albumart.org/index_detail.php'
else:
URL = 'http://www.albumart.org/index_detail.php'
PAT = r'href\s*=\s*"([^>"]*)"[^>]*title\s*=\s*"View larger image"'

def get(self, album, extra):
Expand Down Expand Up @@ -394,8 +404,10 @@ def get(self, album, extra):
class FanartTV(RemoteArtSource):
"""Art from fanart.tv requested using their API"""
NAME = u"fanart.tv"

API_URL = 'http://webservice.fanart.tv/v3/'
if util.SNI_SUPPORTED:
API_URL = 'https://webservice.fanart.tv/v3/'
else:
API_URL = 'htts://webservice.fanart.tv/v3/'
API_ALBUMS = API_URL + 'music/albums/'
PROJECT_KEY = '61a7d0ab4e67162b7a0c7c35915cd48e'

Expand Down Expand Up @@ -488,8 +500,12 @@ def get(self, album, extra):

class Wikipedia(RemoteArtSource):
NAME = u"Wikipedia (queried through DBpedia)"
DBPEDIA_URL = 'http://dbpedia.org/sparql'
WIKIPEDIA_URL = 'http://en.wikipedia.org/w/api.php'
if util.SNI_SUPPORTED:
DBPEDIA_URL = 'https://dbpedia.org/sparql'
WIKIPEDIA_URL = 'https://en.wikipedia.org/w/api.php'
else:
DBPEDIA_URL = 'http://dbpedia.org/sparql'
WIKIPEDIA_URL = 'http://en.wikipedia.org/w/api.php'
SPARQL_QUERY = u'''PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>
PREFIX dbpprop: <http://dbpedia.org/property/>
PREFIX owl: <http://dbpedia.org/ontology/>
Expand Down
7 changes: 5 additions & 2 deletions beetsplug/lastimport.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@

import pylast
from pylast import TopItem, _extract, _number
from beets import ui
from beets import util
from beets import dbcore
from beets import config
from beets import plugins
from beets.dbcore import types

API_URL = 'http://ws.audioscrobbler.com/2.0/'
if util.SNI_SUPPORTED:
API_URL = 'https://ws.audioscrobbler.com/2.0/'
else:
API_URL = 'https://ws.audioscrobbler.com/2.0/'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https => http ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's less readable.



class LastImportPlugin(plugins.BeetsPlugin):
Expand Down
3 changes: 2 additions & 1 deletion beetsplug/lyrics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,8 @@ def get_bing_access_token(self):
params = {
'client_id': 'beets',
'client_secret': self.config['bing_client_secret'],
'scope': 'http://api.microsofttranslator.com',
'scope': 'https://api.microsofttranslator.com' if sys.version_info >= (2, 7, 9) else
'http://api.microsofttranslator.com',
'grant_type': 'client_credentials',
}

Expand Down
24 changes: 24 additions & 0 deletions test/test_art.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,21 @@ def mock_response(self, url, content_type='image/jpeg', file_type=None):

class FetchImageTest(FetchImageHelper, UseThePlugin):
URL = 'http://example.com/test.jpg'
URL_HTTPS = 'https://example.com/test.jpg'

def setUp(self):
super(FetchImageTest, self).setUp()
self.dpath = os.path.join(self.temp_dir, b'arttest')
self.source = fetchart.RemoteArtSource(logger, self.plugin.config)
self.extra = {'maxwidth': 0}
self.candidate = fetchart.Candidate(logger, url=self.URL)
self.candidate_https = fetchart.Candidate(logger, url=self.URL_HTTPS)

def test_invalid_type_returns_none(self):
self.mock_response(self.URL, 'image/watercolour')
self.mock_response(self.URL_HTTPS, 'image/watercolour')
self.source.fetch_image(self.candidate, self.extra)
self.source.fetch_image(self.candidate_https, self.extra)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this test controls the URL on "both ends," i.e. the request and the response, I'm not sure it needs updating. Was it failing before?

self.assertEqual(self.candidate.path, None)

def test_jpeg_type_returns_path(self):
Expand All @@ -88,13 +92,17 @@ def test_jpeg_type_returns_path(self):

def test_extension_set_by_content_type(self):
self.mock_response(self.URL, 'image/png')
self.mock_response(self.URL_HTTPS, 'image/png')
self.source.fetch_image(self.candidate, self.extra)
self.source.fetch_image(self.candidate_https, self.extra)
self.assertEqual(os.path.splitext(self.candidate.path)[1], b'.png')
self.assertExists(self.candidate.path)

def test_does_not_rely_on_server_content_type(self):
self.mock_response(self.URL, 'image/jpeg', 'image/png')
self.mock_response(self.URL_HTTPS, 'image/jpeg', 'imsge/png')
self.source.fetch_image(self.candidate, self.extra)
self.source.fetch_image(self.candidate_https, self.extra)
self.assertEqual(os.path.splitext(self.candidate.path)[1], b'.png')
self.assertExists(self.candidate.path)

Expand Down Expand Up @@ -157,13 +165,21 @@ class CombinedTest(FetchImageHelper, UseThePlugin):
CAA_URL = 'http://coverartarchive.org/release/{0}/front' \
.format(MBID)

AMAZON_URL_HTTPS = 'http://images.amazon.com/images/P/{0}.01.LZZZZZZZ.jpg' \
.format(ASIN)
AAO_URL_HTTPS = 'http://www.albumart.org/index_detail.php?asin={0}' \
.format(ASIN)
CAA_URL_HTTPS = 'http://coverartarchive.org/release/{0}/front' \
.format(MBID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way we can avoid copying and pasting here? For example, we could put most of the URLs (starting with ://) into a variable and then concatenate on the https or http part separately.


def setUp(self):
super(CombinedTest, self).setUp()
self.dpath = os.path.join(self.temp_dir, b'arttest')
os.mkdir(self.dpath)

def test_main_interface_returns_amazon_art(self):
self.mock_response(self.AMAZON_URL)
self.mock_response(self.AMAZON_URL_HTTPS)
album = _common.Bag(asin=self.ASIN)
candidate = self.plugin.art_for_album(album, None)
self.assertIsNotNone(candidate)
Expand All @@ -176,38 +192,46 @@ def test_main_interface_returns_none_for_missing_asin_and_path(self):
def test_main_interface_gives_precedence_to_fs_art(self):
_common.touch(os.path.join(self.dpath, b'art.jpg'))
self.mock_response(self.AMAZON_URL)
self.mock_response(self.AMAZON_URL_HTTPS)
album = _common.Bag(asin=self.ASIN)
candidate = self.plugin.art_for_album(album, [self.dpath])
self.assertIsNotNone(candidate)
self.assertEqual(candidate.path, os.path.join(self.dpath, b'art.jpg'))

def test_main_interface_falls_back_to_amazon(self):
self.mock_response(self.AMAZON_URL)
self.mock_response(self.AMAZON_URL_HTTPS)
album = _common.Bag(asin=self.ASIN)
candidate = self.plugin.art_for_album(album, [self.dpath])
self.assertIsNotNone(candidate)
self.assertFalse(candidate.path.startswith(self.dpath))

def test_main_interface_tries_amazon_before_aao(self):
self.mock_response(self.AMAZON_URL)
self.mock_response(self.AMAZON_URL_HTTPS)
album = _common.Bag(asin=self.ASIN)
self.plugin.art_for_album(album, [self.dpath])
self.assertEqual(len(responses.calls), 1)
self.assertEqual(responses.calls[0].request.url, self.AMAZON_URL)
self.assertEqual(responses.calls[0].request.url, self.AMAZON_URL_HTTPS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to assert that the requested URL is equal to two different URLs, which probably isn't what you meant—instead, you probably want to write an assertion that says that the requested URL is either the HTTP one or the HTTPS one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The next couple of tests seem to have the same issue.)


def test_main_interface_falls_back_to_aao(self):
self.mock_response(self.AMAZON_URL, content_type='text/html')
self.mock_response(self.AMAZON_URL_HTTPS, content_type='text/html')
album = _common.Bag(asin=self.ASIN)
self.plugin.art_for_album(album, [self.dpath])
self.assertEqual(responses.calls[-1].request.url, self.AAO_URL)
self.assertEqual(responses.calls[-1].request.url, self.AAO_URL_HTTPS)

def test_main_interface_uses_caa_when_mbid_available(self):
self.mock_response(self.CAA_URL)
self.mock_response(self.CAA_URL_HTTPS)
album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN)
candidate = self.plugin.art_for_album(album, None)
self.assertIsNotNone(candidate)
self.assertEqual(len(responses.calls), 1)
self.assertEqual(responses.calls[0].request.url, self.CAA_URL)
self.assertEqual(responses.calls[0].request.url, self.CAA_URL_HTTPS)

def test_local_only_does_not_access_network(self):
album = _common.Bag(mb_albumid=self.MBID, asin=self.ASIN)
Expand Down