diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index cce1971ff..2aedb8a2e 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -193,7 +193,7 @@ std::pair Input::fetchToStore(ref store) const assert(final.isFinal()); - scheme->checkLocks(*this, final); + checkLocks(*this, final); return {storePath, final}; } catch (Error & e) { @@ -205,8 +205,35 @@ std::pair Input::fetchToStore(ref store) const return {std::move(storePath), input}; } -void InputScheme::checkLocks(const Input & specified, const Input & final) const +void Input::checkLocks(Input specified, Input & final) { + /* If the original input is final, then we just return the + original attributes, dropping any new fields returned by the + fetcher. However, any fields that are in both the original and + final input must be identical. */ + if (specified.isFinal()) { + + /* Backwards compatibility hack: we had some lock files in the + past that 'narHash' fields with incorrect base-64 + formatting (lacking the trailing '=', e.g. 'sha256-ri...Mw' + instead of ''sha256-ri...Mw='). So fix that. */ + if (auto prevNarHash = specified.getNarHash()) + specified.attrs.insert_or_assign("narHash", prevNarHash->to_string(HashFormat::SRI, true)); + + for (auto & field : specified.attrs) { + auto field2 = final.attrs.find(field.first); + if (field2 != final.attrs.end() && field.second != field2->second) + throw Error("mismatch in field '%s' of final input '%s', got '%s'", + field.first, + attrsToJSON(specified.attrs), + attrsToJSON(final.attrs)); + } + + final.attrs = specified.attrs; + + return; + } + if (auto prevNarHash = specified.getNarHash()) { if (final.getNarHash() != prevNarHash) { if (final.getNarHash()) @@ -235,23 +262,6 @@ void InputScheme::checkLocks(const Input & specified, const Input & final) const throw Error("'revCount' attribute mismatch in input '%s', expected %d", final.to_string(), *prevRevCount); } - - /* If the original input is final, then the result must be the - same (i.e. cannot remove, add or change fields. */ - if (specified.isFinal()) { - - /* Backwards compatibility hack: we had some lock files in the - past that 'narHash' fields with incorrect base-64 - formatting (lacking the trailing '=', e.g. 'sha256-ri...Mw' - instead of ''sha256-ri...Mw='). So fix */ - auto specified2 = specified; - if (auto prevNarHash = specified2.getNarHash()) - specified2.attrs.insert_or_assign("narHash", prevNarHash->to_string(HashFormat::SRI, true)); - - if (specified2.attrs != final.attrs) - throw Error("fetching final input '%s' resulted in different input '%s'", - attrsToJSON(specified2.attrs), attrsToJSON(final.attrs)); - } } std::pair, Input> Input::getAccessor(ref store) const @@ -259,7 +269,7 @@ std::pair, Input> Input::getAccessor(ref store) const try { auto [accessor, final] = getAccessorUnchecked(store); - scheme->checkLocks(*this, final); + checkLocks(*this, final); return {accessor, std::move(final)}; } catch (Error & e) { diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index ce535e6fe..430d6e943 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -91,9 +91,9 @@ public: bool isLocked() const; /** - * Return whether this is a "final" input, meaning that fetching it - * will not add or change any attributes. For instance, a Git - * input with a `rev` attribute but without a `lastModified` + * Return whether this is a "final" input, meaning that fetching + * it will not add, remove or change any attributes. For instance, + * a Git input with a `rev` attribute but without a `lastModified` * attribute is considered locked but not final. Only "final" * inputs can be substituted from a binary cache. * @@ -113,6 +113,19 @@ public: */ std::pair fetchToStore(ref store) const; + /** + * Check the locking attributes in `final` against + * `specified`. E.g. if `specified` has a `rev` attribute, then + * `final` must have the same `rev` attribute. Throw an exception + * if there is a mismatch. + * + * If `specified` is marked final (i.e. has the `__final` + * attribute), then the intersection of attributes in `specified` + * and `final` must be equal, and `final.attrs` is set to + * `specified.attrs` (i.e. we discard any new attributes). + */ + static void checkLocks(Input specified, Input & final); + /** * Return a `SourceAccessor` that allows access to files in the * input without copying it to the store. Also return a possibly @@ -238,14 +251,6 @@ struct InputScheme virtual bool isLocked(const Input & input) const { return false; } - - /** - * Check the locking attributes in `final` against - * `specified`. E.g. if `specified` has a `rev` attribute, then - * `final` must have the same `rev` attribute. Throw an exception - * if there is a mismatch. - */ - virtual void checkLocks(const Input & specified, const Input & final) const; }; void registerInputScheme(std::shared_ptr && fetcher); diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index e723d3061..27ad89b6e 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -384,13 +384,11 @@ struct TarballInputScheme : CurlInputScheme input = immutableInput; } - /* If we got a lastModified, then return it. But for - compatibility with old lock files that didn't include - lastModified, don't do this if the original input was final - and didn't contain a lastModified. */ - if (result.lastModified - && !input.attrs.contains("lastModified") - && (!_input.isFinal() || _input.attrs.contains("lastModified"))) + /* If we got a lastModified and the input is not final and + doesn't have one, then return it. Note that we don't do + this if the input is final for compatibility with old lock + files that didn't include lastModified. */ + if (result.lastModified && !_input.isFinal() && !input.attrs.contains("lastModified")) input.attrs.insert_or_assign("lastModified", uint64_t(result.lastModified)); input.attrs.insert_or_assign("narHash",