Uses std::optional<CompressionAlgo> enum in NarInfo by Hrushi20 · Pull Request #15238 · NixOS/nix

Hrushi20 · 2026-02-14T19:58:54Z

Replaces std::string in NarInfo to use std::optional

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Hrushi20 · 2026-02-14T20:00:52Z

src/libutil/include/nix/util/compression-algo.hh

 namespace nix {

+// Do we want to add Identity to the list???
 #define NIX_FOR_EACH_COMPRESSION_ALGO(MACRO) \


Do we want to add Identity as part of CompressionAlgo enum?

Content-Encoding might contain identity as part of http header.

Thing is that Content-Encoding should be parsed by a very separate function. It's specified by HTTP spec (or rather refers to another RFC), is case-insensitive and has a much more limited range of supported values + some legacy ones like x-gzip. I was going to rewrite this code to better handle Content-Encodinf in a more compliant way. Better to not use the parsing logic here for now. I suppose I can put up a PR to address that soon and we can rebase your one on top of that?

Or could you hold off on replacing strings with enums in the filetransfer code for now?

Would this be a major change or is it something I can look into and implement? If you guide me on any resources, I can take a shot at it.

I think it would be pretty small - it would just take some of your changes from this PR for the decompression sink.

Hrushi20 · 2026-02-14T20:02:36Z

src/libutil-tests/compression.cc

    // Empty-method decompression used e.g. by S3 store
    // (Content-Encoding == "").
-    auto method = "";
+    auto method = CompressionAlgo::none; // Do we handle this in S3 store???


What about cases where we have Content-Encoding as Empty string? Do we want to map it to CompressionAlgo::none?

xokdvium · 2026-02-14T20:22:28Z

src/libstore/nar-info.cc

            url = value;
        else if (name == "Compression")
-            compression = value;
+            compression = parseCompressionAlgo(value);


Hm, this is a behaviour change technically. What if narinfo had an empty string as the value. I find it unlikely since to_string has an assertion, but maybe we can stick with this sneaky behaviour for now (empty string -> std::nullopt -> bzip2 by default)? I lack the historical context to judge whether this would have a potential to break stuff or not.

xokdvium · 2026-02-14T20:23:49Z

src/libstore/nar-info.cc


    if (auto * compression = get(obj, "compression"))
-        res.compression = getString(*compression);
+        res.compression = parseCompressionAlgo(getString(*compression));


ditto, I guess the no-op change would be to treat empty strings as std::nullopt here? It's highly dubious behaviour though

xokdvium · 2026-02-14T20:24:53Z

src/libutil/tarfile.cc

    auto * ar = archive_write_new();
    auto cleanup = Finally{[&ar]() { checkLibArchive(ar, archive_write_close(ar), "failed to close archive: %s"); }};
-    auto err = archive_write_add_filter_by_name(ar, method.c_str());
+    auto err = archive_write_add_filter_by_name(
+        ar, showCompressionAlgo(method.value()).c_str()); // method.value_or(CompressionAlgo::none)
    checkLibArchive(ar, err, "failed to get libarchive filter by name: %s");
    auto code = archive_filter_code(ar, 0);
    return code;


Hm, not this whole function should be possible to delete. It was more of a hack anyway.

By this I mean that we can replace this with direct calls to archive_read_support_filter_ calls. And get rid of the stringly typed code completely.

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Hrushi20 · 2026-02-15T20:37:54Z

src/libstore/filetransfer.cc


                    else if (name == "content-encoding")
-                        encoding = trim(line.substr(i + 1));
+                        encoding = parseCompressionAlgo(trim(line.substr(i + 1)));


@xokdvium, I'm planning to create a function here that parses the content-encoding and returns the CompressionAlgo enum.

Is there an exhaustive list of all the content-encoding? I only see 3 tokens being used in the RFC spec

It's all quite messy, and we are certainly not spec compliant here – we'd want to tighten up our handling here. Good news is: there are not so many non-standard encodings that we'd want to continue supporting.

Basically (all case insensitive parsing): gzip, x-gzip, compress, x-compress (those are in the spec), deflate (doesn't seem like we can support this via libarchive), br, zstd. As for non-standard ones, we can probably stick with supporting bzip2, since it seemingly did appear in practice. xz and everything should probably error out (but we do for whatever reason send it in Accept-Encoding - that was clearly a mistake). If we want to be extra cautious, we can probably keep accepting xz and issue a warning.

Everything else also mostly doesnt work, because we currently error out if libarchive tries to decompress/compress something that hasnt been linked with appropriate dependencies (it tries to fall back to an external program, issues a warning and we treat it as an error).

Stacked encodings when multiple filters are applied (like Content-Encoding: gzip, deflate) should also error out, but can think about supporting those in the future. That will require having multiple chained decompression sinks.

Also identity. none and empty strings should be rejected - those are the compression algo encodings used by the NAR compression algorithms.

Makes sense. I’ll open another PR to parse Content-Encoding (case-insensitive) and support:
gzip, x-gzip, compress, x-compress, br, zstd, bzip2, and identity.

I’ll error out on unknown encodings and reject stacked encodings for now.

That you so much! This has been on my radar for a while! Thanks for tackling it ❤️

xokdvium · 2026-02-15T21:03:41Z

src/libstore/nar-info-disk-cache.cc

                    cache.storeDir, StorePath(hashPart + "-" + namePart), Hash::parseAnyPrefixed(queryNAR.getStr(6)));
                narInfo->url = queryNAR.getStr(2);
-                narInfo->compression = queryNAR.getStr(3);
+                narInfo->compression = parseCompressionAlgo(queryNAR.getStr(3));


Out of caution, shouldn't we continue treating empty strings as none. I'm pretty sure the current version of the sqlite db could never have empty strings there, but can't be too careful with these things.

Maybe drop a FIXME so that we could remove that later?

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

use std::optional<CompressionAlgo> in NarInfo

e083b76

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Hrushi20 requested review from Ericson2314 and edolstra as code owners February 14, 2026 19:58

github-actions bot added the store Issues and pull requests concerning the Nix store label Feb 14, 2026

Hrushi20 commented Feb 14, 2026

View reviewed changes

xokdvium enabled auto-merge February 14, 2026 20:12

xokdvium disabled auto-merge February 14, 2026 20:12

xokdvium reviewed Feb 14, 2026

View reviewed changes

Hrushi20 added 2 commits February 15, 2026 13:44

Handle parsing empty string in json

f97f504

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

use ternary operator

93b1832

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Hrushi20 commented Feb 15, 2026

View reviewed changes

xokdvium reviewed Feb 15, 2026

View reviewed changes

Hrushi20 mentioned this pull request Feb 15, 2026

Refactor Parsing Content Encoding Http Headers #15245

Open

clang formatter

bf9bdee

Signed-off-by: Hrushi20 <hrushikesh20thegreat@gmail.com>

Uh oh!

Conversation

Hrushi20 commented Feb 14, 2026

Uh oh!

Hrushi20 Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hrushi20 Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xokdvium Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Hrushi20 Feb 14, 2026 •

edited

Loading

Hrushi20 Feb 14, 2026 •

edited

Loading

xokdvium Feb 15, 2026 •

edited

Loading