From 3f419cfa4e36bcb192e7d86c773a9f75b8f982e2 Mon Sep 17 00:00:00 2001 From: Brian McKenna Date: Sat, 21 Feb 2026 10:24:43 +1100 Subject: [PATCH] `ParsedURL::path` <-> `std::filesystem::path`, use in `StoreReference` This missing URL functionality allow us to properly fix the path shorthand for local store URLs test case. --- ...nd_2.txt => local_shorthand_path_unix.txt} | 0 .../local_shorthand_path_windows.txt | 1 + src/libstore-tests/store-reference.cc | 20 +++- src/libstore/store-reference.cc | 5 +- src/libutil-tests/url.cc | 104 ++++++++++++++++++ src/libutil/include/nix/util/url.hh | 25 ++++- src/libutil/url.cc | 64 ++++++++++- 7 files changed, 213 insertions(+), 6 deletions(-) rename src/libstore-tests/data/store-reference/{local_shorthand_2.txt => local_shorthand_path_unix.txt} (100%) create mode 100644 src/libstore-tests/data/store-reference/local_shorthand_path_windows.txt diff --git a/src/libstore-tests/data/store-reference/local_shorthand_2.txt b/src/libstore-tests/data/store-reference/local_shorthand_path_unix.txt similarity index 100% rename from src/libstore-tests/data/store-reference/local_shorthand_2.txt rename to src/libstore-tests/data/store-reference/local_shorthand_path_unix.txt diff --git a/src/libstore-tests/data/store-reference/local_shorthand_path_windows.txt b/src/libstore-tests/data/store-reference/local_shorthand_path_windows.txt new file mode 100644 index 000000000..e3c0179e3 --- /dev/null +++ b/src/libstore-tests/data/store-reference/local_shorthand_path_windows.txt @@ -0,0 +1 @@ +C:\foo\bar\baz?trusted=true \ No newline at end of file diff --git a/src/libstore-tests/store-reference.cc b/src/libstore-tests/store-reference.cc index 272d6732a..d4d273a8e 100644 --- a/src/libstore-tests/store-reference.cc +++ b/src/libstore-tests/store-reference.cc @@ -85,6 +85,20 @@ static StoreReference localExample_2{ }, }; +#ifdef _WIN32 +static StoreReference localExample_windows{ + .variant = + StoreReference::Specified{ + .scheme = "local", + .authority = "/C:/foo/bar/baz", + }, + .params = + { + {"trusted", "true"}, + }, +}; +#endif + static StoreReference localExample_3{ .variant = StoreReference::Specified{ @@ -108,7 +122,11 @@ URI_TEST_READ(local_3_no_percent, localExample_3) URI_TEST_READ(local_shorthand_1, localExample_1) -URI_TEST_READ(local_shorthand_2, localExample_2) +#ifndef _WIN32 +URI_TEST_READ(local_shorthand_path_unix, localExample_2) +#else +URI_TEST_READ(local_shorthand_path_windows, localExample_windows) +#endif URI_TEST( local_shorthand_3, diff --git a/src/libstore/store-reference.cc b/src/libstore/store-reference.cc index e26caede0..8f72a3d33 100644 --- a/src/libstore/store-reference.cc +++ b/src/libstore/store-reference.cc @@ -1,4 +1,5 @@ #include "nix/util/error.hh" +#include "nix/util/file-path-impl.hh" #include "nix/util/split.hh" #include "nix/util/url.hh" #include "nix/store/store-reference.hh" @@ -17,7 +18,7 @@ static bool isNonUriPath(const std::string & spec) spec.find("://") == std::string::npos // Has at least one path separator, and so isn't a single word that // might be special like "auto" - && spec.find("/") != std::string::npos; + && OsPathTrait::findPathSep(spec) != std::string::npos; } std::string StoreReference::render(bool withParams) const @@ -111,7 +112,7 @@ StoreReference StoreReference::parse(const std::string & uri, const StoreReferen .variant = Specified{ .scheme = "local", - .authority = absPath(baseURI), + .authority = encodeUrlPath(pathToUrlPath(absPath(std::filesystem::path{baseURI}))), }, .params = std::move(params), }; diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index 9d1a56a21..efc322feb 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -956,4 +956,108 @@ TEST(nix, isValidSchemeName) ASSERT_FALSE(isValidSchemeName("http ")); } +/* ---------------------------------------------------------------------------- + * pathToUrlPath / urlPathToPath + * --------------------------------------------------------------------------*/ + +struct UrlPathTestCase +{ + std::string_view urlString; + ParsedURL urlParsed; + std::filesystem::path path; + std::string description; +}; + +class UrlPathTest : public ::testing::TestWithParam +{}; + +TEST_P(UrlPathTest, pathToUrlPath) +{ + const auto & testCase = GetParam(); + auto urlPath = pathToUrlPath(testCase.path); + EXPECT_EQ(urlPath, testCase.urlParsed.path); +} + +TEST_P(UrlPathTest, urlPathToPath) +{ + const auto & testCase = GetParam(); + auto path = urlPathToPath(testCase.urlParsed.path); + EXPECT_EQ(path, testCase.path); +} + +TEST_P(UrlPathTest, urlToString) +{ + const auto & testCase = GetParam(); + EXPECT_EQ(testCase.urlParsed.to_string(), testCase.urlString); +} + +TEST_P(UrlPathTest, stringToUrl) +{ + const auto & testCase = GetParam(); + auto parsed = parseURL(std::string{testCase.urlString}); + EXPECT_EQ(parsed, testCase.urlParsed); +} + +#ifndef _WIN32 + +INSTANTIATE_TEST_SUITE_P( + Unix, + UrlPathTest, + ::testing::Values( + UrlPathTestCase{ + .urlString = "file:///foo/bar/baz", + .urlParsed = + ParsedURL{ + .scheme = "file", + .authority = ParsedURL::Authority{}, + .path = {"", "foo", "bar", "baz"}, + }, + .path = "/foo/bar/baz", + .description = "absolute_path", + }, + UrlPathTestCase{ + .urlString = "file:///", + .urlParsed = + ParsedURL{ + .scheme = "file", + .authority = ParsedURL::Authority{}, + .path = {"", ""}, + }, + .path = "/", + .description = "root_path", + }), + [](const auto & info) { return info.param.description; }); + +#else // _WIN32 + +INSTANTIATE_TEST_SUITE_P( + Windows, + UrlPathTest, + ::testing::Values( + UrlPathTestCase{ + .urlString = "file:///C:/foo/bar/baz", + .urlParsed = + ParsedURL{ + .scheme = "file", + .authority = ParsedURL::Authority{}, + .path = {"", "C:", "foo", "bar", "baz"}, + }, + .path = L"C:\\foo\\bar\\baz", + .description = "absolute_path", + }, + UrlPathTestCase{ + .urlString = "file:///C:/", + .urlParsed = + ParsedURL{ + .scheme = "file", + .authority = ParsedURL::Authority{}, + .path = {"", "C:", ""}, + }, + .path = L"C:\\", + .description = "drive_root", + }), + [](const auto & info) { return info.param.description; }); + +#endif // _WIN32 + } // namespace nix diff --git a/src/libutil/include/nix/util/url.hh b/src/libutil/include/nix/util/url.hh index 55c475df6..31144c0ab 100644 --- a/src/libutil/include/nix/util/url.hh +++ b/src/libutil/include/nix/util/url.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include #include #include @@ -265,7 +266,13 @@ std::string percentEncode(std::string_view s, std::string_view keep = ""); * paths have no escape sequences --- file names cannot contain a * `/`. */ -Path renderUrlPathEnsureLegal(const std::vector & urlPath); +Path renderUrlPathEnsureLegal(std::span urlPath); + +/** + * Render URL path segments to a string by joining with `/`. + * Does not percent-encode the segments. + */ +std::string renderUrlPathNoPctEncoding(std::span urlPath); /** * Percent encode path. `%2F` for "interior slashes" is the most @@ -347,6 +354,22 @@ ParsedURL fixGitURL(std::string url); */ bool isValidSchemeName(std::string_view scheme); +/** + * Convert a filesystem path to a URL path vector. + * + * On Windows, converts backslashes to forward slashes and prepends a `/` + * before the drive letter (e.g., `C:\foo\bar` becomes `/C:/foo/bar`). + */ +std::vector pathToUrlPath(const std::filesystem::path & path); + +/** + * Convert a URL path vector to a native filesystem path. + * + * On Windows, strips the leading `/` before the drive letter and converts + * to native format (e.g., `/C:/foo/bar` becomes `C:\foo\bar`). + */ +std::filesystem::path urlPathToPath(std::span urlPath); + /** * Either a ParsedURL or a verbatim string. This is necessary because in certain cases URI must be passed * verbatim (e.g. in builtin fetchers), since those are specified by the user. diff --git a/src/libutil/url.cc b/src/libutil/url.cc index d6bda4e9f..afa427a9a 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -321,7 +321,7 @@ std::string encodeQuery(const StringMap & ss) return res; } -Path renderUrlPathEnsureLegal(const std::vector & urlPath) +Path renderUrlPathEnsureLegal(std::span urlPath) { for (const auto & comp : urlPath) { /* This is only really valid for UNIX. Windows has more restrictions. */ @@ -334,6 +334,11 @@ Path renderUrlPathEnsureLegal(const std::vector & urlPath) } } + return renderUrlPathNoPctEncoding(urlPath); +} + +std::string renderUrlPathNoPctEncoding(std::span urlPath) +{ return concatStringsSep("/", urlPath); } @@ -341,7 +346,7 @@ std::string ParsedURL::renderPath(bool encode) const { if (encode) return encodeUrlPath(path); - return concatStringsSep("/", path); + return renderUrlPathNoPctEncoding(path); } std::string ParsedURL::renderAuthorityAndPath() const @@ -452,6 +457,61 @@ bool isValidSchemeName(std::string_view s) return std::regex_match(s.begin(), s.end(), regex, std::regex_constants::match_default); } +std::vector pathToUrlPath(const std::filesystem::path & path) +{ + std::vector urlPath; + + // Prepend empty segment for absolute paths (those with a root directory) + if (path.has_root_directory()) + urlPath.push_back(""); + + // Handle Windows drive letter (root_name like "C:") + if (path.has_root_name()) + urlPath.push_back(path.root_name().generic_string()); + + // Iterate only over the relative path portion + for (const auto & component : path.relative_path()) + urlPath.push_back(component.generic_string()); + + // Add trailing empty segment for paths ending with separator (including root-only paths) + if (path.filename().empty()) + urlPath.push_back(""); + + return urlPath; +} + +std::filesystem::path urlPathToPath(std::span urlPath) +{ + std::filesystem::path result; + auto it = urlPath.begin(); + + // URL path must start with empty segment (representing absolute path "/") + if (it == urlPath.end() || !it->empty()) + throw Error("only absolute URL paths can be converted to filesystem paths"); + + ++it; + result = "/"; +#ifdef _WIN32 + // On Windows, check if next segment is a drive letter (e.g., "C:"). + // If it isn't then this is something like a UNC path rather than a + // DOS path. + if (it != urlPath.end()) { + std::filesystem::path segment{*it}; + if (segment.has_root_name()) { + segment /= "/"; + result = std::move(segment); + ++it; + } + } +#endif + + // Append remaining segments + for (; it != urlPath.end(); ++it) + result /= *it; + + return result; +} + std::ostream & operator<<(std::ostream & os, const VerbatimURL & url) { os << url.to_string();