diff options
| -rw-r--r-- | src/libstore/filetransfer.cc | 70 | ||||
| -rw-r--r-- | tests/unit/libstore/filetransfer.cc | 53 |
2 files changed, 73 insertions, 50 deletions
diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 492463a61..076a5ca56 100644 --- a/src/libstore/filetransfer.cc +++ b/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 diff --git a/tests/unit/libstore/filetransfer.cc b/tests/unit/libstore/filetransfer.cc index 684697c69..0e5c0965e 100644 --- a/tests/unit/libstore/filetransfer.cc +++ b/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"); +} + } |