Преглед изворни кода

Improve LFS URL validation to prevent DNS errors with invalid config

When lfs.url is configured with an invalid value (like a relative path
"objects"), LFSClient.from_config() was using it without validation.
This caused DNS resolution errors when trying to connect to malformed
URLs like "objects/batch".
Jelmer Vernooij пре 3 месеци
родитељ
комит
97dd923548
3 измењених фајлова са 56 додато и 2 уклоњено
  1. 3 0
      NEWS
  2. 21 2
      dulwich/lfs.py
  3. 32 0
      tests/test_lfs.py

+ 3 - 0
NEWS

@@ -11,6 +11,9 @@
  * Fix passing ssh_command, password, and key_filename parameters to the SSH
    vendor. Regression from 0.24.2.  (Jelmer Vernooij, #1945)
 
+ * Fix LFS URL validation to prevent DNS resolution errors when ``lfs.url`` is
+   configured with an invalid value. (Jelmer Vernooij, #1951)
+
 0.24.6	2025-10-19
 
  * Fix import failure when ``sys.stdin`` is ``None``. The ``dulwich.server``

+ 21 - 2
dulwich/lfs.py

@@ -350,6 +350,19 @@ def _get_lfs_user_agent(config: Optional["Config"]) -> str:
     return f"git-lfs/dulwich/{version_str}"
 
 
+def _is_valid_lfs_url(url: str) -> bool:
+    """Check if a URL is valid for LFS (has scheme and netloc).
+
+    Args:
+        url: URL to validate
+
+    Returns:
+        True if URL has both scheme and netloc, False otherwise
+    """
+    parsed = urlparse(url)
+    return bool(parsed.scheme and parsed.netloc)
+
+
 class LFSClient:
     """LFS client for network operations."""
 
@@ -373,6 +386,12 @@ class LFSClient:
         except KeyError:
             pass
         else:
+            # Validate explicitly configured URL - raise error if invalid
+            if not _is_valid_lfs_url(url):
+                raise ValueError(
+                    f"Invalid lfs.url in config: {url!r}. "
+                    "URL must include scheme (http/https) and hostname."
+                )
             return cls(url, config)
 
         # Fall back to deriving from remote URL (same as git-lfs)
@@ -397,8 +416,8 @@ class LFSClient:
             # Standard LFS endpoint is remote_url + "/info/lfs"
             lfs_url = f"{remote_url}/info/lfs"
 
-            parsed = urlparse(lfs_url)
-            if not parsed.scheme or not parsed.netloc:
+            # Return None if derived URL is invalid (LFS is optional)
+            if not _is_valid_lfs_url(lfs_url):
                 return None
 
             return LFSClient(lfs_url, config)

+ 32 - 0
tests/test_lfs.py

@@ -1116,3 +1116,35 @@ class LFSClientTests(TestCase):
             self.client.upload("wrong_oid", 5, b"hello")
         # Server should reject due to OID mismatch
         self.assertIn("OID mismatch", str(cm.exception))
+
+    def test_from_config_validates_lfs_url(self) -> None:
+        """Test that from_config validates lfs.url and raises error for invalid URLs."""
+        from dulwich.config import ConfigFile
+        from dulwich.lfs import LFSClient
+
+        # Test with invalid lfs.url - no scheme/host
+        config = ConfigFile()
+        config.set((b"lfs",), b"url", b"objects")
+        with self.assertRaises(ValueError) as cm:
+            LFSClient.from_config(config)
+        self.assertIn("Invalid lfs.url", str(cm.exception))
+        self.assertIn("objects", str(cm.exception))
+
+        # Test with another malformed URL - no scheme
+        config.set((b"lfs",), b"url", b"//example.com/path")
+        with self.assertRaises(ValueError) as cm:
+            LFSClient.from_config(config)
+        self.assertIn("Invalid lfs.url", str(cm.exception))
+
+        # Test with valid URL - should succeed
+        config.set((b"lfs",), b"url", b"https://example.com/repo.git/info/lfs")
+        client = LFSClient.from_config(config)
+        self.assertIsNotNone(client)
+        self.assertEqual(client.url, "https://example.com/repo.git/info/lfs")
+
+        # Test with no lfs.url but valid remote - should derive URL
+        config2 = ConfigFile()
+        config2.set((b"remote", b"origin"), b"url", b"https://example.com/user/repo.git")
+        client2 = LFSClient.from_config(config2)
+        self.assertIsNotNone(client2)
+        self.assertEqual(client2.url, "https://example.com/user/repo.git/info/lfs")