Solving slashes or other less usual symbols in vcs refs were not being interpreted correctly. (#5935)

* Solving issue 5934 where slashes or other less usual symbols in vcs refs were not being interpreted correctly.
* add some new unit-style integration tests

news/5934.bugfix.rst

@@ -0,0 +1 @@
+Eveb better handling of vcs branch references that contain special characters.

pipenv/utils/dependencies.py

@@ -1025,6 +1025,20 @@ def _file_path_from_pipfile(path_obj, pipfile_entry):
     return req_str
 
 
+def normalize_vcs_url(vcs_url):
+    """Return vcs_url and possible vcs_ref from a given vcs_url."""
+    # We have to handle the fact that some vcs urls have a ref in them
+    # and some have a netloc with a username and password in them, and some have both
+    vcs_ref = ""
+    if "@" in vcs_url:
+        parsed_url = urlparse(vcs_url)
+        if "@" in parsed_url.path:
+            url_parts = vcs_url.rsplit("@", 1)
+            vcs_url = url_parts[0]
+            vcs_ref = url_parts[1]
+    return vcs_url, vcs_ref
+
+
 def install_req_from_pipfile(name, pipfile):
     """Creates an InstallRequirement from a name and a pipfile entry.
     Handles VCS, local & remote paths, and regular named requirements.
@@ -1051,14 +1065,7 @@ def install_req_from_pipfile(name, pipfile):
         subdirectory = _pipfile.get("subdirectory", "")
         if subdirectory:
             subdirectory = f"#subdirectory={subdirectory}"
-        fallback_ref = ""
-        if ("ssh://" in vcs_url and vcs_url.count("@") >= 2) or (
-            "ssh://" not in vcs_url and "@" in vcs_url
-        ):
-            vcs_url_parts = vcs_url.rsplit("@", 1)
-            if re.match(r"^[\w\.]+$", vcs_url_parts[1]):
-                vcs_url = vcs_url_parts[0]
-                fallback_ref = vcs_url_parts[1]
+        vcs_url, fallback_ref = normalize_vcs_url(vcs_url)
         req_str = f"{vcs_url}@{_pipfile.get('ref', fallback_ref)}{extras_str}"
         if not req_str.startswith(f"{vcs}+"):
             req_str = f"{vcs}+{req_str}"

pipenv/utils/requirements.py

@@ -1,7 +1,7 @@
 import os
 import re
-import urllib.parse
 from typing import Tuple
+from urllib.parse import quote, unquote
 
 from pipenv.patched.pip._internal.network.session import PipSession
 from pipenv.patched.pip._internal.req import parse_requirements
@@ -41,7 +41,7 @@ def redact_netloc(netloc: str) -> Tuple[str]:
         else:
             # If it is, leave it as is
             password = ":" + password
-        user = urllib.parse.quote(user)
+        user = quote(user)
     return (f"{user}{password}@{netloc}",)
 
 
@@ -86,7 +86,7 @@ def import_requirements(project, r=None, dev=False, categories=None):
                 if package.editable:
                     package_string = f"-e {package.link}"
                 else:
-                    package_string = urllib.parse.unquote(
+                    package_string = unquote(
                         redact_auth_from_url(package.original_link.url)
                     )
 
@@ -143,7 +143,7 @@ BAD_PACKAGES = (
 def requirement_from_lockfile(
     package_name, package_info, include_hashes=True, include_markers=True
 ):
-    from pipenv.utils.dependencies import is_editable_path, is_star
+    from pipenv.utils.dependencies import is_editable_path, is_star, normalize_vcs_url
 
     # Handle string requirements
     if isinstance(package_info, str):
@@ -166,36 +166,31 @@ def requirement_from_lockfile(
     # Handling vcs repositories
     for vcs in VCS_LIST:
         if vcs in package_info:
-            url = package_info[vcs]
+            vcs_url = package_info[vcs]
             ref = package_info.get("ref", "")
-            if ("ssh://" in url and url.count("@") >= 2) or (
-                "ssh://" not in url and "@" in url
-            ):
-                url_parts = url.rsplit("@", 1)
-                # Check if the second part matches the criteria to be a ref (vcs URLs would likely have a /)
-                if re.match(r"^[\w\.]+$", url_parts[1]):
-                    url = url_parts[0]
-                    if not ref:
-                        ref = url_parts[1]
-
+            # We have to handle the fact that some vcs urls have a ref in them
+            # and some have a netloc with a username and password in them, and some have both
+            vcs_url, fallback_ref = normalize_vcs_url(vcs_url)
+            if not ref:
+                ref = fallback_ref
             extras = (
                 "[{}]".format(",".join(package_info.get("extras", [])))
                 if "extras" in package_info
                 else ""
             )
             subdirectory = package_info.get("subdirectory", "")
-            include_vcs = "" if f"{vcs}+" in url else f"{vcs}+"
-            egg_fragment = "" if "#egg=" in url else f"#egg={package_name}"
-            ref_str = "" if not ref or f"@{ref}" in url else f"@{ref}"
+            include_vcs = "" if f"{vcs}+" in vcs_url else f"{vcs}+"
+            egg_fragment = "" if "#egg=" in vcs_url else f"#egg={package_name}"
+            ref_str = "" if not ref or f"@{ref}" in vcs_url else f"@{ref}"
             if (
-                is_editable_path(url)
-                or "file://" in url
+                is_editable_path(vcs_url)
+                or "file://" in vcs_url
                 or package_info.get("editable", False)
             ):
-                pip_line = f"-e {include_vcs}{url}{ref_str}{egg_fragment}{extras}"
+                pip_line = f"-e {include_vcs}{vcs_url}{ref_str}{egg_fragment}{extras}"
                 pip_line += f"&subdirectory={subdirectory}" if subdirectory else ""
             else:
-                pip_line = f"{package_name}{extras}@ {include_vcs}{url}{ref_str}"
+                pip_line = f"{package_name}{extras}@ {include_vcs}{vcs_url}{ref_str}"
                 pip_line += f"#subdirectory={subdirectory}" if subdirectory else ""
             return pip_line
     # Handling file-sourced packages

tests/integration/test_lockfile.py

@@ -0,0 +1,63 @@
+from collections import defaultdict
+import json
+import pytest
+
+from pipenv.project import Project
+from pipenv.utils import requirements
+
+
+@pytest.fixture
+def pypi_lockfile():
+    lockfile = defaultdict(dict)
+    lockfile["_meta"] = {
+        "sources": [
+            {
+                "name": "pypi",
+                "url": "https://pypi.org/simple",
+                "verify_ssl": True,
+            }
+        ]
+    }
+    yield lockfile
+
+
+def test_git_branch_contains_slashes(
+        pipenv_instance_pypi, pypi_lockfile
+):
+    pypi_lockfile["default"]["google-api-python-client"] = {
+        "git": "https://github.com/thehesiod/google-api-python-client.git@thehesiod/batch-retries2",
+        "markers": "python_version >= '3.7'",
+        "ref": "03803c21fc13a345e978f32775b2f2fa23c8e706"
+    }
+
+    with pipenv_instance_pypi() as p:
+        with open(p.lockfile_path, "w") as f:
+            json.dump(pypi_lockfile, f)
+
+        project = Project()
+        lockfile = project.load_lockfile(expand_env_vars=False)
+        deps = lockfile["default"]
+        pip_installable_lines = requirements.requirements_from_lockfile(
+            deps, include_hashes=False, include_markers=True
+        )
+        assert pip_installable_lines == ["google-api-python-client@ git+https://github.com/thehesiod/google-api-python-client.git@03803c21fc13a345e978f32775b2f2fa23c8e706"]
+
+
+def test_git_branch_contains_subdirectory_fragment(pipenv_instance_pypi, pypi_lockfile):
+    pypi_lockfile["default"]["pep508_package"] = {
+        "git": "https://github.com/techalchemy/test-project.git@master",
+        "subdirectory": "parent_folder/pep508-package",
+        "ref": "03803c21fc13a345e978f32775b2f2fa23c8e706"
+    }
+
+    with pipenv_instance_pypi() as p:
+        with open(p.lockfile_path, "w") as f:
+            json.dump(pypi_lockfile, f)
+
+        project = Project()
+        lockfile = project.load_lockfile(expand_env_vars=False)
+        deps = lockfile["default"]
+        pip_installable_lines = requirements.requirements_from_lockfile(
+            deps, include_hashes=False, include_markers=True
+        )
+        assert pip_installable_lines == ['pep508_package@ git+https://github.com/techalchemy/test-project.git@03803c21fc13a345e978f32775b2f2fa23c8e706#subdirectory=parent_folder/pep508-package']