[PATCH 0/5] Improvements to bb.fetch2.URI class

[PATCH 0/5] Improvements to bb.fetch2.URI class

[Olof Johansson]

This patch series fixes some issues that was encountered after the URI class
was integrated to Bitbake's fetch2 package. Notably, it adds support for query
parameters. This patch series does *not* make encode/decodeurl use the URI
class, as I had issues getting uri_replace to work with it.


Olof Johansson (5):
  fetch2.URI: Coerce urlparse to use netloc for all schemes
  tests.fetch: Remove debug assert
  fetch2.URI: add support for query parameters
  fetch2.URI: Support URIs with both query strings and params
  fetch2.URI: Set username/password should not change the other

 lib/bb/fetch2/__init__.py | 80 +++++++++++++++++++++++++++--------------------
 lib/bb/tests/fetch.py     | 62 +++++++++++++++++++++++++++++++++++-
 2 files changed, 107 insertions(+), 35 deletions(-)

-- 
1.8.5.2

[PATCH 1/5] fetch2.URI: Coerce urlparse to use netloc for all schemes

[Olof Johansson]

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 lib/bb/fetch2/__init__.py | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 9499a91..9c201aa 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -33,9 +33,6 @@ import glob
 import logging
 import urllib
 import urlparse
-if 'git' not in urlparse.uses_netloc:
-    urlparse.uses_netloc.append('git')
-from urlparse import urlparse
 import operator
 import bb.persist_data, bb.utils
 import bb.checksum
@@ -209,13 +206,25 @@ class URI(object):
         if not uri:
             return
 
-        urlp = urlparse(uri)
+        urlp = urlparse.urlparse(uri)
         self.scheme = urlp.scheme
 
-        # Convert URI to be relative
+        reparse = 0
+
+        # Coerce urlparse to make URI scheme use netloc
+        if not self.scheme in urlparse.uses_netloc:
+            urlparse.uses_params.append(self.scheme)
+            reparse = 1
+
+        # Make urlparse happy(/ier) by converting local resources
+        # to RFC compliant URL format. E.g.:
+        #   file://foo.diff -> file:foo.diff
         if urlp.scheme in self._netloc_forbidden:
             uri = re.sub("(?<=:)//(?!/)", "", uri, 1)
-            urlp = urlparse(uri)
+            reparse = 1
+
+        if reparse:
+            urlp = urlparse.urlparse(uri)
 
         # Identify if the URI is relative or not
         if urlp.scheme in self._relative_schemes and \
-- 
1.8.5.2

[PATCH 2/5] tests.fetch: Remove debug assert

[Olof Johansson]

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 lib/bb/tests/fetch.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 4bcff54..3dbeafa 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -243,8 +243,6 @@ class URITest(unittest.TestCase):
             uri.params = test['params']
             self.assertEqual(uri.params, test['params'])
 
-            self.assertEqual(str(uri)+str(uri.relative), str(test['uri'])+str(test['relative']))
-
             self.assertEqual(str(uri), test['uri'])
 
             uri.params = {}
-- 
1.8.5.2

[PATCH 3/5] fetch2.URI: add support for query parameters

[Olof Johansson]

This change introduces the .query property of the URI class. It is a
read/write dict of the parameters supplied in the query string of the
URI. E.g.:

  http://example.com/?foo=bar => .query = {'foo': 'bar'}

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 lib/bb/fetch2/__init__.py | 38 +++++++++++++++++++++++---------------
 lib/bb/tests/fetch.py     | 30 ++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 9c201aa..2ebdafa 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -162,6 +162,7 @@ class URI(object):
       * path_quoted (read/write)
         A URI quoted version of path
       * params (dict) (read/write)
+      * query (dict) (read/write)
       * relative (bool) (read only)
         True if this is a "relative URI", (e.g. file:foo.diff)
 
@@ -201,6 +202,7 @@ class URI(object):
         self.port = None
         self._path = ''
         self.params = {}
+        self.query = {}
         self.relative = False
 
         if not uri:
@@ -253,36 +255,42 @@ class URI(object):
         self.path = urllib.unquote(path)
 
         if param_str:
-            self.params = self._param_dict(param_str)
+            self.params = self._param_str_split(param_str, ";")
+        if urlp.query:
+            self.query = self._param_str_split(urlp.query, "&")
 
     def __str__(self):
         userinfo = self.userinfo
         if userinfo:
             userinfo += '@'
 
-        return "%s:%s%s%s%s%s" % (
+        return "%s:%s%s%s%s%s%s" % (
             self.scheme,
             '' if self.relative else '//',
             userinfo,
             self.hostport,
             self.path_quoted,
-            self._param_str)
+            self._query_str(),
+            self._param_str())
 
-    @property
     def _param_str(self):
