From f705ce7f9a95bc37df236d88eef1dbd9e2ae5f31 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Jan 2025 14:45:58 +0100 Subject: [PATCH 1/2] ParsedURL: Remove url field This prevents a 'url' field that is out of sync with the other fields. You can use to_string() to get the full URL. --- src/libfetchers/fetchers.cc | 2 +- src/libfetchers/github.cc | 16 ++++++++-------- src/libfetchers/indirect.cc | 8 ++++---- src/libfetchers/path.cc | 6 +++--- src/libflake/flake/flakeref.cc | 2 -- src/libutil-tests/url.cc | 22 ---------------------- src/libutil/url.cc | 7 ++++++- src/libutil/url.hh | 5 +++-- 8 files changed, 25 insertions(+), 43 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index b105c252a..87bf0dd71 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -66,7 +66,7 @@ Input Input::fromURL( } } - throw Error("input '%s' is unsupported", url.url); + throw Error("input '%s' is unsupported", url); } Input Input::fromAttrs(const Settings & settings, Attrs && attrs) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 308cff33a..185941988 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -50,7 +50,7 @@ struct GitArchiveInputScheme : InputScheme else if (std::regex_match(path[2], refRegex)) ref = path[2]; else - throw BadURL("in URL '%s', '%s' is not a commit hash or branch/tag name", url.url, path[2]); + throw BadURL("in URL '%s', '%s' is not a commit hash or branch/tag name", url, path[2]); } else if (size > 3) { std::string rs; for (auto i = std::next(path.begin(), 2); i != path.end(); i++) { @@ -63,34 +63,34 @@ struct GitArchiveInputScheme : InputScheme if (std::regex_match(rs, refRegex)) { ref = rs; } else { - throw BadURL("in URL '%s', '%s' is not a branch/tag name", url.url, rs); + throw BadURL("in URL '%s', '%s' is not a branch/tag name", url, rs); } } else if (size < 2) - throw BadURL("URL '%s' is invalid", url.url); + throw BadURL("URL '%s' is invalid", url); for (auto &[name, value] : url.query) { if (name == "rev") { if (rev) - throw BadURL("URL '%s' contains multiple commit hashes", url.url); + throw BadURL("URL '%s' contains multiple commit hashes", url); rev = Hash::parseAny(value, HashAlgorithm::SHA1); } else if (name == "ref") { if (!std::regex_match(value, refRegex)) - throw BadURL("URL '%s' contains an invalid branch/tag name", url.url); + throw BadURL("URL '%s' contains an invalid branch/tag name", url); if (ref) - throw BadURL("URL '%s' contains multiple branch/tag names", url.url); + throw BadURL("URL '%s' contains multiple branch/tag names", url); ref = value; } else if (name == "host") { if (!std::regex_match(value, hostRegex)) - throw BadURL("URL '%s' contains an invalid instance host", url.url); + throw BadURL("URL '%s' contains an invalid instance host", url); host_url = value; } // FIXME: barf on unsupported attributes } if (ref && rev) - throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url.url, *ref, rev->gitRev()); + throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url, *ref, rev->gitRev()); Input input{settings}; input.attrs.insert_or_assign("type", std::string { schemeName() }); diff --git a/src/libfetchers/indirect.cc b/src/libfetchers/indirect.cc index 2e5cd82c7..0e1b86711 100644 --- a/src/libfetchers/indirect.cc +++ b/src/libfetchers/indirect.cc @@ -26,16 +26,16 @@ struct IndirectInputScheme : InputScheme else if (std::regex_match(path[1], refRegex)) ref = path[1]; else - throw BadURL("in flake URL '%s', '%s' is not a commit hash or branch/tag name", url.url, path[1]); + throw BadURL("in flake URL '%s', '%s' is not a commit hash or branch/tag name", url, path[1]); } else if (path.size() == 3) { if (!std::regex_match(path[1], refRegex)) - throw BadURL("in flake URL '%s', '%s' is not a branch/tag name", url.url, path[1]); + throw BadURL("in flake URL '%s', '%s' is not a branch/tag name", url, path[1]); ref = path[1]; if (!std::regex_match(path[2], revRegex)) - throw BadURL("in flake URL '%s', '%s' is not a commit hash", url.url, path[2]); + throw BadURL("in flake URL '%s', '%s' is not a commit hash", url, path[2]); rev = Hash::parseAny(path[2], HashAlgorithm::SHA1); } else - throw BadURL("GitHub URL '%s' is invalid", url.url); + throw BadURL("GitHub URL '%s' is invalid", url); std::string id = path[0]; if (!std::regex_match(id, flakeRegex)) diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 246b68c3a..f628e042c 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -14,7 +14,7 @@ struct PathInputScheme : InputScheme if (url.scheme != "path") return {}; if (url.authority && *url.authority != "") - throw Error("path URL '%s' should not have an authority ('%s')", url.url, *url.authority); + throw Error("path URL '%s' should not have an authority ('%s')", url, *url.authority); Input input{settings}; input.attrs.insert_or_assign("type", "path"); @@ -27,10 +27,10 @@ struct PathInputScheme : InputScheme if (auto n = string2Int(value)) input.attrs.insert_or_assign(name, *n); else - throw Error("path URL '%s' has invalid parameter '%s'", url.to_string(), name); + throw Error("path URL '%s' has invalid parameter '%s'", url, name); } else - throw Error("path URL '%s' has unsupported parameter '%s'", url.to_string(), name); + throw Error("path URL '%s' has unsupported parameter '%s'", url, name); return input; } diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index ab882fdab..957495795 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -162,7 +162,6 @@ std::pair parsePathFlakeRefWithFragment( auto base = std::string("git+file://") + flakeRoot; auto parsedURL = ParsedURL{ - .url = base, // FIXME .base = base, .scheme = "git+file", .authority = "", @@ -220,7 +219,6 @@ static std::optional> parseFlakeIdRef( if (std::regex_match(url, match, flakeRegex)) { auto parsedURL = ParsedURL{ - .url = url, .base = "flake:" + match.str(1), .scheme = "flake", .authority = "", diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index 7d08f467e..6ffae4f3e 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -20,23 +20,11 @@ namespace nix { } - std::ostream& operator<<(std::ostream& os, const ParsedURL& p) { - return os << "\n" - << "url: " << p.url << "\n" - << "base: " << p.base << "\n" - << "scheme: " << p.scheme << "\n" - << "authority: " << p.authority.value() << "\n" - << "path: " << p.path << "\n" - << "query: " << print_map(p.query) << "\n" - << "fragment: " << p.fragment << "\n"; - } - TEST(parseURL, parsesSimpleHttpUrl) { auto s = "http://www.example.org/file.tar.gz"; auto parsed = parseURL(s); ParsedURL expected { - .url = "http://www.example.org/file.tar.gz", .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", @@ -53,7 +41,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "https://www.example.org/file.tar.gz", .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", @@ -70,7 +57,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "https://www.example.org/file.tar.gz", .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", @@ -87,7 +73,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://www.example.org/file.tar.gz", .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", @@ -104,7 +89,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "file+https://www.example.org/video.mp4", .base = "https://www.example.org/video.mp4", .scheme = "file+https", .authority = "www.example.org", @@ -126,7 +110,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://127.0.0.1:8080/file.tar.gz", .base = "https://127.0.0.1:8080/file.tar.gz", .scheme = "http", .authority = "127.0.0.1:8080", @@ -143,7 +126,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .base = "http://[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .scheme = "http", .authority = "[fe80::818c:da4d:8975:415c\%enp0s25]:8080", @@ -161,7 +143,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .base = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .scheme = "http", .authority = "[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", @@ -185,7 +166,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://user:pass@www.example.org/file.tar.gz", .base = "http://user:pass@www.example.org/file.tar.gz", .scheme = "http", .authority = "user:pass@www.example.org:8080", @@ -203,7 +183,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "", .base = "", .scheme = "file", .authority = "", @@ -228,7 +207,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "ftp://ftp.nixos.org/downloads/nixos.iso", .base = "ftp://ftp.nixos.org/downloads/nixos.iso", .scheme = "ftp", .authority = "ftp.nixos.org", diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 63b9734ee..86ca035b9 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -40,7 +40,6 @@ ParsedURL parseURL(const std::string & url) path = "/"; return ParsedURL{ - .url = url, .base = base, .scheme = scheme, .authority = authority, @@ -136,6 +135,12 @@ std::string ParsedURL::to_string() const + (fragment.empty() ? "" : "#" + percentEncode(fragment)); } +std::ostream & operator << (std::ostream & os, const ParsedURL & url) +{ + os << url.to_string(); + return os; +} + bool ParsedURL::operator ==(const ParsedURL & other) const noexcept { return diff --git a/src/libutil/url.hh b/src/libutil/url.hh index 738ee9f82..d85001185 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -7,9 +7,8 @@ namespace nix { struct ParsedURL { - std::string url; /// URL without query/fragment - std::string base; + std::string base; // FIXME: remove std::string scheme; std::optional authority; std::string path; @@ -26,6 +25,8 @@ struct ParsedURL ParsedURL canonicalise(); }; +std::ostream & operator << (std::ostream & os, const ParsedURL & url); + MakeError(BadURL, Error); std::string percentDecode(std::string_view in); From 4077aa43a803db33108ce39e799089608707dcbb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Jan 2025 14:52:00 +0100 Subject: [PATCH 2/2] ParsedURL: Remove base field --- src/libfetchers/git.cc | 2 +- src/libfetchers/mercurial.cc | 2 +- src/libflake/flake/flakeref.cc | 4 ---- src/libutil-tests/url.cc | 11 ----------- src/libutil/url.cc | 1 - src/libutil/url.hh | 2 -- 6 files changed, 2 insertions(+), 20 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index c73f53765..d894550c0 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -426,7 +426,7 @@ struct GitInputScheme : InputScheme auto url = parseURL(getStrAttr(input.attrs, "url")); bool isBareRepository = url.scheme == "file" && !pathExists(url.path + "/.git"); repoInfo.isLocal = url.scheme == "file" && !forceHttp && !isBareRepository; - repoInfo.url = repoInfo.isLocal ? url.path : url.base; + repoInfo.url = repoInfo.isLocal ? url.path : url.to_string(); // If this is a local directory and no ref or revision is // given, then allow the use of an unclean working tree. diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 2c987f79d..c2fd8139c 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -161,7 +161,7 @@ struct MercurialInputScheme : InputScheme { auto url = parseURL(getStrAttr(input.attrs, "url")); bool isLocal = url.scheme == "file"; - return {isLocal, isLocal ? url.path : url.base}; + return {isLocal, isLocal ? url.path : url.to_string()}; } StorePath fetchToStore(ref store, Input & input) const diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index 957495795..60efe1612 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -159,10 +159,7 @@ std::pair parsePathFlakeRefWithFragment( while (flakeRoot != "/") { if (pathExists(flakeRoot + "/.git")) { - auto base = std::string("git+file://") + flakeRoot; - auto parsedURL = ParsedURL{ - .base = base, .scheme = "git+file", .authority = "", .path = flakeRoot, @@ -219,7 +216,6 @@ static std::optional> parseFlakeIdRef( if (std::regex_match(url, match, flakeRegex)) { auto parsedURL = ParsedURL{ - .base = "flake:" + match.str(1), .scheme = "flake", .authority = "", .path = match[1], diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index 6ffae4f3e..7e1d2aa15 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -25,7 +25,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", .path = "/file.tar.gz", @@ -41,7 +40,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", .path = "/file.tar.gz", @@ -57,7 +55,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", .path = "/file.tar.gz", @@ -73,7 +70,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", .path = "/file.tar.gz", @@ -89,7 +85,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://www.example.org/video.mp4", .scheme = "file+https", .authority = "www.example.org", .path = "/video.mp4", @@ -110,7 +105,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://127.0.0.1:8080/file.tar.gz", .scheme = "http", .authority = "127.0.0.1:8080", .path = "/file.tar.gz", @@ -126,7 +120,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .scheme = "http", .authority = "[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .path = "", @@ -143,7 +136,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .scheme = "http", .authority = "[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .path = "", @@ -166,7 +158,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://user:pass@www.example.org/file.tar.gz", .scheme = "http", .authority = "user:pass@www.example.org:8080", .path = "/file.tar.gz", @@ -183,7 +174,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "", .scheme = "file", .authority = "", .path = "/none/of//your/business", @@ -207,7 +197,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "ftp://ftp.nixos.org/downloads/nixos.iso", .scheme = "ftp", .authority = "ftp.nixos.org", .path = "/downloads/nixos.iso", diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 86ca035b9..4d4d983b8 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -40,7 +40,6 @@ ParsedURL parseURL(const std::string & url) path = "/"; return ParsedURL{ - .base = base, .scheme = scheme, .authority = authority, .path = percentDecode(path), diff --git a/src/libutil/url.hh b/src/libutil/url.hh index d85001185..2b12f5af2 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -7,8 +7,6 @@ namespace nix { struct ParsedURL { - /// URL without query/fragment - std::string base; // FIXME: remove std::string scheme; std::optional authority; std::string path;