From d89400d0521e452750a04a3465c39b7941a511b9 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 23 Feb 2026 14:24:33 +0300 Subject: [PATCH 1/2] libstore/filetransfer: Use bitfields for boolean flags in TransferItem Wastes slightly less memory. Also gets the doc-comments in order. --- src/libstore/filetransfer.cc | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 8e055c141..08270cade 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -88,14 +88,10 @@ struct curlFileTransfer : public FileTransfer FileTransferRequest request; FileTransferResult result; std::unique_ptr _act; - bool done = false; // whether either the success or failure function has been called Callback callback; CURL * req = 0; // buffer to accompany the `req` above char errbuf[CURL_ERROR_SIZE]; - bool active = false; // whether the handle has been added to the multi object - bool paused = false; // whether the request has been paused previously - bool enqueued = false; // whether the request has been added the incoming queue std::string statusMsg; unsigned int attempt = 0; @@ -108,7 +104,30 @@ struct curlFileTransfer : public FileTransfer std::string encoding; - bool acceptRanges = false; + /** + * Whether either the success or failure function has been called. + */ + bool done:1 = false; + + /** + * Whether the handle has been added to the multi object. + */ + bool active:1 = false; + + /** + * Whether the request has been paused previously. + */ + bool paused:1 = false; + + /** + * Whether the request has been added the incoming queue. + */ + bool enqueued:1 = false; + + /** + * Whether we can use range downloads for retries. + */ + bool acceptRanges:1 = false; curl_off_t writtenToSink = 0; From fa07d9d05533d2299c1cdce07e6c1975572cadc7 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 25 Feb 2026 20:44:35 +0300 Subject: [PATCH 2/2] filetransfer: Use libcurl for Content-Encoding/Transfer-Encoding transparent decompression Get rid of manual decompression entirely. We can support zstd, br, deflate (which was broken previously) and gzip/x-gzip encodings via curl without manual work on our side. This is the same approach that was taken by lix. Also adds some tests for x-gzip support. --- .../rl-next/libcurl-content-encoding.md | 10 +++ packaging/dependencies.nix | 16 +++- src/libstore/filetransfer.cc | 73 ++++++++----------- src/libstore/meson.build | 15 +--- tests/nixos/content-encoding.nix | 4 +- tests/nixos/s3-binary-cache-store.nix | 16 ++-- 6 files changed, 69 insertions(+), 65 deletions(-) create mode 100644 doc/manual/rl-next/libcurl-content-encoding.md diff --git a/doc/manual/rl-next/libcurl-content-encoding.md b/doc/manual/rl-next/libcurl-content-encoding.md new file mode 100644 index 000000000..85ee03363 --- /dev/null +++ b/doc/manual/rl-next/libcurl-content-encoding.md @@ -0,0 +1,10 @@ +--- +synopsis: Content-Encoding decompression is now handled by libcurl +prs: [15336] +issues: [14324] +--- + +Transparent decompression of HTTP downloads specifying `Content-Encoding` header now uses libcurl. This adds support for previously advertised, but not supported `deflate` encoding as well as deprecated `x-gzip` alias. +Non-standard `xz`, `bzip2` encodings that were previously advertised are no longer supported, as they do not commonly appear in the wild and should not be sent by compliant servers. + +`br`, `zstd`, `gzip` continue to be supported. Distro packaging should ensure that the `libcurl` dependency is linked against required libraries to support these encodings. By default now the build system requires libcurl >= 8.17.0 which is not known to have issues around [pausing and decompression](https://github.com/curl/curl/issues/16280). diff --git a/packaging/dependencies.nix b/packaging/dependencies.nix index c08b23a1e..5338f70e5 100644 --- a/packaging/dependencies.nix +++ b/packaging/dependencies.nix @@ -30,9 +30,19 @@ scope: { NIX_CFLAGS_COMPILE = "-DINITIAL_MARK_STACK_SIZE=1048576"; }); - curl = pkgs.curl.override { - http3Support = !pkgs.stdenv.hostPlatform.isWindows; - }; + curl = + (pkgs.curl.override { + http3Support = !pkgs.stdenv.hostPlatform.isWindows; + # Make sure we enable all the dependencies for Content-Encoding/Transfer-Encoding decompression. + zstdSupport = true; + brotliSupport = true; + zlibSupport = true; + }).overrideAttrs + { + # TODO: Fix in nixpkgs. Static build with brotli is marked as broken, but it's not the case. + # Remove once https://github.com/NixOS/nixpkgs/pull/494111 lands in the 25.11 channel. + meta.broken = false; + }; libblake3 = pkgs.libblake3.override { useTBB = !(stdenv.hostPlatform.isWindows || stdenv.hostPlatform.isStatic); diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 08270cade..ba86e9ebb 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -2,7 +2,6 @@ #include "nix/store/globals.hh" #include "nix/util/config-global.hh" #include "nix/store/store-api.hh" -#include "nix/util/compression.hh" #include "nix/util/finally.hh" #include "nix/util/callback.hh" #include "nix/util/signals.hh" @@ -102,8 +101,6 @@ struct curlFileTransfer : public FileTransfer curlSList requestHeaders; - std::string encoding; - /** * Whether either the success or failure function has been called. */ @@ -129,6 +126,11 @@ struct curlFileTransfer : public FileTransfer */ bool acceptRanges:1 = false; + /** + * Whether the response has a non-trivial (not "identity") Content-Encoding. + */ + bool hasContentEncoding:1 = false; + curl_off_t writtenToSink = 0; std::chrono::steady_clock::time_point startTime = std::chrono::steady_clock::now(); @@ -188,15 +190,6 @@ struct curlFileTransfer : public FileTransfer { result.urls.push_back(request.uri.to_string()); - /* Don't set Accept-Encoding for S3 requests that use AWS SigV4 signing. - curl's SigV4 implementation signs all headers including Accept-Encoding, - but some S3-compatible services (like GCS) modify this header in transit, - causing signature verification to fail. - See https://github.com/NixOS/nix/issues/15019 */ -#if NIX_WITH_AWS_AUTH - if (!request.awsSigV4Provider) -#endif - appendHeaders("Accept-Encoding: zstd, br, gzip, deflate, bzip2, xz"); if (!request.expectedETag.empty()) appendHeaders("If-None-Match: " + request.expectedETag); if (!request.mimeType.empty()) @@ -244,7 +237,6 @@ struct curlFileTransfer : public FileTransfer } LambdaSink finalSink; - std::shared_ptr decompressionSink; std::optional errorSink; std::exception_ptr callbackException; @@ -254,18 +246,15 @@ struct curlFileTransfer : public FileTransfer size_t realSize = size * nmemb; result.bodySize += realSize; - if (!decompressionSink) { - decompressionSink = makeDecompressionSink(encoding, finalSink); - if (!successfulStatuses.count(getHTTPStatus())) { - // In this case we want to construct a TeeSink, to keep - // the response around (which we figure won't be big - // like an actual download should be) to improve error - // messages. - errorSink = StringSink{}; - } + if (!errorSink && !successfulStatuses.count(getHTTPStatus())) { + // In this case we want to construct a TeeSink, to keep + // the response around (which we figure won't be big + // like an actual download should be) to improve error + // messages. + errorSink = StringSink{}; } - (*decompressionSink)({(char *) contents, realSize}); + finalSink({static_cast(contents), realSize}); if (paused) { /* The callback has signaled that the transfer needs to be paused. Already consumed data won't be returned twice unlike @@ -307,7 +296,7 @@ struct curlFileTransfer : public FileTransfer result.bodySize = 0; statusMsg = trim(match.str(1)); acceptRanges = false; - encoding = ""; + hasContentEncoding = false; appendCurrentUrl(); } else { @@ -330,8 +319,10 @@ struct curlFileTransfer : public FileTransfer } } - else if (name == "content-encoding") - encoding = trim(line.substr(i + 1)); + else if (name == "content-encoding") { + auto value = toLower(trim(line.substr(i + 1))); + hasContentEncoding = !value.empty() && value != "identity"; + } else if (name == "accept-ranges" && toLower(trim(line.substr(i + 1))) == "bytes") acceptRanges = true; @@ -349,14 +340,10 @@ struct curlFileTransfer : public FileTransfer } return realSize; } catch (...) { -#if LIBCURL_VERSION_NUM >= 0x075700 /* https://curl.se/libcurl/c/CURLOPT_HEADERFUNCTION.html: You can also abort the transfer by returning CURL_WRITEFUNC_ERROR. */ callbackException = std::current_exception(); return CURL_WRITEFUNC_ERROR; -#else - return realSize; -#endif } static size_t headerCallbackWrapper(void * contents, size_t size, size_t nmemb, void * userp) @@ -494,6 +481,16 @@ struct curlFileTransfer : public FileTransfer } curl_easy_setopt(req, CURLOPT_URL, request.uri.to_string().c_str()); + + /* Enable transparent decompression for downloads. + Skip for uploads (Accept-Encoding is meaningless when sending data) + and when resuming from an offset (byte ranges don't work with + compressed content). */ + if (writtenToSink == 0 && !request.data) + /* Empty string means to enable all supported (that libcurl has + been linked to support) encodings. */ + curl_easy_setopt(req, CURLOPT_ACCEPT_ENCODING, ""); + curl_easy_setopt(req, CURLOPT_FOLLOWLOCATION, 1L); curl_easy_setopt(req, CURLOPT_MAXREDIRS, 10); curl_easy_setopt(req, CURLOPT_NOSIGNAL, 1); @@ -640,14 +637,6 @@ struct curlFileTransfer : public FileTransfer appendCurrentUrl(); - if (decompressionSink) { - try { - decompressionSink->finish(); - } catch (...) { - callbackException = std::current_exception(); - } - } - if (code == CURLE_WRITE_ERROR && result.etag == request.expectedETag) { code = CURLE_OK; httpStatus = 304; @@ -665,7 +654,9 @@ struct curlFileTransfer : public FileTransfer if (httpStatus == 304 && result.etag == "") result.etag = request.expectedETag; - act().progress(result.bodySize, result.bodySize); + curl_off_t dlSize = 0; + curl_easy_getinfo(req, CURLINFO_SIZE_DOWNLOAD_T, &dlSize); + act().progress(dlSize, dlSize); done = true; callback(std::move(result)); } @@ -714,6 +705,7 @@ struct curlFileTransfer : public FileTransfer case CURLE_TOO_MANY_REDIRECTS: case CURLE_WRITE_ERROR: case CURLE_UNSUPPORTED_PROTOCOL: + case CURLE_BAD_CONTENT_ENCODING: err = Misc; break; default: // Shut up warnings @@ -757,7 +749,7 @@ struct curlFileTransfer : public FileTransfer sink, we can only retry if the server supports ranged requests. */ if (err == Transient && attempt < fileTransfer.settings.tries - && (!this->request.dataCallback || writtenToSink == 0 || (acceptRanges && encoding.empty()))) { + && (!this->request.dataCallback || writtenToSink == 0 || (acceptRanges && !hasContentEncoding))) { int ms = retryTimeMs * std::pow( 2.0f, attempt - 1 + std::uniform_real_distribution<>(0.0, 0.5)(fileTransfer.mt19937)); @@ -765,7 +757,6 @@ struct curlFileTransfer : public FileTransfer warn("%s; retrying from offset %d in %d ms", exc.what(), writtenToSink, ms); else warn("%s; retrying in %d ms", exc.what(), ms); - decompressionSink.reset(); errorSink.reset(); embargo = std::chrono::steady_clock::now() + std::chrono::milliseconds(ms); try { diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 4d9b22f56..9e52517d4 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -115,17 +115,10 @@ boost = dependency( # put in `deps_other`. deps_other += boost -curl = dependency('libcurl', 'curl', version : '>= 7.75.0') -if curl.version().version_compare('>=8.16.0') and curl.version().version_compare( - '<8.17.0', -) - # Out of precaution, avoid building with libcurl version that suffer from https://github.com/curl/curl/issues/19334. - error( - 'curl @0@ has issues with write pausing, please use libcurl < 8.16 or >= 8.17, see https://github.com/curl/curl/issues/19334'.format( - curl.version(), - ), - ) -endif +# This is quite new, but curl has a bunch of known issues with write pausing and decompression. +# Please use libcurl >= 8.17. See https://github.com/curl/curl/issues/19334, https://github.com/curl/curl/issues/16280, https://github.com/curl/curl/issues/16955 if in doubt. +# Patch out this check at your own risk. +curl = dependency('libcurl', 'curl', version : '>= 8.17.0') deps_private += curl diff --git a/tests/nixos/content-encoding.nix b/tests/nixos/content-encoding.nix index debee377b..5faca035f 100644 --- a/tests/nixos/content-encoding.nix +++ b/tests/nixos/content-encoding.nix @@ -82,7 +82,7 @@ in # Serve .narinfo files with gzip encoding location ~ \.narinfo$ { - add_header Content-Encoding gzip; + add_header Content-Encoding x-gzip; default_type "text/x-nix-narinfo"; } @@ -172,7 +172,7 @@ in # Check Content-Encoding headers on the download endpoint narinfo_headers = machine.succeed(f"curl -I http://localhost/cache/{narinfoHash}.narinfo 2>&1") - assert "content-encoding: gzip" in narinfo_headers.lower(), f"Expected 'content-encoding: gzip' for .narinfo file, but headers were: {narinfo_headers}" + assert "content-encoding: x-gzip" in narinfo_headers.lower(), f"Expected 'content-encoding: x-gzip' for .narinfo file, but headers were: {narinfo_headers}" ls_headers = machine.succeed(f"curl -I http://localhost/cache/{narinfoHash}.ls 2>&1") assert "content-encoding: gzip" in ls_headers.lower(), f"Expected 'content-encoding: gzip' for .ls file, but headers were: {ls_headers}" diff --git a/tests/nixos/s3-binary-cache-store.nix b/tests/nixos/s3-binary-cache-store.nix index df1365510..b2a86643d 100644 --- a/tests/nixos/s3-binary-cache-store.nix +++ b/tests/nixos/s3-binary-cache-store.nix @@ -128,7 +128,7 @@ in def verify_no_compression(machine, bucket, object_path): """Verify S3 object has no compression headers""" stat = machine.succeed(f"mc stat minio/{bucket}/{object_path}") - if "Content-Encoding" in stat and ("gzip" in stat or "xz" in stat): + if "Content-Encoding" in stat and ("gzip" in stat or "br" in stat): print(f"mc stat output for {object_path}:") print(stat) raise Exception(f"Object {object_path} should not have compression Content-Encoding") @@ -537,20 +537,20 @@ in @setup_s3() def test_compression_mixed(bucket): - """Test mixed compression (narinfo=xz, ls=gzip)""" - print("\n=== Testing Compression: mixed (narinfo=xz, ls=gzip) ===") + """Test mixed compression (narinfo=br, ls=gzip)""" + print("\n=== Testing Compression: mixed (narinfo=br, ls=gzip) ===") store_url = make_s3_url( bucket, - **{'narinfo-compression': 'xz', 'write-nar-listing': 'true', 'ls-compression': 'gzip'} + **{'narinfo-compression': 'br', 'write-nar-listing': 'true', 'ls-compression': 'gzip'} ) server.succeed(f"{ENV_WITH_CREDS} nix copy --to '{store_url}' {PKGS['C']}") pkg_hash = get_package_hash(PKGS['C']) - # Verify .narinfo has xz compression - verify_content_encoding(server, bucket, f"{pkg_hash}.narinfo", "xz") + # Verify .narinfo has br compression + verify_content_encoding(server, bucket, f"{pkg_hash}.narinfo", "br") # Verify .ls has gzip compression verify_content_encoding(server, bucket, f"{pkg_hash}.ls", "gzip") @@ -673,7 +673,7 @@ in ).strip() chunk_size = 5 * 1024 * 1024 - expected_parts = 3 # 10 MB raw becomes ~10.5 MB compressed (NAR + xz overhead) + expected_parts = 3 # 10 MB raw becomes ~10.5 MB compressed (NAR + br overhead) store_url = make_s3_url( bucket, @@ -755,7 +755,7 @@ in "multipart-upload": "true", "multipart-threshold": str(5 * 1024 * 1024), "multipart-chunk-size": str(5 * 1024 * 1024), - "log-compression": "xz", + "log-compression": "br", } )