-        ret = ''
-        for key, val in self.params.items():
-            ret += ";%s=%s" % (key, val)
+        return (
+            ''.join([';', self._param_str_join(self.params, ";")])
+            if self.params else '')
+
+    def _query_str(self):
+        return (
+            ''.join(['?', self._param_str_join(self.query, "&")])
+            if self.query else '')
+
+    def _param_str_split(self, string, elmdelim, kvdelim="="):
+        ret = {}
+        for k, v in [x.split(kvdelim, 1) for x in string.split(elmdelim)]:
+            ret[k] = v
         return ret
 
-    def _param_dict(self, param_str):
-        parm = {}
-
-        for keyval in param_str.split(";"):
-            key, val = keyval.split("=", 1)
-            parm[key] = val
-
-        return parm
+    def _param_str_join(self, dict_, elmdelim, kvdelim="="):
+        return elmdelim.join([kvdelim.join([k, v]) for k, v in dict_.items()])
 
     @property
     def hostport(self):
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 3dbeafa..5169496 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -39,6 +39,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': False
         },
         "http://www.google.com/index.html;param1=value1" : {
@@ -54,6 +55,23 @@ class URITest(unittest.TestCase):
             'params': {
                 'param1': 'value1'
             },
+            'query': {},
+            'relative': False
+        },
+        "http://www.example.org/index.html?param1=value1" : {
+            'uri': 'http://www.example.org/index.html?param1=value1',
+            'scheme': 'http',
+            'hostname': 'www.example.org',
+            'port': None,
+            'hostport': 'www.example.org',
+            'path': '/index.html',
+            'userinfo': '',
+            'username': '',
+            'password': '',
+            'params': {},
+            'query': {
+                'param1': 'value1'
+            },
             'relative': False
         },
         "http://www.example.com:8080/index.html" : {
@@ -67,6 +85,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': False
         },
         "cvs://anoncvs@cvs.handhelds.org/cvs;module=familiar/dist/ipkg" : {
@@ -82,6 +101,7 @@ class URITest(unittest.TestCase):
             'params': {
                 'module': 'familiar/dist/ipkg'
             },
+            'query': {},
             'relative': False
         },
         "cvs://anoncvs:anonymous@cvs.handhelds.org/cvs;tag=V0-99-81;module=familiar/dist/ipkg": {
@@ -98,6 +118,7 @@ class URITest(unittest.TestCase):
                 'tag': 'V0-99-81',
                 'module': 'familiar/dist/ipkg'
             },
+            'query': {},
             'relative': False
         },
         "file://example.diff": { # NOTE: Not RFC compliant!
@@ -111,6 +132,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': True
         },
         "file:example.diff": { # NOTE: RFC compliant version of the former
@@ -125,6 +147,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': True
         },
         "file:///tmp/example.diff": {
@@ -139,6 +162,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': False
         },
         "git:///path/example.git": {
@@ -153,6 +177,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': False
         },
         "git:path/example.git": {
@@ -167,6 +192,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': True
         },
         "git://example.net/path/example.git": {
@@ -181,6 +207,7 @@ class URITest(unittest.TestCase):
             'username': '',
             'password': '',
             'params': {},
+            'query': {},
             'relative': False
         }
     }
@@ -243,6 +270,9 @@ class URITest(unittest.TestCase):
             uri.params = test['params']
             self.assertEqual(uri.params, test['params'])
 
+            uri.query = test['query']
+            self.assertEqual(uri.query, test['query'])
+
             self.assertEqual(str(uri), test['uri'])
 
             uri.params = {}
-- 
1.8.5.2

[PATCH 4/5] fetch2.URI: Support URIs with both query strings and params

[Olof Johansson]

There is a case in meta-intel where a SRC_URI contains both a query string and
URI parameter:

 https://edc.intel.com/Download.aspx?id=6190;downloadfilename=LIN_IEMGD_1_14_GOLD_2443.tgz

Python's urlparse thought the URI parameters were part of the query parameter
value, but in the bitbake context this is obviously not the case. As bitbake's
usage isn't really RFC compliant, we have to extract and remove the URI parameters
*before* urlparse sees the URI.

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 lib/bb/fetch2/__init__.py | 16 +++++-----------
 lib/bb/tests/fetch.py     | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 2ebdafa..abff9e2 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -208,6 +208,10 @@ class URI(object):
         if not uri:
             return
 
+        # We hijack the URL parameters, since the way bitbake uses
+        # them are not quite RFC compliant.
+        uri, param_str = (uri.split(";", 1) + [None])[:2]
+
         urlp = urlparse.urlparse(uri)
         self.scheme = urlp.scheme
 
@@ -242,17 +246,7 @@ class URI(object):
             if urlp.password:
                 self.userinfo += ':%s' % urlp.password
 
-        # Do support params even for URI schemes that Python's
-        # urlparse doesn't support params for.
-        path = ''
-        param_str = ''
-        if not urlp.params:
-            path, param_str = (list(urlp.path.split(";", 1)) + [None])[:2]
-        else:
-            path = urlp.path
-            param_str = urlp.params
-
-        self.path = urllib.unquote(path)
+        self.path = urllib.unquote(urlp.path)
 
         if param_str:
             self.params = self._param_str_split(param_str, ";")
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 5169496..6ffe296 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -74,6 +74,24 @@ class URITest(unittest.TestCase):
             },
             'relative': False
         },
