Merge branch 'master' of https://github.com/alekswn/nix

7 files changed, 254 insertions, 33 deletions

diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc
index 065efc855..9b4cfd835 100644
--- a/src/libstore/build/entry-points.cc
+++ b/src/libstore/build/entry-points.cc
@@ -1,4 +1,3 @@
-#include "machines.hh"
 #include "worker.hh"
 #include "substitution-goal.hh"
 #include "derivation-goal.hh"
diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc
index 9843ccf04..910e32a76 100644
--- a/src/libstore/machines.cc
+++ b/src/libstore/machines.cc
@@ -83,53 +83,83 @@ ref<Store> Machine::openStore() const {
     return nix::openStore(storeUri, storeParams);
 }
 
-void parseMachines(const std::string & s, Machines & machines)
-{
-    for (auto line : tokenizeString<std::vector<string>>(s, "\n;")) {
+static std::vector<std::string> expandBuilderLines(const std::string& builders) {
+    std::vector<std::string> result;
+    for (auto line : tokenizeString<std::vector<string>>(builders, "\n;")) {
         trim(line);
         line.erase(std::find(line.begin(), line.end(), '#'), line.end());
         if (line.empty()) continue;
 
         if (line[0] == '@') {
-            auto file = trim(std::string(line, 1));
+            const std::string path = trim(std::string(line, 1));
+            std::string text;
             try {
-                parseMachines(readFile(file), machines);
+                text = readFile(path);
             } catch (const SysError & e) {
                 if (e.errNo != ENOENT)
                     throw;
-                debug("cannot find machines file '%s'", file);
+                debug("cannot find machines file '%s'", path);
             }
+
+            const auto lines = expandBuilderLines(text);
+            result.insert(end(result), begin(lines), end(lines));
             continue;
         }
 
-        auto tokens = tokenizeString<std::vector<string>>(line);
-        auto sz = tokens.size();
-        if (sz < 1)
-            throw FormatError("bad machine specification '%s'", line);
+        result.emplace_back(line);
+    }
+    return result;
+}
 
-        auto isSet = [&](size_t n) {
-            return tokens.size() > n && tokens[n] != "" && tokens[n] != "-";
-        };
+static Machine parseBuilderLine(const std::string& line) {
+    const auto tokens = tokenizeString<std::vector<string>>(line);
+
+    auto isSet = [&](size_t fieldIndex) {
+        return tokens.size() > fieldIndex && tokens[fieldIndex] != "" && tokens[fieldIndex] != "-";
+    };
+
+    auto parseUnsignedIntField = [&](size_t fieldIndex) {
+        const auto result = string2Int<unsigned int>(tokens[fieldIndex]);
+        if (!result) {
+            throw FormatError("bad machine specification: failed to convert column #%lu in a row: '%s' to 'unsigned int'", fieldIndex, line);
+        }
+        return result.value();
+    };
+
+    auto ensureBase64 = [&](size_t fieldIndex) {
+        const auto& str = tokens[fieldIndex];
+        try {
+            base64Decode(str);
+        } catch (const Error& e) {
+            throw FormatError("bad machine specification: a column #%lu in a row: '%s' is not valid base64 string: %s", fieldIndex, line, e.what());
+        }
+        return str;
+    };
+
+    if (!isSet(0)) {
+        throw FormatError("bad machine specification: store URI was not found at the first column of a row: '%s'", line);
+    }
 
-        machines.emplace_back(tokens[0],
+    return {tokens[0],
             isSet(1) ? tokenizeString<std::vector<string>>(tokens[1], ",") : std::vector<string>{settings.thisSystem},
             isSet(2) ? tokens[2] : "",
-            isSet(3) ? std::stoull(tokens[3]) : 1LL,
-            isSet(4) ? std::stoull(tokens[4]) : 1LL,
+            isSet(3) ? parseUnsignedIntField(3) : 1U,
+            isSet(4) ? parseUnsignedIntField(4) : 1U,
             isSet(5) ? tokenizeString<std::set<string>>(tokens[5], ",") : std::set<string>{},
             isSet(6) ? tokenizeString<std::set<string>>(tokens[6], ",") : std::set<string>{},
-            isSet(7) ? tokens[7] : "");
-    }
+            isSet(7) ? ensureBase64(7) : ""};
+}
+
+static Machines parseBuilderLines(const std::vector<std::string>& builders) {
+    Machines result;
+    std::transform(builders.begin(), builders.end(), std::back_inserter(result), parseBuilderLine);
+    return result;
 }
 
 Machines getMachines()
 {
-    static auto machines = [&]() {
-        Machines machines;
-        parseMachines(settings.builders, machines);
-        return machines;
-    }();
-    return machines;
+    const auto builderLines = expandBuilderLines(settings.builders);
+    return parseBuilderLines(builderLines);
 }
 
 }
