fixes #129, and also doesn't break existing favicon functionality - t…

…hanks unit tests!
oduwsdl · Sep 6, 2018 · 788d791cf35dab0dc5bf881e0defa82921bf9a82 · 788d791
1 parent e428687
commit 788d791cf35dab0dc5bf881e0defa82921bf9a82
Unified Split

Showing with 58 additions and 2 deletions.

+9 −2 mementoembed/favicon.py

+49 −0 tests/test_archiveresource.py
diff --git a/mementoembed/favicon.py b/mementoembed/favicon.py
@@ -52,20 +52,27 @@ def get_favicon_from_html(content):

    for link in links:

-        if 'rel' in link:
+        try:
            if 'icon' in link['rel']:
                favicon_uri = link['href']
                break
+        except KeyError:
+            module_logger.exception("there was no 'rel' attribute in this link tag: {}".format(link))
+            favicon_uri == None

    # if that fails, try the older, nonstandard relation 'shortcut'
    if favicon_uri == None:

        for link in links:

-            if 'rel' in link:
+            try:
                if 'shortcut' in link['rel']:
                    favicon_uri = link['href']
                    break
+            except KeyError:
+                module_logger.exception("there was no 'rel' attribute in this link tag: {}".format(link))
+                favicon_uri == None
+

    return favicon_uri

diff --git a/tests/test_archiveresource.py b/tests/test_archiveresource.py
@@ -279,3 +279,52 @@ def test_favicon_from_html_relative_uri(self):
        x = ArchiveResource(urim, httpcache)

        self.assertEqual(x.favicon, expected_favicon)
+
+    def test_link_tag_no_rel(self):
+
+        expected_favicon = None
+
+        cachedict = {
+            "http://myarchive.org":
+                mock_response(
+                    headers={},
+                    content="""<html>
+                    <head>
+                        <title>Is this a good title?</title>
+                        <link title="a good title" href="content/favicon.ico">
+                    </head>
+                    <body>Is this all there is to content?</body>
+                    </html>""",
+                    status=200,
+                    url = "testing-url://notused"
+                ),
+            expected_favicon:
+                mock_response(
+                    headers = { 'content-type': 'image/x-testing'},
+                    content = "a",
+                    status=200,
+                    url = "testing-url://notused"
+                ),
+            "http://myarchive.org/favicon.ico":
+                mock_response(
+                    headers={},
+                    content="not found",
+                    status=404,
+                    url="testing-url://notused"
+                ),
+            "https://www.google.com/s2/favicons?domain=myarchive.org":
+                mock_response(
+                    headers={},
+                    content="not found",
+                    status=404,
+                    url="testing-url://notused"
+                )
+        }
+
+        httpcache = mock_httpcache(cachedict)
+
+        urim = "http://myarchive.org/20160518000858/http://example.com/somecontent"
+
+        x = ArchiveResource(urim, httpcache)
+
+        self.assertEqual(x.favicon, expected_favicon)