+        "http://www.example.org/index.html?qparam1=qvalue1;param2=value2" : {
+            'uri': 'http://www.example.org/index.html?qparam1=qvalue1;param2=value2',
+            'scheme': 'http',
+            'hostname': 'www.example.org',
+            'port': None,
+            'hostport': 'www.example.org',
+            'path': '/index.html',
+            'userinfo': '',
+            'username': '',
+            'password': '',
+            'params': {
+                'param2': 'value2'
+            },
+            'query': {
+                'qparam1': 'qvalue1'
+            },
+            'relative': False
+        },
         "http://www.example.com:8080/index.html" : {
             'uri': 'http://www.example.com:8080/index.html',
             'scheme': 'http',
-- 
1.8.5.2

[PATCH 5/5] fetch2.URI: Set username/password should not change the other

[Olof Johansson]

When setting the username after already having set the password, the password
was unexpectedly reset. This change fixes this issue and introduces unit tests
to make sure it doesn't happen again.

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 lib/bb/fetch2/__init__.py |  5 +++--
 lib/bb/tests/fetch.py     | 14 ++++++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index abff9e2..18cd821 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -321,9 +321,10 @@ class URI(object):
 
     @username.setter
     def username(self, username):
+        password = self.password
         self.userinfo = username
-        if self.password:
-            self.userinfo += ":%s" % self.password
+        if password:
+            self.userinfo += ":%s" % password
 
     @property
     def password(self):
diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index 6ffe296..707062e 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -274,6 +274,20 @@ class URITest(unittest.TestCase):
             self.assertEqual(uri.username, test['username'])
             self.assertEqual(uri.password, test['password'])
 
+            # make sure changing the values doesn't do anything unexpected
+            uri.username = 'changeme'
+            self.assertEqual(uri.username, 'changeme')
+            self.assertEqual(uri.password, test['password'])
+            uri.password = 'insecure'
+            self.assertEqual(uri.username, 'changeme')
+            self.assertEqual(uri.password, 'insecure')
+
+            # reset back after our trickery
+            uri.userinfo = test['userinfo']
+            self.assertEqual(uri.userinfo, test['userinfo'])
+            self.assertEqual(uri.username, test['username'])
+            self.assertEqual(uri.password, test['password'])
+
             uri.hostname = test['hostname']
             self.assertEqual(uri.hostname, test['hostname'])
             self.assertEqual(uri.hostport, test['hostname'])
-- 
1.8.5.2

Re: [PATCH 0/5] Improvements to bb.fetch2.URI class

[Olof Johansson]

On 14-01-20 12:03 +0100, Olof Johansson wrote:
> This patch series fixes some issues that was encountered after the URI class
> was integrated to Bitbake's fetch2 package. Notably, it adds support for query
> parameters. This patch series does *not* make encode/decodeurl use the URI
> class, as I had issues getting uri_replace to work with it.

Ping?

-- 
olofjn

[PATCH 1/5] fetch2.URI: Coerce urlparse to use netloc for all schemes

[Olof Johansson]

With this change, we don't have to think about what urlparse
supports and not, and can treat all URIs the same in this regard.
This should also make a previous, git specific, fix for this
problem redundant (and thus removed).

Signed-off-by: Olof Johansson <olof.johansson@axis.com>
---
 lib/bb/fetch2/__init__.py |   21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 4cfe089..455ab6a 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -31,9 +31,6 @@ import os, re
 import logging
 import urllib
 import urlparse
-if 'git' not in urlparse.uses_netloc:
-    urlparse.uses_netloc.append('git')
-from urlparse import urlparse
 import operator
 import bb.persist_data, bb.utils
 import bb.checksum
@@ -202,13 +199,25 @@ class URI(object):
         if not uri:
             return
 
-        urlp = urlparse(uri)
+        urlp = urlparse.urlparse(uri)
         self.scheme = urlp.scheme
 
-        # Convert URI to be relative
+        reparse = 0
+
+        # Coerce urlparse to make URI scheme use netloc
+        if not self.scheme in urlparse.uses_netloc:
+            urlparse.uses_params.append(self.scheme)
+            reparse = 1
+
+        # Make urlparse happy(/ier) by converting local resources
+        # to RFC compliant URL format. E.g.:
+        #   file://foo.diff -> file:foo.diff
         if urlp.scheme in self._netloc_forbidden:
             uri = re.sub("(?<=:)//(?!/)", "", uri, 1)
-            urlp = urlparse(uri)
+            reparse = 1
+
+        if reparse:
+            urlp = urlparse.urlparse(uri)
 
         # Identify if the URI is relative or not
         if urlp.scheme in self._relative_schemes and \
-- 
1.7.10.4