libstore: fix http abuses no longer working

while refactoring the curl wrapper we inadvertently broken the immutable
flake protocol, because the immutable flake protocol accumulates headers
across the entire redirect chain instead of using only the headers given
in the final response of the chain. this is a problem because Some Known
Providers Of Flake Infrastructure set rel=immutable link headers only in
the penultimate entry of the redirect chain, and curl does not regard it
as worth returning to us via its response header enumeration mechanisms.

fixes #358

Change-Id: I645c3932b465cde848bd6a3565925a1e3cbcdda0

src/libstore/filetransfer.cc

@@ -51,7 +51,6 @@ struct curlFileTransfer : public FileTransfer
         std::function<void(TransferItem &, std::string_view data)> dataCallback;
         CURL * req = 0;
         bool active = false; // whether the handle has been added to the multi object
-        bool headersProcessed = false;
         std::string statusMsg;
 
         unsigned int attempt = 0;
@@ -136,35 +135,11 @@ struct curlFileTransfer : public FileTransfer
 
         std::exception_ptr writeException;
 
-        std::optional<std::string> getHeader(const char * name)
-        {
-            curl_header * result;
-            auto e = curl_easy_header(req, name, 0, CURLH_HEADER, -1, &result);
-            if (e == CURLHE_OK) {
-                return result->value;
-            } else if (e == CURLHE_MISSING || e == CURLHE_NOHEADERS) {
-                return std::nullopt;
-            } else {
-                throw nix::Error("unexpected error from curl_easy_header(): %i", e);
-            }
-        }
-
         size_t writeCallback(void * contents, size_t size, size_t nmemb)
         {
             const size_t realSize = size * nmemb;
 
             try {
-                if (!headersProcessed) {
-                    if (auto h = getHeader("content-encoding")) {
-                        encoding = std::move(*h);
-                    }
-                    if (auto h = getHeader("accept-ranges"); h && *h == "bytes") {
-                        acceptRanges = true;
-                    }
-
-                    headersProcessed = true;
-                }
-
                 result.bodySize += realSize;
 
                 if (successfulStatuses.count(getHTTPStatus()) && this->dataCallback) {
@@ -200,7 +175,31 @@ struct curlFileTransfer : public FileTransfer
                 statusMsg = trim(match.str(1));
                 acceptRanges = false;
                 encoding = "";
-                headersProcessed = false;
+            } else {
+                auto i = line.find(':');
+                if (i != std::string::npos) {
+                    std::string name = toLower(trim(line.substr(0, i)));
+
+                    if (name == "etag") {
+                        result.etag = trim(line.substr(i + 1));
+                    }
+
+                    else if (name == "content-encoding")
+                        encoding = trim(line.substr(i + 1));
+
+                    else if (name == "accept-ranges" && toLower(trim(line.substr(i + 1))) == "bytes")
+                        acceptRanges = true;
+
+                    else if (name == "link" || name == "x-amz-meta-link") {
+                        auto value = trim(line.substr(i + 1));
+                        static std::regex linkRegex("<([^>]*)>; rel=\"immutable\"", std::regex::extended | std::regex::icase);
+                        if (std::smatch match; std::regex_match(value, match, linkRegex))
+                            result.immutableUrl = match.str(1);
+                        else
+                            debug("got invalid link header '%s'", value);
+                        warn("foo %s", value);
+                    }
+                }
             }
             return realSize;
         }
@@ -336,25 +335,6 @@ struct curlFileTransfer : public FileTransfer
             debug("finished %s of '%s'; curl status = %d, HTTP status = %d, body = %d bytes",
                 request.verb(), request.uri, code, httpStatus, result.bodySize);
 
-            auto link = getHeader("link");
-            if (!link) {
-                link = getHeader("x-amz-meta-link");
-            }
-            if (link) {
-                static std::regex linkRegex(
-                    "<([^>]*)>; rel=\"immutable\"", std::regex::extended | std::regex::icase
-                );
-                if (std::smatch match; std::regex_match(*link, match, linkRegex)) {
-                    result.immutableUrl = match.str(1);
-                } else {
-                    debug("got invalid link header '%s'", *link);
-                }
-            }
-
-            if (auto etag = getHeader("etag")) {
-                result.etag = std::move(*etag);
-            }
-
             // 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

tests/unit/libstore/filetransfer.cc

@@ -23,10 +23,19 @@
 
 using namespace std::chrono_literals;
 
+namespace {
+
+struct Reply {
+    std::string status, headers;
+    std::function<std::string()> content;
+};
+
+}
+
 namespace nix {
 
 static std::tuple<uint16_t, AutoCloseFD>
-serveHTTP(std::string_view status, std::string_view headers, std::function<std::string()> content)
+serveHTTP(std::vector<Reply> replies)
 {
     AutoCloseFD listener(::socket(AF_INET6, SOCK_STREAM, 0));
     if (!listener) {
@@ -52,7 +61,7 @@ serveHTTP(std::string_view status, std::string_view headers, std::function<std::
     }
 
     std::thread(
-        [status, headers, content](AutoCloseFD socket, AutoCloseFD trigger) {
+        [replies, at{0}](AutoCloseFD socket, AutoCloseFD trigger) mutable {
             while (true) {
                 pollfd pfds[2] = {
                     {
@@ -90,12 +99,14 @@ serveHTTP(std::string_view status, std::string_view headers, std::function<std::
                     }
                 };
 
+                const auto & reply = replies[at++ % replies.size()];
+
                 send("HTTP/1.1 ");
-                send(status);
+                send(reply.status);
                 send("\r\n");
-                send(headers);
+                send(reply.headers);
                 send("\r\n");
-                send(content());
+                send(reply.content());
                 ::shutdown(conn.get(), SHUT_RDWR);
             }
         },
@@ -110,6 +121,12 @@ serveHTTP(std::string_view status, std::string_view headers, std::function<std::
     };
 }
 
+static std::tuple<uint16_t, AutoCloseFD>
+serveHTTP(std::string status, std::string headers, std::function<std::string()> content)
+{
+    return serveHTTP({{{status, headers, content}}});
+}
+
 TEST(FileTransfer, exceptionAbortsDownload)
 {
     struct Done
@@ -166,4 +183,30 @@ TEST(FileTransfer, NOT_ON_DARWIN(handlesContentEncoding))
     ft->download(FileTransferRequest(fmt("http://[::1]:%d/index", port)), sink);
     EXPECT_EQ(sink.s, original);
 }
+
+TEST(FileTransfer, usesIntermediateLinkHeaders)
+{
+    auto [port, srv] = serveHTTP({
+        {"301 ok",
+         "location: /second\r\n"
+         "content-length: 0\r\n",
+         [] { return ""; }},
+        {"307 ok",
+         "location: /third\r\n"
+         "content-length: 0\r\n",
+         [] { return ""; }},
+        {"307 ok",
+         "location: /fourth\r\n"
+         "link: <http://foo>; rel=\"immutable\"\r\n"
+         "content-length: 0\r\n",
+         [] { return ""; }},
+        {"200 ok", "content-length: 1\r\n", [] { return "a"; }},
+    });
+    auto ft = makeFileTransfer();
+    FileTransferRequest req(fmt("http://[::1]:%d/first", port));
+    req.baseRetryTimeMs = 0;
+    auto result = ft->download(req);
+    ASSERT_EQ(result.immutableUrl, "http://foo");
+}
+
 }