From fac0c2d54a6b04175b40009506f2720d2594ed4e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 29 May 2020 16:19:48 -0400 Subject: Remove addToStore variant as requested by `FIXME` The idea is it's always more flexible to consumer a `Source` than a plain string, and it might even reduce memory consumption. I also looked at `addToStoreFromDump` with its `// FIXME: remove?`, but the worked needed for that is far more up for interpretation, so I punted for now. --- src/libfetchers/tarball.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/libfetchers') diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index bf2b2a5ff..b6e57379b 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -71,7 +71,8 @@ DownloadFileResult downloadFile( info.narHash = hashString(htSHA256, *sink.s); info.narSize = sink.s->size(); info.ca = makeFixedOutputCA(FileIngestionMethod::Flat, hash); - store->addToStore(info, sink.s, NoRepair, NoCheckSigs); + auto source = StringSource { *sink.s }; + store->addToStore(info, source, NoRepair, NoCheckSigs); storePath = std::move(info.path); } -- cgit v1.2.3 From 77007d4eabcf7090d1d3fbbdf84a67fb2262cf78 Mon Sep 17 00:00:00 2001 From: Nikola Knezevic Date: Sat, 30 May 2020 12:29:35 +0200 Subject: Improve ref validity checking in fetchGit The previous regex was too strict and did not match what git was allowing. It could lead to `fetchGit` not accepting valid branch names, even though they exist in a repository (for example, branch names containing `/`, which are pretty standard, like `release/1.0` branches). The new regex defines what a branch name should **NOT** contain. It takes the definitions from `refs.c` in https://github.com/git/git and `git help check-ref-format` pages. This change also introduces a test for ref name validity checking, which compares the result from Nix with the result of `git check-ref-format --branch`. --- src/libfetchers/git.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libfetchers') diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 17cc60228..e069057eb 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -418,7 +418,7 @@ struct GitInputScheme : InputScheme auto input = std::make_unique(parseURL(getStrAttr(attrs, "url"))); if (auto ref = maybeGetStrAttr(attrs, "ref")) { - if (!std::regex_match(*ref, refRegex)) + if (std::regex_search(*ref, badGitRefRegex)) throw BadURL("invalid Git branch/tag name '%s'", *ref); input->ref = *ref; } -- cgit v1.2.3 From fb38459d6e58508245553380cccc03c0dbaa1542 Mon Sep 17 00:00:00 2001 From: Nikola Knezevic Date: Sat, 30 May 2020 12:33:38 +0200 Subject: Ensure we restrict refspec interpretation while fetching As `git fetch` may chose to interpret refspec to it's liking, ensure that we only pass refs that begin with `refs/` as is, otherwise, prepend them with `refs/heads`. Otherwise, branches named `heads/foo` (I know it's bad, but it's allowed), would be fetched as `foo`, instead of `heads/foo`. --- src/libfetchers/git.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/libfetchers') diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index e069057eb..75ce5ee8b 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -282,7 +282,10 @@ struct GitInput : Input // FIXME: git stderr messes up our progress indicator, so // we're using --quiet for now. Should process its stderr. try { - runProgram("git", true, { "-C", repoDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", *input->ref, *input->ref) }); + auto fetchRef = input->ref->compare(0, 5, "refs/") == 0 + ? *input->ref + : "refs/heads/" + *input->ref; + runProgram("git", true, { "-C", repoDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }); } catch (Error & e) { if (!pathExists(localRefFile)) throw; warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl); -- cgit v1.2.3