From 3b028edbf7b2acc53dbca6b45193825943215884 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Mon, 12 Jan 2026 03:08:18 +0100
Subject: [PATCH] fixGitURL: fix crash for relative paths and reject
 unsupported SCP URLs

Relative paths (e.g., "relative/repo") would crash in renderAuthorityAndPath()
because an empty authority was set, violating RFC 3986 section 3.3 which
requires paths to start with "/" when an authority is present.

Fix by only setting authority for absolute paths:
- Absolute paths: file:///path (empty authority)
- Relative paths: file:path (no authority)

Also reject SCP-like URLs without a user (e.g., "github.com:path") with a
clear error message, since proper support requires careful implementation,
which is not something I can do right now.
---
 src/libutil-tests/url.cc | 53 +++++++++++++---------------------------
 src/libutil/url.cc       | 24 +++++++++++++-----
 2 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc
index 356a134dc9b..9d1a56a2158 100644
--- a/src/libutil-tests/url.cc
+++ b/src/libutil-tests/url.cc
@@ -112,19 +112,13 @@ TEST_P(FixGitURLTestSuite, parsesVariedGitUrls)
     EXPECT_EQ(actual.to_string(), p.expected);
 }
 
-TEST(FixGitURLTestSuite, scpLikeNoUserParsesPoorly)
+TEST(FixGitURLTestSuite, rejectScpLikeNoUser)
 {
-    // SCP-like URL (no user)
-
-    // Cannot "to_string" this because has illegal path not starting
-    // with `/`.
-    EXPECT_EQ(
-        fixGitURL("github.com:owner/repo.git"),
-        (ParsedURL{
-            .scheme = "file",
-            .authority = ParsedURL::Authority{},
-            .path = {"github.com:owner", "repo.git"},
-        }));
+    // SCP-like URL without user. Proper support can be implemented, but this is
+    // a deceptively deep feature - study existing implementations carefully.
+    EXPECT_THAT(
+        []() { fixGitURL("github.com:owner/repo.git"); },
+        ::testing::ThrowsMessage<BadURL>(testing::HasSubstrIgnoreANSIMatcher("SCP-like URL")));
 }
 
 TEST(FixGitURLTestSuite, properlyRejectFileURLWithAuthority)
@@ -136,37 +130,24 @@ TEST(FixGitURLTestSuite, properlyRejectFileURLWithAuthority)
             testing::HasSubstrIgnoreANSIMatcher("file:// URL 'file://var/repos/x' has unexpected authority 'var'")));
 }
 
-TEST(FixGitURLTestSuite, scpLikePathLeadingSlashParsesPoorly)
+TEST(FixGitURLTestSuite, rejectScpLikeNoUserLeadingSlash)
 {
-    // SCP-like URL (no user)
-
-    // Cannot "to_string" this because has illegal path not starting
-    // with `/`.
-    EXPECT_EQ(
-        fixGitURL("github.com:/owner/repo.git"),
-        (ParsedURL{
-            .scheme = "file",
-            .authority = ParsedURL::Authority{},
-            .path = {"github.com:", "owner", "repo.git"},
-        }));
+    EXPECT_THAT(
+        []() { fixGitURL("github.com:/owner/repo.git"); },
+        ::testing::ThrowsMessage<BadURL>(testing::HasSubstrIgnoreANSIMatcher("SCP-like URL")));
 }
 
-TEST(FixGitURLTestSuite, relativePathParsesPoorly)
+TEST(FixGitURLTestSuite, relativePath)
 {
-    // Relative path (becomes file:// absolute)
-
-    // Cannot "to_string" this because has illegal path not starting
-    // with `/`.
+    // Relative path - parsed as file path without authority
+    auto parsed = fixGitURL("relative/repo");
     EXPECT_EQ(
-        fixGitURL("relative/repo"),
+        parsed,
         (ParsedURL{
             .scheme = "file",
-            .authority =
-                ParsedURL::Authority{
-                    .hostType = ParsedURL::Authority::HostType::Name,
-                    .host = "",
-                },
-            .path = {"relative", "repo"}}));
+            .path = {"relative", "repo"},
+        }));
+    EXPECT_EQ(parsed.to_string(), "file:relative/repo");
 }
 
 struct ParseURLSuccessCase
diff --git a/src/libutil/url.cc b/src/libutil/url.cc
index 0a8b6452814..d6bda4e9f5b 100644
--- a/src/libutil/url.cc
+++ b/src/libutil/url.cc
@@ -417,12 +417,24 @@ ParsedURL fixGitURL(std::string url)
     std::regex scpRegex("([^/]*)@(.*):(.*)");
     if (!hasPrefix(url, "/") && std::regex_match(url, scpRegex))
         url = std::regex_replace(url, scpRegex, "ssh://$1@$2/$3");
-    if (!hasPrefix(url, "file:") && !hasPrefix(url, "git+file:") && url.find("://") == std::string::npos)
-        return ParsedURL{
-            .scheme = "file",
-            .authority = ParsedURL::Authority{},
-            .path = splitString<std::vector<std::string>>(url, "/"),
-        };
+    if (!hasPrefix(url, "file:") && !hasPrefix(url, "git+file:") && url.find("://") == std::string::npos) {
+        auto path = splitString<std::vector<std::string>>(url, "/");
+        // Reject SCP-like URLs without user (e.g., "github.com:path") - colon in first component
+        if (!path.empty() && path[0].find(':') != std::string::npos)
+            throw BadURL("SCP-like URL '%s' is not supported; use SSH URL syntax instead (ssh://...)", url);
+        // Absolute paths get an empty authority (file:///path), relative paths get none (file:path)
+        if (hasPrefix(url, "/"))
+            return ParsedURL{
+                .scheme = "file",
+                .authority = ParsedURL::Authority{},
+                .path = path,
+            };
+        else
+            return ParsedURL{
+                .scheme = "file",
+                .path = path,
+            };
+    }
     auto parsed = parseURL(url);
     // Drop the superfluous "git+" from the scheme.
     auto scheme = parseUrlScheme(parsed.scheme);