diff --git a/src/libstore/tests/machines.cc b/src/libstore/tests/machines.cc
new file mode 100644
index 000000000..f51052b14
--- /dev/null
+++ b/src/libstore/tests/machines.cc
@@ -0,0 +1,169 @@
+#include "machines.hh"
+#include "globals.hh"
+
+#include <gmock/gmock-matchers.h>
+
+using testing::Contains;
+using testing::ElementsAre;
+using testing::EndsWith;
+using testing::Eq;
+using testing::Field;
+using testing::SizeIs;
+
+using nix::absPath;
+using nix::FormatError;
+using nix::getMachines;
+using nix::Machine;
+using nix::Machines;
+using nix::pathExists;
+using nix::Settings;
+using nix::settings;
+
+class Environment : public ::testing::Environment {
+  public:
+    void SetUp() override { settings.thisSystem = "TEST_ARCH-TEST_OS"; }
+};
+
+testing::Environment* const foo_env =
+    testing::AddGlobalTestEnvironment(new Environment);
+
+TEST(machines, getMachinesWithEmptyBuilders) {
+    settings.builders = "";
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(0));
+}
+
+TEST(machines, getMachinesUriOnly) {
+    settings.builders = "nix@scratchy.labs.cs.uu.nl";
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(1));
+    EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq("ssh://nix@scratchy.labs.cs.uu.nl")));
+    EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("TEST_ARCH-TEST_OS")));
+    EXPECT_THAT(actual[0], Field(&Machine::sshKey, SizeIs(0)));
+    EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(1)));
+    EXPECT_THAT(actual[0], Field(&Machine::speedFactor, Eq(1)));
+    EXPECT_THAT(actual[0], Field(&Machine::supportedFeatures, SizeIs(0)));
+    EXPECT_THAT(actual[0], Field(&Machine::mandatoryFeatures, SizeIs(0)));
+    EXPECT_THAT(actual[0], Field(&Machine::sshPublicHostKey, SizeIs(0)));
+}
+
+TEST(machines, getMachinesDefaults) {
+    settings.builders = "nix@scratchy.labs.cs.uu.nl - - - - - - -";
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(1));
+    EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq("ssh://nix@scratchy.labs.cs.uu.nl")));
+    EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("TEST_ARCH-TEST_OS")));
+    EXPECT_THAT(actual[0], Field(&Machine::sshKey, SizeIs(0)));
+    EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(1)));
+    EXPECT_THAT(actual[0], Field(&Machine::speedFactor, Eq(1)));
+    EXPECT_THAT(actual[0], Field(&Machine::supportedFeatures, SizeIs(0)));
+    EXPECT_THAT(actual[0], Field(&Machine::mandatoryFeatures, SizeIs(0)));
+    EXPECT_THAT(actual[0], Field(&Machine::sshPublicHostKey, SizeIs(0)));
+}
+
+TEST(machines, getMachinesWithNewLineSeparator) {
+    settings.builders = "nix@scratchy.labs.cs.uu.nl\nnix@itchy.labs.cs.uu.nl";
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(2));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl"))));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@itchy.labs.cs.uu.nl"))));
+}
+
+TEST(machines, getMachinesWithSemicolonSeparator) {
+    settings.builders = "nix@scratchy.labs.cs.uu.nl ; nix@itchy.labs.cs.uu.nl";
+    Machines actual = getMachines();
+    EXPECT_THAT(actual, SizeIs(2));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl"))));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@itchy.labs.cs.uu.nl"))));
+}
+
+TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) {
+    settings.builders = "nix@scratchy.labs.cs.uu.nl     i686-linux      "
+                        "/home/nix/.ssh/id_scratchy_auto        8 3 kvm "
+                        "benchmark SSH+HOST+PUBLIC+KEY+BASE64+ENCODED==";
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(1));
+    EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl")));
+    EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("i686-linux")));
+    EXPECT_THAT(actual[0], Field(&Machine::sshKey, Eq("/home/nix/.ssh/id_scratchy_auto")));
+    EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(8)));
+    EXPECT_THAT(actual[0], Field(&Machine::speedFactor, Eq(3)));
+    EXPECT_THAT(actual[0], Field(&Machine::supportedFeatures, ElementsAre("kvm")));
+    EXPECT_THAT(actual[0], Field(&Machine::mandatoryFeatures, ElementsAre("benchmark")));
+    EXPECT_THAT(actual[0], Field(&Machine::sshPublicHostKey, Eq("SSH+HOST+PUBLIC+KEY+BASE64+ENCODED==")));
+}
+
+TEST(machines,
+     getMachinesWithCorrectCompleteSingleBuilderWithTabColumnDelimiter) {
+    settings.builders =
+        "nix@scratchy.labs.cs.uu.nl\ti686-linux\t/home/nix/.ssh/"
+        "id_scratchy_auto\t8\t3\tkvm\tbenchmark\tSSH+HOST+PUBLIC+"
+        "KEY+BASE64+ENCODED==";
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(1));
+    EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl")));
+    EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("i686-linux")));
+    EXPECT_THAT(actual[0], Field(&Machine::sshKey, Eq("/home/nix/.ssh/id_scratchy_auto")));
+    EXPECT_THAT(actual[0], Field(&Machine::maxJobs, Eq(8)));
+    EXPECT_THAT(actual[0], Field(&Machine::speedFactor, Eq(3)));
+    EXPECT_THAT(actual[0], Field(&Machine::supportedFeatures, ElementsAre("kvm")));
+    EXPECT_THAT(actual[0], Field(&Machine::mandatoryFeatures, ElementsAre("benchmark")));
+    EXPECT_THAT(actual[0], Field(&Machine::sshPublicHostKey, Eq("SSH+HOST+PUBLIC+KEY+BASE64+ENCODED==")));
+}
+
+TEST(machines, getMachinesWithMultiOptions) {
+    settings.builders = "nix@scratchy.labs.cs.uu.nl Arch1,Arch2 - - - "
+                        "SupportedFeature1,SupportedFeature2 "
+                        "MandatoryFeature1,MandatoryFeature2";
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(1));
+    EXPECT_THAT(actual[0], Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl")));
+    EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("Arch1", "Arch2")));
+    EXPECT_THAT(actual[0], Field(&Machine::supportedFeatures, ElementsAre("SupportedFeature1", "SupportedFeature2")));
+    EXPECT_THAT(actual[0], Field(&Machine::mandatoryFeatures, ElementsAre("MandatoryFeature1", "MandatoryFeature2")));
+}
+
+TEST(machines, getMachinesWithIncorrectFormat) {
+    settings.builders = "nix@scratchy.labs.cs.uu.nl - - eight";
+    EXPECT_THROW(getMachines(), FormatError);
+    settings.builders = "nix@scratchy.labs.cs.uu.nl - - -1";
+    EXPECT_THROW(getMachines(), FormatError);
+    settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 three";
+    EXPECT_THROW(getMachines(), FormatError);
+    settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 -3";
+    EXPECT_THROW(getMachines(), FormatError);
+    settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 3 - - BAD_BASE64";
+    EXPECT_THROW(getMachines(), FormatError);
+}
+
+TEST(machines, getMachinesWithCorrectFileReference) {
+    auto path = absPath("src/libstore/tests/test-data/machines.valid");
+    ASSERT_TRUE(pathExists(path));
+
+    settings.builders = std::string("@") + path;
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(3));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@scratchy.labs.cs.uu.nl"))));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@itchy.labs.cs.uu.nl"))));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, EndsWith("nix@poochie.labs.cs.uu.nl"))));
+}
+
+TEST(machines, getMachinesWithCorrectFileReferenceToEmptyFile) {
+    auto path = "/dev/null";
+    ASSERT_TRUE(pathExists(path));
+
+    settings.builders = std::string("@") + path;
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(0));
+}
+
+TEST(machines, getMachinesWithIncorrectFileReference) {
+    settings.builders = std::string("@") + absPath("/not/a/file");
+    Machines actual = getMachines();
+    ASSERT_THAT(actual, SizeIs(0));
+}
+
+TEST(machines, getMachinesWithCorrectFileReferenceToIncorrectFile) {
+    settings.builders = std::string("@") + absPath("src/libstore/tests/test-data/machines.bad_format");
+    EXPECT_THROW(getMachines(), FormatError);
+}
diff --git a/src/libstore/tests/test-data/machines.bad_format b/src/libstore/tests/test-data/machines.bad_format
new file mode 100644
index 000000000..7255a1216
--- /dev/null
+++ b/src/libstore/tests/test-data/machines.bad_format
@@ -0,0 +1 @@
+nix@scratchy.labs.cs.uu.nl - - eight
diff --git a/src/libstore/tests/test-data/machines.valid b/src/libstore/tests/test-data/machines.valid
new file mode 100644
index 000000000..1a6c8017c
--- /dev/null
+++ b/src/libstore/tests/test-data/machines.valid
@@ -0,0 +1,3 @@
+nix@scratchy.labs.cs.uu.nl  i686-linux      /home/nix/.ssh/id_scratchy_auto        8 1 kvm
+nix@itchy.labs.cs.uu.nl     i686-linux      /home/nix/.ssh/id_scratchy_auto        8 2
+nix@poochie.labs.cs.uu.nl   i686-linux      /home/nix/.ssh/id_scratchy_auto        1 2 kvm benchmark c3NoLXJzYSBBQUFBQjNOemFDMXljMkVBQUFBREFRQUJBQUFDQVFDWWV5R1laNTNzd1VjMUZNSHBWL1BCcXlKaFR5S1JoRkpWWVRpRHlQN2h5c1JGa0w4VDlLOGdhL2Y2L3c3QjN2SjNHSFRIUFkybENiUEdZbGNLd2h6M2ZRbFNNOEViNi95b3ZLajdvM1FsMEx5Y0dzdGJvRmcwWkZKNldncUxsR0ltS0NobUlxOGZ3TW5ZTWUxbnRQeTBUZFZjSU1tOTV3YzF3SjBMd2c3cEVMRmtHazdkeTVvYnM4a3lGZ0pORDVRSmFwQWJjeWp4Z1QzdzdMcktNZ2xzeWhhd01JNVpkMGZsQTVudW5OZ3pid3plYVhLaUsyTW0vdGJXYTU1YTd4QmNYdHpIZGlPSWdSajJlRWxaMGh5bk10YjBmcklsdmxIcEtLaVFaZ3pQdCtIVXQ2bXpRMkRVME52MGYyYnNSU0krOGpJU2pQcmdlcVVHRldMUzVIUTg2N2xSMlpiaWtyclhZNTdqbVFEZk5DRHY1VFBHZU9UekFEd2pjMDc2aFZ3VFJCd3VTZFhtaWNxTS95b3lrWitkV1dnZ25MenE5QU1tdlNZcDhmZkZDcS9CSDBZNUFXWTFHay9vS3hMVTNaOWt3ZDd2UWNFQWFCQ2dxdnVZRGdTaHE1RlhndDM3OVZESWtEL05ZSTg2QXVvajVDRmVNTzlRM2pJSlRadlh6c1VldjVoSnA2djcxSVh5ODVtbTY5R20zcXdicVE1SjVQZDU1Um56SitpaW5BNjZxTEFSc0Y4amNsSnd5ekFXclBoYU9DRVY2bjVMeVhVazhzMW9EVVR4V1pWN25rVkFTbHJ0MllGcjN5dzdjRTRXQVhsemhHcDhocmdLMVVkMUlyeDVnZWRaSnBWcy9uNWVybmJFMUxmb2x5UHUvRUFIWlh6VGd4dHVDUFNobXc9PQo=
diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc
index 58df9c5ac..92972ed14 100644
--- a/src/libutil/tests/tests.cc
+++ b/src/libutil/tests/tests.cc
@@ -4,6 +4,8 @@
 #include <limits.h>
 #include <gtest/gtest.h>
 
+#include <numeric>
+
 namespace nix {
 
 /* ----------- tests for util.hh ------------------------------------------------*/
@@ -282,6 +284,17 @@ namespace nix {
         ASSERT_EQ(decoded, s);
     }
 
+    TEST(base64Encode, encodeAndDecodeNonPrintable) {
+        char s[256];
+        std::iota(std::rbegin(s), std::rend(s), 0);
+
+        auto encoded = base64Encode(s);
+        auto decoded = base64Decode(encoded);
+
+        EXPECT_EQ(decoded.length(), 255);
+        ASSERT_EQ(decoded, s);
+    }
+
     /* ----------------------------------------------------------------------------
      * base64Decode
      * --------------------------------------------------------------------------*/
@@ -294,6 +307,10 @@ namespace nix {
         ASSERT_EQ(base64Decode("cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="), "quod erat demonstrandum");
     }
 
+    TEST(base64Decode, decodeThrowsOnInvalidChar) {
+        ASSERT_THROW(base64Decode("cXVvZCBlcm_0IGRlbW9uc3RyYW5kdW0="), Error);
+    }
+
     /* ----------------------------------------------------------------------------
      * toLower
      * --------------------------------------------------------------------------*/
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index e99baf1fc..7687e3e01 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -1436,8 +1436,7 @@ std::string filterANSIEscapes(const std::string & s, bool filterAll, unsigned in
 }
 
 
-static char base64Chars[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
-static std::array<char, 256> base64DecodeChars;
+constexpr char base64Chars[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
 
 string base64Encode(std::string_view s)
 {
@@ -1462,12 +1461,15 @@ string base64Encode(std::string_view s)
 
 string base64Decode(std::string_view s)
 {
-    static std::once_flag flag;
-    std::call_once(flag, [](){
-        base64DecodeChars = { (char)-1 };
+    constexpr char npos = -1;
+    constexpr std::array<char, 256> base64DecodeChars = [&]() {
+        std::array<char, 256>  result{};
+        for (auto& c : result)
+            c = npos;
         for (int i = 0; i < 64; i++)
-            base64DecodeChars[(int) base64Chars[i]] = i;
-    });
+            result[base64Chars[i]] = i;
+        return result;
+    }();
 
     string res;
     unsigned int d = 0, bits = 0;
@@ -1477,7 +1479,7 @@ string base64Decode(std::string_view s)
         if (c == '\n') continue;
 
         char digit = base64DecodeChars[(unsigned char) c];
-        if (digit == -1)
+        if (digit == npos)
             throw Error("invalid character in Base64 string: '%c'", c);
 
         bits += 6;