Repository: peterbe/premailer

Issue:
Replacement of urls beginning with //
When using a link whose href starts with "//", Premailer should just add http or https depending on the base url it uses. Currently, it replaces it with the full host, so ``//another.host/`` becomes ``http://my.base.host/another.host/``

Discussion:
graingert: Use
 
`if urlparse.urlparse(base_url).scheme:`
peterbe: @graingert done. 
peterbe: @graingert I force-pushed a new commit message. Just for you :)
graingert: Bling
peterbe: So, @graingert you approve?

What do you think @ewjoachim ?
ewjoachim: Nothing to add, I'm ok.

Patch:
diff --git a/premailer/premailer.py b/premailer/premailer.py
index b53fc23..2d3a221 100644
--- a/premailer/premailer.py
+++ b/premailer/premailer.py
@@ -17,7 +17,7 @@ if sys.version_info >= (3,):  # pragma: no cover
     # As in, Python 3
     from io import StringIO
     from urllib.request import urlopen
-    from urllib.parse import urljoin
+    from urllib.parse import urljoin, urlparse
     STR_TYPE = str
 else:  # Python 2
     try:
@@ -26,7 +26,7 @@ else:  # Python 2
         from StringIO import StringIO
         StringIO = StringIO  # shut up pyflakes
     from urllib2 import urlopen
-    from urlparse import urljoin
+    from urlparse import urljoin, urlparse
     STR_TYPE = basestring
 
 import cssutils
@@ -402,25 +402,23 @@ class Premailer(object):
         # URLs
         #
         if self.base_url:
-            if not self.base_url.endswith('/'):
-                self.base_url += '/'
+            if not urlparse(self.base_url).scheme:
+                raise ValueError('Base URL must start have a scheme')
             for attr in ('href', 'src'):
                 for item in page.xpath("//@%s" % attr):
                     parent = item.getparent()
+                    url = parent.attrib[attr]
                     if (
                         attr == 'href' and self.preserve_internal_links and
-                        parent.attrib[attr].startswith('#')
+                        url.startswith('#')
                     ):
                         continue
                     if (
                         attr == 'src' and self.preserve_inline_attachments and
-                        parent.attrib[attr].startswith('cid:')
+                        url.startswith('cid:')
                     ):
                         continue
-                    parent.attrib[attr] = urljoin(
-                        self.base_url,
-                        parent.attrib[attr].lstrip('/')
-                    )
+                    parent.attrib[attr] = urljoin(self.base_url, url)
 
         if hasattr(self.html, "getroottree"):
             return root


Test patch:
diff --git a/premailer/tests/test_premailer.py b/premailer/tests/test_premailer.py
index ba63f85..4390a4e 100644
--- a/premailer/tests/test_premailer.py
+++ b/premailer/tests/test_premailer.py
@@ -478,7 +478,6 @@ class Tests(unittest.TestCase):
         </head>
         <body>
         <img src="/images/foo.jpg">
-        <img src="/images/bar.gif">
         <img src="http://www.googe.com/photos/foo.jpg">
         <a href="/home">Home</a>
         <a href="http://www.peterbe.com">External</a>
@@ -494,10 +493,9 @@ class Tests(unittest.TestCase):
         <title>Title</title>
         </head>
         <body>
-        <img src="http://kungfupeople.com/base/images/foo.jpg">
-        <img src="http://kungfupeople.com/base/images/bar.gif">
+        <img src="http://kungfupeople.com/images/foo.jpg">
         <img src="http://www.googe.com/photos/foo.jpg">
-        <a href="http://kungfupeople.com/base/home">Home</a>
+        <a href="http://kungfupeople.com/home">Home</a>
         <a href="http://www.peterbe.com">External</a>
         <a href="http://www.peterbe.com/base/">External 2</a>
         <a href="http://kungfupeople.com/base/subpage">Subpage</a>
@@ -505,7 +503,7 @@ class Tests(unittest.TestCase):
         </body>
         </html>'''
 
-        p = Premailer(html, base_url='http://kungfupeople.com/base',
+        p = Premailer(html, base_url='http://kungfupeople.com/base/',
                       preserve_internal_links=True)
         result_html = p.transform()
 
@@ -2302,3 +2300,36 @@ sheet" type="text/css">
         result_html = p.transform()
 
         compare_html(expect_html, result_html)
+
+    def test_links_without_protocol(self):
+        """If you the base URL is set to https://example.com and your html
+        contains <img src="//otherdomain.com/">... then the URL to point to
+        is "https://otherdomain.com/" not "https://example.com/file.css"
+        """
+        html = """<html>
+        <head>
+        </head>
+        <body>
+        <img src="//example.com">
+        </body>
+        </html>"""
+
+        expect_html = """<html>
+        <head>
+        </head>
+        <body>
+        <img src="{protocol}://example.com">
+        </body>
+        </html>"""
+
+        p = Premailer(html, base_url='https://www.peterbe.com')
+        result_html = p.transform()
+        compare_html(expect_html.format(protocol="https"), result_html)
+
+        p = Premailer(html, base_url='http://www.peterbe.com')
+        result_html = p.transform()
+        compare_html(expect_html.format(protocol="http"), result_html)
+
+        # Because you can't set a base_url without a full protocol
+        p = Premailer(html, base_url='www.peterbe.com')
+        assert_raises(ValueError, p.transform)