Fix trailing slash in agent URL (#768)

src/api-engine/auth/tests.py

@@ -1,3 +1,77 @@
-from django.test import TestCase
+import sys
+import os
 
-# Create your tests here.
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+from django.test import SimpleTestCase
+from common.utils import safe_urljoin
+
+
+class SafeUrljoinTests(SimpleTestCase):
+    """
+    Unit tests for Issue #768: Deal with the trailing slash of agent URL.
+
+    The fix uses safe_urljoin() instead of urllib.parse.urljoin() to correctly
+    preserve path segments in the base URL regardless of whether the user
+    stored the agent_url with or without a trailing slash.
+    """
+
+    def test_base_without_slash_health_endpoint(self):
+        """safe_urljoin correctly handles base URL without trailing slash."""
+        self.assertEqual(
+            safe_urljoin("http://127.0.0.1:5001", "health"),
+            "http://127.0.0.1:5001/health"
+        )
+
+    def test_base_with_slash_health_endpoint(self):
+        """safe_urljoin works correctly when base URL already has trailing slash."""
+        self.assertEqual(
+            safe_urljoin("http://127.0.0.1:5001/", "health"),
+            "http://127.0.0.1:5001/health"
+        )
+
+    def test_base_with_path_without_slash(self):
+        """safe_urljoin preserves path segment when base has no trailing slash."""
+        self.assertEqual(
+            safe_urljoin("http://example.com/api", "health"),
+            "http://example.com/api/health"
+        )
+
+    def test_base_with_path_and_slash(self):
+        """safe_urljoin works correctly when base path already ends with slash."""
+        self.assertEqual(
+            safe_urljoin("http://example.com/api/", "health"),
+            "http://example.com/api/health"
+        )
+
+    def test_organizations_endpoint(self):
+        self.assertEqual(
+            safe_urljoin("http://example.com/api", "organizations"),
+            "http://example.com/api/organizations"
+        )
+
+    def test_nodes_status_endpoint(self):
+        self.assertEqual(
+            safe_urljoin("http://example.com/api", "nodes/status"),
+            "http://example.com/api/nodes/status"
+        )
+
+    def test_channels_endpoint(self):
+        self.assertEqual(
+            safe_urljoin("http://example.com/api", "channels"),
+            "http://example.com/api/channels"
+        )
+
+    def test_chaincodes_install_endpoint(self):
+        self.assertEqual(
+            safe_urljoin("http://example.com/api", "chaincodes/install"),
+            "http://example.com/api/chaincodes/install"
+        )
+
+    def test_original_urljoin_bug(self):
+        """Documents the original bug: stdlib urljoin drops path segments."""
+        from urllib.parse import urljoin
+        # This is the bug: /api is silently dropped
+        self.assertEqual(urljoin("http://example.com/api", "health"), "http://example.com/health")
+        # safe_urljoin fixes it
+        self.assertEqual(safe_urljoin("http://example.com/api", "health"), "http://example.com/api/health")

src/api-engine/chaincode/service.py

@@ -6,7 +6,7 @@
 import tarfile
 import threading
 from typing import Optional, List, Any, Dict, Tuple
-from urllib.parse import urljoin
+from common.utils import safe_urljoin
 
 import requests
 from django.db import transaction
@@ -30,9 +30,9 @@ def get_chaincode(pk: str) -> Optional[Chaincode]:
 
 def get_chaincode_status(organization: Organization, chaincode: Chaincode) -> str:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     response = requests.get(
-        urljoin(agent_url, "chaincodes/status"),
+        safe_urljoin(agent_url, "chaincodes/status"),
         params=dict(
             name=chaincode.name,
             package_id=chaincode.package_id,
@@ -46,9 +46,9 @@ def get_chaincode_status(organization: Organization, chaincode: Chaincode) -> st
 
 def get_chaincode_commit_readiness(organization: Organization, chaincode: Chaincode) -> str:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     response = requests.get(
-        urljoin(agent_url, "chaincodes/commit/readiness"),
+        safe_urljoin(agent_url, "chaincodes/commit/readiness"),
         params=dict(
             name=chaincode.name,
             version=chaincode.version,
@@ -73,9 +73,9 @@ def create_chaincode(
         init_required: bool = False,
         signature_policy: str = None) -> Chaincode:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     response = requests.post(
-        urljoin(agent_url, "chaincodes"),
+        safe_urljoin(agent_url, "chaincodes"),
         data=dict(
             name=name,
             version=version,
@@ -123,9 +123,9 @@ def metadata_exists(file) -> bool:
 
 def install_chaincode(organization: Organization, chaincode: Chaincode) -> None:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     requests.put(
-        urljoin(agent_url, "chaincodes/install"),
+        safe_urljoin(agent_url, "chaincodes/install"),
         data=dict(
             name=chaincode.name,
             version=chaincode.version,
@@ -142,9 +142,9 @@ def approve_chaincode(
         organization: Organization,
         chaincode: Chaincode) -> None:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     requests.put(
-        urljoin(agent_url, "chaincodes/approve"),
+        safe_urljoin(agent_url, "chaincodes/approve"),
         json=dict(
             name=chaincode.name,
             version=chaincode.version,
@@ -161,9 +161,9 @@ def commit_chaincode(
         organization: Organization,
         chaincode: Chaincode) -> None:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     requests.put(
-        urljoin(agent_url, "chaincodes/commit"),
+        safe_urljoin(agent_url, "chaincodes/commit"),
         json=dict(
             name=chaincode.name,
             version=chaincode.version,

src/api-engine/channel/service.py

@@ -1,5 +1,5 @@
 import logging
-from urllib.parse import urljoin
+from common.utils import safe_urljoin
 
 import requests
 
@@ -13,9 +13,9 @@ def create(
         channel_organization: Organization,
         channel_name: str) -> Channel:
     agent_url = channel_organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     requests.post(
-        urljoin(agent_url, "channels"),
+        safe_urljoin(agent_url, "channels"),
         json=dict(name=channel_name)
     ).raise_for_status()
 

src/api-engine/common/utils.py

@@ -1,4 +1,5 @@
 import uuid
+from urllib.parse import urljoin
 
 
 def make_uuid():
@@ -15,3 +16,22 @@ def separate_upper_class(class_name):
             x += c
         i += 1
     return "_".join(x.strip().split(" "))
+
+
+def safe_urljoin(base: str, path: str) -> str:
+    """Join a base URL and a path, ensuring the base URL's path is preserved.
+
+    Unlike urllib.parse.urljoin, this function guarantees the base URL ends
+    with a trailing slash before joining, so that any path segments in the
+    base are not silently dropped.
+
+    Example:
+        safe_urljoin("http://host/api", "health")
+        => "http://host/api/health"   (correct)
+
+        urljoin("http://host/api", "health")
+        => "http://host/health"       (wrong — drops /api)
+    """
+    if not base.endswith("/"):
+        base = base + "/"
+    return urljoin(base, path)

src/api-engine/node/service.py

@@ -3,7 +3,7 @@
 import os
 import sys
 from typing import Optional, Dict, Any, List
-from urllib.parse import urljoin
+from common.utils import safe_urljoin
 from zipfile import ZipFile
 
 import docker
@@ -26,9 +26,9 @@ def get_node(node_id: str) -> Optional[Node]:
 
 def get_node_status(organization: Organization, node: Node) -> str:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
     return requests.get(
-        urljoin(agent_url, "nodes/status"),
+        safe_urljoin(agent_url, "nodes/status"),
         params=dict(type=node.type, name=node.name)).json()["status"]
 
 
@@ -42,8 +42,8 @@ def organization_orderer_exists(organization: Organization) -> bool:
 
 def create(organization: Organization, node_type: Node.Type, node_name: str) -> Node:
     agent_url = organization.agent_url
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
-    response = requests.post(urljoin(agent_url, "nodes"), json=dict(type=node_type, name=node_name))
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
+    response = requests.post(safe_urljoin(agent_url, "nodes"), json=dict(type=node_type, name=node_name))
     response.raise_for_status()
 
     node = Node(

src/api-engine/organization/models.py

@@ -4,6 +4,8 @@
 from common.utils import make_uuid
 
 
+# Create your models here.
+
 class Organization(models.Model):
     id = models.UUIDField(
         primary_key=True,

src/api-engine/organization/service.py

@@ -1,4 +1,4 @@
-from urllib.parse import urljoin
+from common.utils import safe_urljoin
 
 import requests
 
@@ -13,5 +13,5 @@ def create_organization(org_name: str, agent_url: str) -> Organization:
 
 
 def _create_organization(org_name: str, agent_url: str):
-    requests.get(urljoin(agent_url, "health")).raise_for_status()
-    requests.post(urljoin(agent_url, "organizations"), json=dict(name=org_name)).raise_for_status()
+    requests.get(safe_urljoin(agent_url, "health")).raise_for_status()
+    requests.post(safe_urljoin(agent_url, "organizations"), json=dict(name=org_name)).raise_for_status()