fetchurl: Allow passing curl options with spaces

[infinisil]

Description of changes

It previously was impossible to pass arguments with spaces with curlOpts. This commit makes it possible with

curlOptsList = [ "-H" "Some: Header" ];

This is fully backwards compatible, because the behavior of curlOpts is maintained, though it now throws a warning if a list is passed to it.

Fixes #41820 and #90034

Things done

Tested with

let pkgs = import ./. {};
in pkgs.fetchurl {
  url = "https://httpbin.org/headers";
  sha256 = "0000000000000000000000000000000000000000000000000000";
  curlOptsList = [
    "-s"
    "-H"
    ''Hello: Test '" <- these are some quotes''
  ];
  postFetch = "${pkgs.jq}/bin/jq -r '.headers.Hello' $out";
}

which sets the Hello header using -H, and curls some API that returns the sent HTTP headers as a JSON response, then prints the Hello header that was sent

this derivation will be built:
  /nix/store/0s6z6jq9riri601gjgkw1rawm2d1z8ci-headers.drv
Resolved derivation: '/nix/store/0s6z6jq9riri601gjgkw1rawm2d1z8ci-headers.drv' -> '/nix/store/hzyjjn9y9m93cq887kmryvcr6h820349-headers.drv'...
building '/nix/store/hzyjjn9y9m93cq887kmryvcr6h820349-headers.drv'...

trying https://httpbin.org/headers
Test '" <- these are some quotes
error: hash mismatch in fixed-output derivation '/nix/store/hzyjjn9y9m93cq887kmryvcr6h820349-headers.drv':
         specified: sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
            got:    sha256-Ycaj2hgkcF36d9oI3gPTPqOubJjJkytN1p78cyaIVFM=
error: build of '/nix/store/0s6z6jq9riri601gjgkw1rawm2d1z8ci-headers.drv' failed

If you were to use curlOpts for this instead, this would be the error and behavior:

trace: warning: fetchurl for https://httpbin.org/headers: curlOpts is a list ([ "-s" "-H" "Hello: Test '\" <- these are some quotes" ]), which is not supported anymore.
- If you wish to get the same effect as before, for elements with spaces (even if escaped) to expand to multiple curl arguments, use a string argument instead:
  curlOpts = "-s -H Hello: Test '\" <- these are some quotes";
- If you wish for each list element to be passed as a separate curl argument, allowing arguments to contain spaces, use curlOptsList instead:
  curlOptsList = [ "-s" "-H" "Hello: Test '\" <- these are some quotes" ];
this derivation will be built:
  /nix/store/y491zn21n61hhpykkc2lqcgkk8vvalh8-headers.drv
Resolved derivation: '/nix/store/y491zn21n61hhpykkc2lqcgkk8vvalh8-headers.drv' -> '/nix/store/sdxfnfsgy8lggfy515qw2yg326r8g6a9-headers.drv'...
building '/nix/store/sdxfnfsgy8lggfy515qw2yg326r8g6a9-headers.drv'...
error checking the existence of https://tarballs.nixos.org/sha256/0000000000000000000000000000000000000000000000000000:
curl: (6) Could not resolve host: Test
curl: (6) Could not resolve host: '"
curl: (6) Could not resolve host: <-
curl: (6) Could not resolve host: these
curl: (6) Could not resolve host: are
curl: (6) Could not resolve host: some
curl: (6) Could not resolve host: quotes
curl: (22) The requested URL returned error: 404

[pennae]

this looks correct. currently there seems to be only one derivation using a curlOpts list (factorio), and that shouldn't see changed behaviour. a number of others seems to use quotes incorrectly, like xlockmore:

nixpkgs/pkgs/misc/screensavers/xlockmore/default.nix

Line 11 in 6e3ee89

curlOpts = "--user-agent 'Mozilla/5.0'";

this kind of misuse seems to be confined to user agents though.

[infinisil]

Turns out it wasn't entirely backwards compatible before. But now it is, but I had to introduce a separate curlOptsList argument. Now curlOpts expects a string and curlOptsList expects a list. If a list is passed to curlOpts, it falls back to the old behavior, but gives this deprecation warning:

trace: warning: fetchurl for http_browser_request_headers.php: curlOpts is a list
  ([ "-s" "-H" "Hello: Header" ]), which is not supported anymore.
- If you wish to get the same effect as previously, for elements with spaces (even
  if escaped) to expand to multiple curl arguments, use a string argument instead:
  curlOpts = "-s -H Hello: Header";
- If you wish for the arguments to be passed to curl directly, without involving bash,
  use curlOptsList instead:
  curlOptsList = [ "-s" "-H" "Hello: Header" ];

[pennae]

factorio no longer evaluates cleanly because it's the only derivation in nixpkgs that passes a list. wondering whether that's enough to infer that lists are also rare outside of nixpkgs and keep the original version, the interface in that one was somewhat nicer than having two parameters for the same function but different semantics for the space character of all things 😕

[infinisil]

@pennae It's only a warning, but I fixed it now. I don't think it's a problem since it's not an error. Regarding having two parameters, I'm thinking that we could reunify these in the future, once nobody uses the list semantics of curlOpts anymore.

[infinisil]

I did some minor improvements to the error message, it now looks like

trace: warning: fetchurl for https://httpbin.org/headers: curlOpts is a list ([ "-s" "-H" "Hello: Test '\" <- these are some quotes" ]), which is not supported anymore.
- If you wish to get the same effect as before, for elements with spaces (even if escaped) to expand to multiple curl arguments, use a string argument instead:
  curlOpts = "-s -H Hello: Test '\" <- these are some quotes";
- If you wish for each list element to be passed as a separate curl argument, allowing arguments to contain spaces, use curlOptsList instead:
  curlOptsList = [ "-s" "-H" "Hello: Test '\" <- these are some quotes" ];

I'll merge this PR soon if there are no complaints :)

[roberth]

Could you add a test using testers.invalidateFetcherByDrvHash?
The entrypoint for such tests is in pkgs/test/default.nix.

[thiagokokada]

Is there anything blocking this PR 🤔 ?

[roberth]

Is there anything blocking this PR thinking ?

It does not have any tests. See earlier comment.

[infinisil]

@roberth @thiagokokada I added a test now for the new functionality