diff --git a/src/api-engine/auth/tests.py b/src/api-engine/auth/tests.py index 7ce503c2d..ff7d11e48 100644 --- a/src/api-engine/auth/tests.py +++ b/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") diff --git a/src/api-engine/chaincode/service.py b/src/api-engine/chaincode/service.py index dc68330dc..ec5d5dde9 100644 --- a/src/api-engine/chaincode/service.py +++ b/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, diff --git a/src/api-engine/channel/service.py b/src/api-engine/channel/service.py index f4ecc74b8..5246d2d52 100644 --- a/src/api-engine/channel/service.py +++ b/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() diff --git a/src/api-engine/common/utils.py b/src/api-engine/common/utils.py index 9ecee9ad3..10e114a13 100644 --- a/src/api-engine/common/utils.py +++ b/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) diff --git a/src/api-engine/node/service.py b/src/api-engine/node/service.py index 3ebfd53f5..f37438388 100644 --- a/src/api-engine/node/service.py +++ b/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( diff --git a/src/api-engine/organization/models.py b/src/api-engine/organization/models.py index 61955c6c1..fd6767bc7 100644 --- a/src/api-engine/organization/models.py +++ b/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, diff --git a/src/api-engine/organization/service.py b/src/api-engine/organization/service.py index 754783d8e..ffca54289 100644 --- a/src/api-engine/organization/service.py +++ b/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()