guiwf/lix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

libstore: don't use curl decompression support

Browse Source 
it's broken with http2 and transfer flow control. cf fj#662

Change-Id: Iaf6312bfcefa18d168faef47f57481199dd30b8d
This commit is contained in:
eldritch horrors
2025-03-08 20:58:34 +01:00
parent 93c3ca4e92
commit bba678e5c5
3 changed files with 101 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
doc/manual/rl-next/curl-http2-decompression-fix.md
+14
View File
@@ -0,0 +1,14 @@
---
synopsis: "Fix curl error `A value or data field grew larger than allowed`"
cls: [2780]
category: Fixes
credits: horrors
---
2.92 started using curl-provided HTTP decompression code, but it as discovered
that curl has [a bug] that effectively breaks its decompression code on HTTP/2
transfers. We have partially rolled back our changes and no longer use builtin
decompression methods provided by curl, but have kept the refusal of bzip2 and
xz content encodings introduced with 2.92 since they are not in the HTTP spec.
[a bug]: https://git.lix.systems/lix-project/lix/issues/662
lix/libstore/filetransfer.cc
+58 -17
View File
@@ -1,4 +1,6 @@
#include "lix/libstore/filetransfer.hh"
#include "lix/libutil/box_ptr.hh"
#include "lix/libutil/compression.hh"
#include "lix/libutil/namespaces.hh"
#include "lix/libstore/globals.hh"
#include "lix/libstore/store-api.hh"
@@ -35,6 +37,14 @@ FileTransferSettings fileTransferSettings;
static GlobalConfig::Register rFileTransferSettings(&fileTransferSettings);
namespace {
struct FileTransferResultWithEncoding : FileTransferResult
{
// empty string means identity (cf makeDecompressionSource)
std::string encoding;
};
}
struct curlFileTransfer : public FileTransfer
{
std::unique_ptr<CURLM, decltype([](auto * m) { curl_multi_cleanup(m); })> curlm;
@@ -51,13 +61,13 @@ struct curlFileTransfer : public FileTransfer
};
std::string uri;
FileTransferResult result;
FileTransferResultWithEncoding result;
Activity act;
std::unique_ptr<FILE, decltype([](FILE * f) { fclose(f); })> uploadData;
Sync<DownloadState> downloadState;
std::condition_variable downloadEvent;
bool headersDone = false, metadataReturned = false;
std::promise<FileTransferResult> metadataPromise;
std::promise<FileTransferResultWithEncoding> metadataPromise;
std::string statusMsg;
uint64_t bodySize = 0;
@@ -84,6 +94,17 @@ struct curlFileTransfer : public FileTransfer
return uploadData ? "upload" : "download";
}
void appendCurlHeader(std::string_view name, std::string_view value)
{
auto header = fmt("%s: %s", name, value);
if (auto next = curl_slist_append(requestHeaders.get(), header.c_str())) {
(void) requestHeaders.release(); // next now owns this pointer
requestHeaders.reset(next);
} else {
throw FileTransferError(Misc, {}, "could not allocate curl request headers");
}
}
TransferItem(const std::string & uri,
const Headers & headers,
ActivityId parentAct,
@@ -100,17 +121,8 @@ struct curlFileTransfer : public FileTransfer
if (req == nullptr) {
throw FileTransferError(Misc, {}, "could not allocate curl handle");
}
for (auto it = headers.begin(); it != headers.end(); ++it){
if (auto next = curl_slist_append(
requestHeaders.get(), fmt("%s: %s", it->first, it->second).c_str()
);
next != nullptr)
{
(void) requestHeaders.release(); // next now owns this pointer
requestHeaders.reset(next);
} else {
throw FileTransferError(Misc, {}, "could not allocate curl request headers");
}
for (auto it = headers.begin(); it != headers.end(); ++it) {
appendCurlHeader(it->first, it->second);
}
if (verbosity >= lvlVomit) {
@@ -120,7 +132,14 @@ struct curlFileTransfer : public FileTransfer
curl_easy_setopt(req.get(), CURLOPT_URL, uri.c_str());
curl_easy_setopt(req.get(), CURLOPT_FOLLOWLOCATION, 1L);
curl_easy_setopt(req.get(), CURLOPT_ACCEPT_ENCODING, ""); // all of them!
{
// curl builtin decompression disabled due to bugs, instead we add
// an accept-encoding header of our own and decompress manually :(
// we don't support deflate because libarchive also doesn't either
// cf https://git.lix.systems/lix-project/lix/issues/662 for infos
// curl_easy_setopt(req.get(), CURLOPT_ACCEPT_ENCODING, ""); // all of them!
appendCurlHeader("Accept-Encoding", "gzip, br, zstd");
}
curl_easy_setopt(req.get(), CURLOPT_MAXREDIRS, 10);
curl_easy_setopt(req.get(), CURLOPT_NOSIGNAL, 1);
curl_easy_setopt(req.get(), CURLOPT_USERAGENT,
@@ -270,6 +289,7 @@ struct curlFileTransfer : public FileTransfer
static std::regex statusLine("HTTP/[^ ]+ +[0-9]+(.*)", std::regex::extended | std::regex::icase);
if (std::smatch match; std::regex_match(line, match, statusLine)) {
statusMsg = trim(match.str(1));
result.encoding = "";
} else {
auto i = line.find(':');
if (i != std::string::npos) {
@@ -290,6 +310,10 @@ struct curlFileTransfer : public FileTransfer
} else
debug("got invalid link header '%s'", value);
}
else if (name == "content-encoding") {
result.encoding = trim(line.substr(i + 1));
}
}
}
return realSize;
@@ -765,7 +789,7 @@ struct curlFileTransfer : public FileTransfer
auto source = make_box_ptr<TransferSource>(*this, uri, headers, std::move(data), noBody);
source->awaitData();
return {source->metadata, std::move(source)};
return {source->metadata, make_box_ptr<DecompressionWrapper>(std::move(source))};
}
struct TransferSource : Source
@@ -778,7 +802,7 @@ struct curlFileTransfer : public FileTransfer
ActivityId parentAct = getCurActivity();
std::shared_ptr<TransferItem> transfer;
FileTransferResult metadata;
FileTransferResultWithEncoding metadata;
std::string chunk;
std::string_view buffered;
@@ -840,7 +864,7 @@ struct curlFileTransfer : public FileTransfer
}
}
FileTransferResult startTransfer(const std::string & uri, curl_off_t offset = 0)
FileTransferResultWithEncoding startTransfer(const std::string & uri, curl_off_t offset = 0)
{
attempt += 1;
auto uploadData = data ? std::optional(std::string_view(*data)) : std::nullopt;
@@ -887,6 +911,7 @@ struct curlFileTransfer : public FileTransfer
metadata.immutableUrl.value_or(""),
newMeta.immutableUrl.value_or("")
);
throwChangedTarget("compression", metadata.encoding, newMeta.encoding);
}
bool awaitData()
@@ -938,6 +963,22 @@ struct curlFileTransfer : public FileTransfer
}
};
struct DecompressionWrapper : Source
{
box_ptr<TransferSource> wrapped;
std::unique_ptr<Source> decompressor;
explicit DecompressionWrapper(box_ptr<TransferSource> inner) : wrapped(std::move(inner)) {}
size_t read(char * data, size_t len) override
{
if (!decompressor) {
decompressor = makeDecompressionSource(wrapped->metadata.encoding, *wrapped);
}
return decompressor->read(data, len);
}
};
bool exists(const std::string & uri, const Headers & headers) override
{
try {
tests/unit/libstore/filetransfer.cc
+29 -3
View File
@@ -25,6 +25,7 @@
#endif
using namespace std::chrono_literals;
using namespace std::string_literals;
namespace {
@@ -257,12 +258,16 @@ TEST(FileTransfer, NOT_ON_DARWIN(defersFailures))
ASSERT_THROW(src->drain(), FileTransferError);
}
TEST(FileTransfer, NOT_ON_DARWIN(handlesContentEncoding))
class FileTransferEncoding : public testing::TestWithParam<std::pair<std::string, std::string>>
{};
TEST_P(FileTransferEncoding, NOT_ON_DARWIN(handlesContentEncoding))
{
std::string original = "Test data string";
std::string compressed = compress("gzip", original);
auto [method, compressed] = GetParam();
auto [port, srv] = serveHTTP("200 ok", "content-encoding: gzip\r\n", [&] { return compressed; });
auto [port, srv] =
serveHTTP("200 ok", "content-encoding: " + method + "\r\n", [&] { return compressed; });
auto ft = makeFileTransfer();
StringSink sink;
@@ -270,6 +275,27 @@ TEST(FileTransfer, NOT_ON_DARWIN(handlesContentEncoding))
EXPECT_EQ(sink.s, original);
}
INSTANTIATE_TEST_SUITE_P(
,
FileTransferEncoding,
testing::Values(
std::pair{
"gzip",
"\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03\x0b\x49\x2d\x2e\x51\x48\x49\x2c\x49\x54\x28"
"\x2e\x29\xca\xcc\x4b\x07\x00\x34\xfd\xff\xfa\x10\x00\x00\x00"s
},
std::pair{
"zstd",
"\x28\xb5\x2f\xfd\x04\x58\x81\x00\x00\x54\x65\x73\x74\x20\x64\x61\x74\x61\x20\x73\x74"
"\x72\x69\x6e\x67\x5e\xc9\x0e\xca"s
},
std::pair{
"br",
"\x8f\x07\x80\x54\x65\x73\x74\x20\x64\x61\x74\x61\x20\x73\x74\x72\x69\x6e\x67\x03"s
}
)
);
TEST(FileTransfer, usesIntermediateLinkHeaders)
{
auto [port, srv] = serveHTTP({