From 10488f743152e2ce811eb594c477cafd5781ae1a Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 28 Oct 2024 18:59:43 +0100 Subject: [PATCH] libstore: use curl content-encoding support, not our own let's use the automatic decoding functions curl provides instead of implementing them ourselves for the dubious ability to support both xz and bzip2 encodings as well, neither of which anything will send Change-Id: I3edfebeb596a0e9d5c986efca9270501c996f2dd --- doc/manual/rl-next/content-encodings.md | 15 +++++++ src/libstore/filetransfer.cc | 58 +++---------------------- 2 files changed, 22 insertions(+), 51 deletions(-) create mode 100644 doc/manual/rl-next/content-encodings.md diff --git a/doc/manual/rl-next/content-encodings.md b/doc/manual/rl-next/content-encodings.md new file mode 100644 index 000000000..00aad5640 --- /dev/null +++ b/doc/manual/rl-next/content-encodings.md @@ -0,0 +1,15 @@ +--- +synopsis: "Drop support for `xz` and `bzip2` Content-Encoding" +category: Miscellany +cls: [2134] +credits: horrors +--- + +Lix no longer supports the non-standard HTTP Content-Encoding values `xz` and `bzip2`. +We do not expect this to cause any problems in practice since these encodings *aren't* +standard, and any server delivering them anyway without being asked to is already well +and truly set on the path of causing inexplicable client breakages. + +Lix's ability to decompress files compressed with `xz` or `bzip2` is unaffected. We're +only bringing Lix more in line with the HTTP standard; all post-transfer data handling +remains as it was before. diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 5a0f6e634..904100f7d 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -4,7 +4,6 @@ #include "store-api.hh" #include "s3.hh" #include "signals.hh" -#include "compression.hh" #include "strings.hh" #include @@ -78,8 +77,6 @@ struct curlFileTransfer : public FileTransfer struct curl_slist * requestHeaders = 0; - std::string encoding; - bool acceptRanges = false; curl_off_t writtenToSink = 0; @@ -127,7 +124,6 @@ struct curlFileTransfer : public FileTransfer if (req == nullptr) { throw FileTransferError(Misc, {}, "could not allocate curl handle"); } - requestHeaders = curl_slist_append(requestHeaders, "Accept-Encoding: zstd, br, gzip, deflate, bzip2, xz"); for (auto it = headers.begin(); it != headers.end(); ++it){ requestHeaders = curl_slist_append(requestHeaders, fmt("%s: %s", it->first, it->second).c_str()); } @@ -244,14 +240,6 @@ struct curlFileTransfer : public FileTransfer result.etag = std::move(etag); } - else if (name == "content-encoding") { - auto encoding = trim(line.substr(i + 1)); - if (!this->encoding.empty() && this->encoding != encoding) { - throwChangedTarget("encoding", this->encoding, encoding); - } - this->encoding = std::move(encoding); - } - else if (name == "accept-ranges" && toLower(trim(line.substr(i + 1))) == "bytes") acceptRanges = true; @@ -340,6 +328,7 @@ struct curlFileTransfer : public FileTransfer curl_easy_setopt(req, CURLOPT_URL, uri.c_str()); curl_easy_setopt(req, CURLOPT_FOLLOWLOCATION, 1L); + curl_easy_setopt(req, CURLOPT_ACCEPT_ENCODING, ""); // all of them! curl_easy_setopt(req, CURLOPT_MAXREDIRS, 10); curl_easy_setopt(req, CURLOPT_NOSIGNAL, 1); curl_easy_setopt(req, CURLOPT_USERAGENT, @@ -405,14 +394,6 @@ struct curlFileTransfer : public FileTransfer debug("finished %s of '%s'; curl status = %d, HTTP status = %d, body = %d bytes", verb(), uri, code, httpStatus, bodySize); - // this has to happen here until we can return an actual future. - // wrapping user `callback`s instead is not possible because the - // Callback api expects std::functions, and copying Callbacks is - // not possible due the promises they hold. - if (code == CURLE_OK && !dataCallback && downloadData.length() > 0) { - downloadData = decompress(encoding, downloadData); - } - if (callbackException) failEx(callbackException); @@ -500,7 +481,7 @@ struct curlFileTransfer : public FileTransfer && attempt < tries && (!this->dataCallback || writtenToSink == 0 - || (acceptRanges && encoding.empty()))) + || acceptRanges)) { int ms = fileTransfer.baseRetryTimeMs * std::pow(2.0f, attempt - 1 + std::uniform_real_distribution<>(0.0, 0.5)(fileTransfer.mt19937)); if (writtenToSink) @@ -819,7 +800,7 @@ struct curlFileTransfer : public FileTransfer struct State { bool done = false, failed = false; std::exception_ptr exc; - std::string data, encoding; + std::string data; std::condition_variable avail, request; }; @@ -853,10 +834,6 @@ struct curlFileTransfer : public FileTransfer state.wait_for(state->request, std::chrono::seconds(10)); } - if (state->encoding.empty()) { - state->encoding = transfer.encoding; - } - /* Append data to the buffer and wake up the calling thread. */ state->data.append(data); @@ -866,15 +843,15 @@ struct curlFileTransfer : public FileTransfer false ); - struct InnerSource : Source + struct DownloadSource : Source { const std::shared_ptr> _state; std::string chunk; std::string_view buffered; - explicit InnerSource(const std::shared_ptr> & state) : _state(state) {} + explicit DownloadSource(const std::shared_ptr> & state) : _state(state) {} - ~InnerSource() + ~DownloadSource() { // wake up the download thread if it's still going and have it abort auto state(_state->lock()); @@ -936,30 +913,9 @@ struct curlFileTransfer : public FileTransfer } }; - struct DownloadSource : Source - { - InnerSource inner; - std::unique_ptr decompressor; - - explicit DownloadSource(const std::shared_ptr> & state) : inner(state) {} - - size_t read(char * data, size_t len) override - { - checkInterrupt(); - - if (!decompressor) { - auto state(inner._state->lock()); - inner.awaitData(state); - decompressor = makeDecompressionSource(state->encoding, inner); - } - - return decompressor->read(data, len); - } - }; - auto source = make_box_ptr(_state); auto lock(_state->lock()); - source->inner.awaitData(lock); + source->awaitData(lock); return source; } };