mirror of
https://github.com/NixOS/nix.git
synced 2025-11-10 20:46:01 +01:00
Make fixGitURL reject file:/foo/bar/ URL
This is a breaking change, but I think a good one. If we had allowed it, Git/SCP and the rest of our code would have disagreed on what this URL meant.
This commit is contained in:
parent
7195250fc4
commit
c80805cb61
2 changed files with 20 additions and 15 deletions
|
|
@ -74,19 +74,6 @@ INSTANTIATE_TEST_SUITE_P(
|
||||||
.path = {"", "home", "me", "repo"},
|
.path = {"", "home", "me", "repo"},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
// Already file: scheme
|
|
||||||
// NOTE: Git/SCP treat this as a `<hostname>:<path>`, 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
|
// IPV6 test case
|
||||||
FixGitURLParam{
|
FixGitURLParam{
|
||||||
.input = "user@[2001:db8:1::2]:/home/file",
|
.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'")));
|
testing::HasSubstrIgnoreANSIMatcher("file:// URL 'file://var/repos/x' has unexpected authority 'var'")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
TEST(FixGitURLTestSuite, ambiguousScpLikeOrFileURL)
|
||||||
|
{
|
||||||
|
/* Git/SCP treat this as a `<hostname>:<path>`, 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<BadURL>(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)
|
TEST(FixGitURLTestSuite, scpLikePathLeadingSlashParsesPoorly)
|
||||||
{
|
{
|
||||||
// SCP-like URL (no user)
|
// SCP-like URL (no user)
|
||||||
|
|
|
||||||
|
|
@ -413,8 +413,15 @@ ParsedURL fixGitURL(const std::string & url)
|
||||||
std::regex scpRegex("([^/]*)@(.*):(.*)");
|
std::regex scpRegex("([^/]*)@(.*):(.*)");
|
||||||
if (!hasPrefix(url, "/") && std::regex_match(url, scpRegex))
|
if (!hasPrefix(url, "/") && std::regex_match(url, scpRegex))
|
||||||
return parseURL(std::regex_replace(url, scpRegex, "ssh://$1@$2/$3"));
|
return parseURL(std::regex_replace(url, scpRegex, "ssh://$1@$2/$3"));
|
||||||
if (hasPrefix(url, "file:"))
|
std::string_view path = url;
|
||||||
return parseURL(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) {
|
if (url.find("://") == std::string::npos) {
|
||||||
return ParsedURL{
|
return ParsedURL{
|
||||||
.scheme = "file",
|
.scheme = "file",
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue