From a5521b7d9445af63a159d4fe7b44a0902c3a2a24 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 7 Dec 2023 10:49:29 -0500 Subject: [PATCH] Factor out `ServeProto::Serialiser` and test In the process, partially undo e89b5bd0bfeb4dfdd8fe7e6929544cb9ceb8a505 in that the ancient < 2.4 version is now supported again by the serializer again. `LegacySSHStore`, instead of also asserting that the version is at least 4, just checks that `narHash` is set. This allows us to better test the serializer in isolation for both versions (< 4 and >= 4). --- src/libstore/legacy-ssh-store.cc | 22 ++--- src/libstore/serve-protocol.cc | 44 ++++++++++ src/libstore/serve-protocol.hh | 3 + src/nix-store/nix-store.cc | 12 +-- .../unkeyed-valid-path-info-2.3.bin | Bin 0 -> 184 bytes .../unkeyed-valid-path-info-2.4.bin | Bin 0 -> 648 bytes tests/unit/libstore/serve-protocol.cc | 77 ++++++++++++++++++ 7 files changed, 131 insertions(+), 27 deletions(-) create mode 100644 tests/unit/libstore/data/serve-protocol/unkeyed-valid-path-info-2.3.bin create mode 100644 tests/unit/libstore/data/serve-protocol/unkeyed-valid-path-info-2.4.bin diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index fb1580dd6..277445ee6 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -172,24 +172,12 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor if (p.empty()) return callback(nullptr); auto path2 = parseStorePath(p); assert(path == path2); - /* Hash will be set below. FIXME construct ValidPathInfo at end. */ - auto info = std::make_shared(path, Hash::dummy); + auto info = std::make_shared( + path, + ServeProto::Serialise::read(*this, *conn)); - auto deriver = readString(conn->from); - if (deriver != "") - info->deriver = parseStorePath(deriver); - info->references = ServeProto::Serialise::read(*this, *conn); - readLongLong(conn->from); // download size - info->narSize = readLongLong(conn->from); - - { - auto s = readString(conn->from); - if (s == "") - throw Error("NAR hash is now mandatory"); - info->narHash = Hash::parseAnyPrefixed(s); - } - info->ca = ContentAddress::parseOpt(readString(conn->from)); - info->sigs = readStrings(conn->from); + if (info->narHash == Hash::dummy) + throw Error("NAR hash is now mandatory"); auto s = readString(conn->from); assert(s == ""); diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index fb33553c5..c37b3095c 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -5,6 +5,7 @@ #include "serve-protocol.hh" #include "serve-protocol-impl.hh" #include "archive.hh" +#include "path-info.hh" #include @@ -54,4 +55,47 @@ void ServeProto::Serialise::write(const StoreDirConfig & store, Ser } } + +UnkeyedValidPathInfo ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + /* Hash should be set below unless very old `nix-store --serve`. + Caller should assert that it did set it. */ + UnkeyedValidPathInfo info { Hash::dummy }; + + auto deriver = readString(conn.from); + if (deriver != "") + info.deriver = store.parseStorePath(deriver); + info.references = ServeProto::Serialise::read(store, conn); + + readLongLong(conn.from); // download size, unused + info.narSize = readLongLong(conn.from); + + if (GET_PROTOCOL_MINOR(conn.version) >= 4) { + auto s = readString(conn.from); + if (!s.empty()) + info.narHash = Hash::parseAnyPrefixed(s); + info.ca = ContentAddress::parseOpt(readString(conn.from)); + info.sigs = readStrings(conn.from); + } + + return info; +} + +void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const UnkeyedValidPathInfo & info) +{ + conn.to + << (info.deriver ? store.printStorePath(*info.deriver) : ""); + + ServeProto::write(store, conn, info.references); + // !!! Maybe we want compression? + conn.to + << info.narSize // downloadSize, lie a little + << info.narSize; + if (GET_PROTOCOL_MINOR(conn.version) >= 4) + conn.to + << info.narHash.to_string(HashFormat::Nix32, true) + << renderContentAddress(info.ca) + << info.sigs; +} + } diff --git a/src/libstore/serve-protocol.hh b/src/libstore/serve-protocol.hh index 6e9d66e2d..ada67a149 100644 --- a/src/libstore/serve-protocol.hh +++ b/src/libstore/serve-protocol.hh @@ -18,6 +18,7 @@ struct Source; // items being serialised struct BuildResult; +struct UnkeyedValidPathInfo; /** @@ -141,6 +142,8 @@ inline std::ostream & operator << (std::ostream & s, ServeProto::Command op) template<> DECLARE_SERVE_SERIALISER(BuildResult); +template<> +DECLARE_SERVE_SERIALISER(UnkeyedValidPathInfo); template DECLARE_SERVE_SERIALISER(std::vector); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index db45be2a8..45af7879c 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -891,16 +891,8 @@ static void opServe(Strings opFlags, Strings opArgs) for (auto & i : paths) { try { auto info = store->queryPathInfo(i); - out << store->printStorePath(info->path) - << (info->deriver ? store->printStorePath(*info->deriver) : ""); - ServeProto::write(*store, wconn, info->references); - // !!! Maybe we want compression? - out << info->narSize // downloadSize - << info->narSize; - if (GET_PROTOCOL_MINOR(clientVersion) >= 4) - out << info->narHash.to_string(HashFormat::Nix32, true) - << renderContentAddress(info->ca) - << info->sigs; + out << store->printStorePath(info->path); + ServeProto::write(*store, wconn, static_cast(*info)); } catch (InvalidPath &) { } } diff --git a/tests/unit/libstore/data/serve-protocol/unkeyed-valid-path-info-2.3.bin b/tests/unit/libstore/data/serve-protocol/unkeyed-valid-path-info-2.3.bin new file mode 100644 index 0000000000000000000000000000000000000000..8056ec055ed2039814ab2654aad984f07115518f GIT binary patch literal 184 zcmZQzKm~Rk5I&4HhDz(_Wmf1Hm*f|v>Zco)n`cxS7viFIlM;*cQi{sJIvJt*ahg+E XS&435ArJt~*Gcp=fmdD+P%du3+9=$K@X zTQa|lwn`cL3n8wg;O8!IE8WDiY-4aNzU{MP{{~7(rMCb8 literal 0 HcmV?d00001 diff --git a/tests/unit/libstore/serve-protocol.cc b/tests/unit/libstore/serve-protocol.cc index 6d2054f7d..c2298c6db 100644 --- a/tests/unit/libstore/serve-protocol.cc +++ b/tests/unit/libstore/serve-protocol.cc @@ -225,6 +225,83 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + unkeyedValidPathInfo_2_3, + "unkeyed-valid-path-info-2.3", + 2 << 8 | 3, + (std::tuple { + ({ + UnkeyedValidPathInfo info { Hash::dummy }; + info.narSize = 34878; + info; + }), + ({ + UnkeyedValidPathInfo info { Hash::dummy }; + info.deriver = StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }; + info.references = { + StorePath { + "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo.drv", + }, + }; + info.narSize = 34878; + info; + }), + })) + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, + unkeyedValidPathInfo_2_4, + "unkeyed-valid-path-info-2.4", + 2 << 8 | 4, + (std::tuple { + ({ + UnkeyedValidPathInfo info { + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }; + info.deriver = StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }; + info.references = { + StorePath { + "g1w7hyyyy1w7hy3qg1w7hy3qgqqqqy3q-foo.drv", + }, + }; + info.narSize = 34878; + info; + }), + ({ + ValidPathInfo info { + *LibStoreTest::store, + "foo", + FixedOutputInfo { + .method = FileIngestionMethod::Recursive, + .hash = hashString(HashAlgorithm::SHA256, "(...)"), + .references = { + .others = { + StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar", + }, + }, + .self = true, + }, + }, + Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + }; + info.deriver = StorePath { + "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + }; + info.narSize = 34878; + info.sigs = { + "fake-sig-1", + "fake-sig-2", + }, + static_cast(std::move(info)); + }), + })) + VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, vector,