openlp-core team mailing list archive

[Tomas Groth <tomasgroth@xxxxxxxx>]

Tomas Groth has proposed merging lp:~tomasgroth/openlp/bug1251437 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1251437 in OpenLP: "BibleGateway importer crashes on non unicode urls"
  https://bugs.launchpad.net/openlp/+bug/1251437

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/bug1251437/+merge/232370

Implemented a workaround for bug1251437 / http://bugs.python.org/issue22248
The problem is the urllib.request.urlopen cannot handle redirect urls with non-ascii chars, so I've added url-encoding of the redirect url.
-- 
https://code.launchpad.net/~tomasgroth/openlp/bug1251437/+merge/232370
Your team OpenLP Core is requested to review the proposed merge of lp:~tomasgroth/openlp/bug1251437 into lp:openlp.

=== modified file 'openlp/core/utils/__init__.py'
--- openlp/core/utils/__init__.py	2014-07-01 21:10:26 +0000
+++ openlp/core/utils/__init__.py	2014-08-27 08:56:22 +0000
@@ -109,6 +109,22 @@
             Registry().execute('openlp_version_check', '%s' % version)
 
 
+class HTTPRedirectHandlerFixed(urllib.request.HTTPRedirectHandler):
+    """
+    Special HTTPRedirectHandler used to work around http://bugs.python.org/issue22248
+    (Redirecting to urls with special chars)
+    """
+    def redirect_request(self, req, fp, code, msg, headers, newurl):
+        # Test if the newurl can be decoded to ascii
+        try:
+            test_url = newurl.encode('latin1').decode('ascii')
+            fixed_url = newurl
+        except Exception:
+            # The url could not be decoded to ascii, so we do some url encoding
+            fixed_url = urllib.parse.quote(newurl.encode('latin1').decode('utf-8', 'replace'), safe='/:')
+        return super(HTTPRedirectHandlerFixed, self).redirect_request(req, fp, code, msg, headers, fixed_url)
+
+
 def get_application_version():
     """
     Returns the application version of the running instance of OpenLP::
@@ -341,6 +357,9 @@
     # http://docs.python.org/library/urllib2.html
     if not url:
         return None
+    # This is needed to work around http://bugs.python.org/issue22248 and https://bugs.launchpad.net/openlp/+bug/1251437
+    opener = urllib.request.build_opener(HTTPRedirectHandlerFixed())
+    urllib.request.install_opener(opener)
     req = urllib.request.Request(url)
     if not header or header[0].lower() != 'user-agent':
         user_agent = _get_user_agent()

=== modified file 'openlp/plugins/bibles/lib/http.py'
--- openlp/plugins/bibles/lib/http.py	2014-08-26 16:51:52 +0000
+++ openlp/plugins/bibles/lib/http.py	2014-08-27 08:56:22 +0000
@@ -32,7 +32,6 @@
 import logging
 import re
 import socket
-import urllib.request
 import urllib.parse
 import urllib.error
 from html.parser import HTMLParseError

=== modified file 'tests/interfaces/openlp_plugins/bibles/test_lib_http.py'
--- tests/interfaces/openlp_plugins/bibles/test_lib_http.py	2014-04-02 19:35:09 +0000
+++ tests/interfaces/openlp_plugins/bibles/test_lib_http.py	2014-08-27 08:56:22 +0000
@@ -59,6 +59,19 @@
         # THEN: We should get back a valid service item
         assert len(books) == 66, 'The bible should not have had any books added or removed'
 
+    def bible_gateway_extract_books_support_redirect_test(self):
+        """
+        Test the Bible Gateway retrieval of book list for DN1933 bible with redirect (bug 1251437)
+        """
+        # GIVEN: A new Bible Gateway extraction class
+        handler = BGExtract()
+
+        # WHEN: The Books list is called
+        books = handler.get_books_from_http('DN1933')
+
+        # THEN: We should get back a valid service item
+        assert len(books) == 66, 'This bible should have 66 books'
+
     def bible_gateway_extract_verse_test(self):
         """
         Test the Bible Gateway retrieval of verse list for NIV bible John 3