diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index ac123fc17..93dcd232e 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -74,19 +74,6 @@ INSTANTIATE_TEST_SUITE_P( .path = {"", "home", "me", "repo"}, }, }, - // Already file: scheme - // NOTE: Git/SCP treat this as a `:`, so we are - // failing to "fix up" this case. - FixGitURLParam{ - .input = "file:/var/repos/x", - .expected = "file:/var/repos/x", - .parsed = - ParsedURL{ - .scheme = "file", - .authority = std::nullopt, - .path = {"", "var", "repos", "x"}, - }, - }, // IPV6 test case FixGitURLParam{ .input = "user@[2001:db8:1::2]:/home/file", @@ -136,6 +123,17 @@ TEST(FixGitURLTestSuite, properlyRejectFileURLWithAuthority) testing::HasSubstrIgnoreANSIMatcher("file:// URL 'file://var/repos/x' has unexpected authority 'var'"))); } +TEST(FixGitURLTestSuite, ambiguousScpLikeOrFileURL) +{ + /* Git/SCP treat this as a `:`, but under IETF RFC + 8089 it is a valid (if sloppy) file URL. Rather than decide who + is right, we just make it an error. */ + EXPECT_THAT( + []() { fixGitURL("file:/var/repos/x"); }, + ::testing::ThrowsMessage(testing::HasSubstrIgnoreANSIMatcher( + "URL 'file:/var/repos/x' would parse as SCP authority = 'file', path = '/var/repos/x' but this is also a valid `file:..` URL, and so we choose to disallow it"))); +} + TEST(FixGitURLTestSuite, scpLikePathLeadingSlashParsesPoorly) { // SCP-like URL (no user) diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 1c7fd3f0f..64ad2a566 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -413,8 +413,15 @@ ParsedURL fixGitURL(const std::string & url) std::regex scpRegex("([^/]*)@(.*):(.*)"); if (!hasPrefix(url, "/") && std::regex_match(url, scpRegex)) return parseURL(std::regex_replace(url, scpRegex, "ssh://$1@$2/$3")); - if (hasPrefix(url, "file:")) - return parseURL(url); + std::string_view path = url; + if (splitPrefix(path, "file:")) { + if (hasPrefix(url, "file://")) + return parseURL(url); + throw BadURL( + "URL '%s' would parse as SCP authority = 'file', path = '%s' but this is also a valid `file:..` URL, and so we choose to disallow it", + url, + path); + } if (url.find("://") == std::string::npos) { return ParsedURL{ .scheme = "file",