salt.utils.url fails to properly quote URL userinfo

[cr1st1p]

Description of Issue

git module is lacking proper encoding of user and password fields inside 'userinfo' part of a url so it does not work with a personal repository for which the password contains more than letters and digits.

Setup

I'm trying to install a formula from a git repo, and it returns error
Port number ended with 'N' (yes, 'N' is part of the password)
And my password, being program generated, included a '+' and a '/' character in it.

I can easily replicate it with an sls like:

git_pass_test:
  module.run:
    - git.clone:
      - cwd: /tmp/repo
      - url: https://url/repo.git
      - https_user: someUser
      - https_pass: "some+Generated/Password"

Steps to Reproduce Issue

Use a sls file like above. Have a git account with a password containing '+' and/or '/'. Try to clone it.

Fix

Check section 3.2.1. from RFC https://tools.ietf.org/html/rfc3986 and you'll see that user and password can not appear as-is. utils/url.py code has no processing done on those fields before being added to the url.
Possible fix below (tested):

--- utils/url.py.orig   2019-12-08 22:20:34.564206447 +0000
+++ utils/url.py.fixed  2019-12-08 22:37:25.920496152 +0000
@@ -9,7 +9,7 @@
 import sys
 
 # Import salt libs
-from salt.ext.six.moves.urllib.parse import urlparse, urlunparse  # pylint: disable=import-error,no-name-in-module
+from salt.ext.six.moves.urllib.parse import urlparse, urlunparse, quote  # pylint: disable=import-error,no-name-in-module
 import salt.utils.data
 import salt.utils.path
 import salt.utils.platform
@@ -165,15 +165,15 @@
             raise ValueError('Basic Auth only supported for HTTPS')
         if password is None:
             netloc = '{0}@{1}'.format(
-                user,
+                quote(user, ""),
                 urltuple.netloc
             )
             urltuple = urltuple._replace(netloc=netloc)
             return urlunparse(urltuple)
         else:
             netloc = '{0}:{1}@{2}'.format(
-                user,
-                password,
+                quote(user, ""),
+                quote(password, ""),
                 urltuple.netloc
             )
             urltuple = urltuple._replace(netloc=netloc)

Versions Report

Salt Version:
           Salt: 2019.2.2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: 0.26.0
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.2
         Python: 2.7.15+ (default, Oct  7 2019, 17:39:04)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5
 
System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 5.3.12-1-MANJARO
         system: Linux
        version: Ubuntu 18.04 bionic

[waynew]

Hey @cr1st1p thanks for the report!

I changed the title here, since it's not just the git module - everything using salt.utils.url will run into issues with unquoted user/pass combinations, if someone has those chars in their password.