diff --git a/llvm/include/llvm/Debuginfod/HTTPClient.h b/llvm/include/llvm/Debuginfod/HTTPClient.h --- a/llvm/include/llvm/Debuginfod/HTTPClient.h +++ b/llvm/include/llvm/Debuginfod/HTTPClient.h @@ -7,9 +7,8 @@ //===----------------------------------------------------------------------===// /// /// \file -/// This file contains the declarations of the HTTPClient, HTTPMethod, -/// HTTPResponseHandler, and BufferedHTTPResponseHandler classes, as well as -/// the HTTPResponseBuffer and HTTPRequest structs. +/// This file contains the declarations of the HTTPClient library for issuing +/// HTTP requests and handling the responses. /// //===----------------------------------------------------------------------===// @@ -40,43 +39,13 @@ /// of its methods. class HTTPResponseHandler { public: - /// Processes one line of HTTP response headers. - virtual Error handleHeaderLine(StringRef HeaderLine) = 0; - /// Processes an additional chunk of bytes of the HTTP response body. virtual Error handleBodyChunk(StringRef BodyChunk) = 0; - /// Processes the HTTP response status code. - virtual Error handleStatusCode(unsigned Code) = 0; - protected: ~HTTPResponseHandler(); }; -/// An HTTP response status code bundled with a buffer to store the body. -struct HTTPResponseBuffer { - unsigned Code = 0; - std::unique_ptr Body; -}; - -/// A simple handler which writes returned data to an HTTPResponseBuffer. -/// Ignores all headers except the Content-Length, which it uses to -/// allocate an appropriately-sized Body buffer. -class BufferedHTTPResponseHandler final : public HTTPResponseHandler { - size_t Offset = 0; - -public: - /// Stores the data received from the HTTP server. - HTTPResponseBuffer ResponseBuffer; - - /// These callbacks store the body and status code in an HTTPResponseBuffer - /// allocated based on Content-Length. The Content-Length header must be - /// handled by handleHeaderLine before any calls to handleBodyChunk. - Error handleHeaderLine(StringRef HeaderLine) override; - Error handleBodyChunk(StringRef BodyChunk) override; - Error handleStatusCode(unsigned Code) override; -}; - /// A reusable client that can perform HTTPRequests through a network socket. class HTTPClient { #ifdef LLVM_ENABLE_CURL @@ -107,13 +76,8 @@ /// Handler method. Error perform(const HTTPRequest &Request, HTTPResponseHandler &Handler); - /// Performs the Request with the default BufferedHTTPResponseHandler, and - /// returns its HTTPResponseBuffer or an Error. - Expected perform(const HTTPRequest &Request); - - /// Performs an HTTPRequest with the default configuration to make a GET - /// request to the given Url. Returns an HTTPResponseBuffer or an Error. - Expected get(StringRef Url); + /// Returns the last received response code or zero if none. + unsigned responseCode(); }; } // end namespace llvm diff --git a/llvm/lib/Debuginfod/Debuginfod.cpp b/llvm/lib/Debuginfod/Debuginfod.cpp --- a/llvm/lib/Debuginfod/Debuginfod.cpp +++ b/llvm/lib/Debuginfod/Debuginfod.cpp @@ -115,6 +115,40 @@ getDefaultDebuginfodTimeout()); } +namespace { + +/// A simple handler which streams the returned data to a cache file. The cache +/// file is only created if a 200 OK status is observed. +class StreamedHTTPResponseHandler : public HTTPResponseHandler { + using CreateStreamFn = + std::function>()>; + CreateStreamFn CreateStream; + HTTPClient &Client; + std::unique_ptr FileStream; + +public: + StreamedHTTPResponseHandler(CreateStreamFn CreateStream, HTTPClient &Client) + : CreateStream(CreateStream), Client(Client) {} + + Error handleBodyChunk(StringRef BodyChunk) override; +}; + +} // namespace + +Error StreamedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) { + if (!FileStream) { + if (Client.responseCode() != 200) + return Error::success(); + Expected> FileStreamOrError = + CreateStream(); + if (!FileStreamOrError) + return FileStreamOrError.takeError(); + FileStream = std::move(*FileStreamOrError); + } + *FileStream->OS << BodyChunk; + return Error::success(); +} + Expected getCachedOrDownloadArtifact( StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath, ArrayRef DebuginfodUrls, std::chrono::milliseconds Timeout) { @@ -155,28 +189,18 @@ SmallString<64> ArtifactUrl; sys::path::append(ArtifactUrl, sys::path::Style::posix, ServerUrl, UrlPath); - Expected ResponseOrErr = Client.get(ArtifactUrl); - if (!ResponseOrErr) - return ResponseOrErr.takeError(); + // Perform the HTTP request and if successful, write the response body to + // the cache. + StreamedHTTPResponseHandler Handler([&]() { return CacheAddStream(Task); }, + Client); + HTTPRequest Request(ArtifactUrl); + Error Err = Client.perform(Request, Handler); + if (Err) + return Err; - HTTPResponseBuffer &Response = *ResponseOrErr; - if (Response.Code != 200) + if (Client.responseCode() != 200) continue; - // We have retrieved the artifact from this server, and now add it to the - // file cache. - Expected> FileStreamOrErr = - CacheAddStream(Task); - if (!FileStreamOrErr) - return FileStreamOrErr.takeError(); - std::unique_ptr &FileStream = *FileStreamOrErr; - if (!Response.Body) - return createStringError( - errc::io_error, "Unallocated MemoryBuffer in HTTPResponseBuffer."); - - *FileStream->OS << StringRef(Response.Body->getBufferStart(), - Response.Body->getBufferSize()); - // Return the path to the artifact on disk. return std::string(AbsCachedArtifactPath); } diff --git a/llvm/lib/Debuginfod/HTTPClient.cpp b/llvm/lib/Debuginfod/HTTPClient.cpp --- a/llvm/lib/Debuginfod/HTTPClient.cpp +++ b/llvm/lib/Debuginfod/HTTPClient.cpp @@ -7,9 +7,8 @@ //===----------------------------------------------------------------------===// /// /// \file -/// -/// This file defines the methods of the HTTPRequest, HTTPClient, and -/// BufferedHTTPResponseHandler classes. +/// This file defines the implementation of the HTTPClient library for issuing +/// HTTP requests and handling the responses. /// //===----------------------------------------------------------------------===// @@ -34,44 +33,6 @@ HTTPResponseHandler::~HTTPResponseHandler() = default; -static inline bool parseContentLengthHeader(StringRef LineRef, - size_t &ContentLength) { - // Content-Length is a mandatory header, and the only one we handle. - return LineRef.consume_front("Content-Length: ") && - to_integer(LineRef.trim(), ContentLength, 10); -} - -Error BufferedHTTPResponseHandler::handleHeaderLine(StringRef HeaderLine) { - if (ResponseBuffer.Body) - return Error::success(); - - size_t ContentLength; - if (parseContentLengthHeader(HeaderLine, ContentLength)) - ResponseBuffer.Body = - WritableMemoryBuffer::getNewUninitMemBuffer(ContentLength); - - return Error::success(); -} - -Error BufferedHTTPResponseHandler::handleBodyChunk(StringRef BodyChunk) { - if (!ResponseBuffer.Body) - return createStringError(errc::io_error, - "Unallocated response buffer. HTTP Body data " - "received before Content-Length header."); - if (Offset + BodyChunk.size() > ResponseBuffer.Body->getBufferSize()) - return createStringError(errc::io_error, - "Content size exceeds buffer size."); - memcpy(ResponseBuffer.Body->getBufferStart() + Offset, BodyChunk.data(), - BodyChunk.size()); - Offset += BodyChunk.size(); - return Error::success(); -} - -Error BufferedHTTPResponseHandler::handleStatusCode(unsigned Code) { - ResponseBuffer.Code = Code; - return Error::success(); -} - bool HTTPClient::IsInitialized = false; class HTTPClientCleanup { @@ -80,18 +41,6 @@ }; static const HTTPClientCleanup Cleanup; -Expected HTTPClient::perform(const HTTPRequest &Request) { - BufferedHTTPResponseHandler Handler; - if (Error Err = perform(Request, Handler)) - return std::move(Err); - return std::move(Handler.ResponseBuffer); -} - -Expected HTTPClient::get(StringRef Url) { - HTTPRequest Request(Url); - return perform(Request); -} - #ifdef LLVM_ENABLE_CURL bool HTTPClient::isAvailable() { return true; } @@ -128,18 +77,6 @@ llvm::Error ErrorState = Error::success(); }; -static size_t curlHeaderFunction(char *Contents, size_t Size, size_t NMemb, - CurlHTTPRequest *CurlRequest) { - assert(Size == 1 && "The Size passed by libCURL to CURLOPT_HEADERFUNCTION " - "should always be 1."); - if (Error Err = - CurlRequest->Handler.handleHeaderLine(StringRef(Contents, NMemb))) { - CurlRequest->storeError(std::move(Err)); - return 0; - } - return NMemb; -} - static size_t curlWriteFunction(char *Contents, size_t Size, size_t NMemb, CurlHTTPRequest *CurlRequest) { Size *= NMemb; @@ -160,7 +97,6 @@ assert(Curl && "Curl could not be initialized"); // Set the callback hooks. curl_easy_setopt(Curl, CURLOPT_WRITEFUNCTION, curlWriteFunction); - curl_easy_setopt(Curl, CURLOPT_HEADERFUNCTION, curlHeaderFunction); } HTTPClient::~HTTPClient() { curl_easy_cleanup(Curl); } @@ -177,22 +113,19 @@ CurlHTTPRequest CurlRequest(Handler); curl_easy_setopt(Curl, CURLOPT_WRITEDATA, &CurlRequest); - curl_easy_setopt(Curl, CURLOPT_HEADERDATA, &CurlRequest); CURLcode CurlRes = curl_easy_perform(Curl); if (CurlRes != CURLE_OK) return joinErrors(std::move(CurlRequest.ErrorState), createStringError(errc::io_error, "curl_easy_perform() failed: %s\n", curl_easy_strerror(CurlRes))); - if (CurlRequest.ErrorState) - return std::move(CurlRequest.ErrorState); + return std::move(CurlRequest.ErrorState); +} - unsigned Code; +unsigned HTTPClient::responseCode() { + long Code = 0; curl_easy_getinfo(Curl, CURLINFO_RESPONSE_CODE, &Code); - if (Error Err = Handler.handleStatusCode(Code)) - return joinErrors(std::move(CurlRequest.ErrorState), std::move(Err)); - - return std::move(CurlRequest.ErrorState); + return Code; } #else @@ -214,4 +147,8 @@ llvm_unreachable("No HTTP Client implementation available."); } +unsigned HTTPClient::responseCode() { + llvm_unreachable("No HTTP Client implementation available."); +} + #endif diff --git a/llvm/unittests/Debuginfod/CMakeLists.txt b/llvm/unittests/Debuginfod/CMakeLists.txt --- a/llvm/unittests/Debuginfod/CMakeLists.txt +++ b/llvm/unittests/Debuginfod/CMakeLists.txt @@ -1,5 +1,4 @@ add_llvm_unittest(DebuginfodTests - HTTPClientTests.cpp DebuginfodTests.cpp ) diff --git a/llvm/unittests/Debuginfod/HTTPClientTests.cpp b/llvm/unittests/Debuginfod/HTTPClientTests.cpp deleted file mode 100644 --- a/llvm/unittests/Debuginfod/HTTPClientTests.cpp +++ /dev/null @@ -1,94 +0,0 @@ -//===-- llvm/unittest/Debuginfod/HTTPClientTests.cpp - unit tests ---------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "llvm/Debuginfod/HTTPClient.h" -#include "llvm/Support/Errc.h" -#include "llvm/Testing/Support/Error.h" -#include "gtest/gtest.h" - -using namespace llvm; - -TEST(BufferedHTTPResponseHandler, Lifecycle) { - BufferedHTTPResponseHandler Handler; - EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: 36\r\n"), - Succeeded()); - - EXPECT_THAT_ERROR(Handler.handleBodyChunk("body:"), Succeeded()); - EXPECT_THAT_ERROR(Handler.handleBodyChunk("this puts the total at 36 chars"), - Succeeded()); - EXPECT_EQ(Handler.ResponseBuffer.Body->MemoryBuffer::getBuffer(), - "body:this puts the total at 36 chars"); - - // Additional content should be rejected by the handler. - EXPECT_THAT_ERROR( - Handler.handleBodyChunk("extra content past the content-length"), - Failed()); - - // Test response code is set. - EXPECT_THAT_ERROR(Handler.handleStatusCode(200u), Succeeded()); - EXPECT_EQ(Handler.ResponseBuffer.Code, 200u); - EXPECT_THAT_ERROR(Handler.handleStatusCode(400u), Succeeded()); - EXPECT_EQ(Handler.ResponseBuffer.Code, 400u); -} - -TEST(BufferedHTTPResponseHandler, NoContentLengthLifecycle) { - BufferedHTTPResponseHandler Handler; - EXPECT_EQ(Handler.ResponseBuffer.Code, 0u); - EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr); - - // A body chunk passed before the content-length header is an error. - EXPECT_THAT_ERROR(Handler.handleBodyChunk("body"), - Failed()); - EXPECT_THAT_ERROR(Handler.handleHeaderLine("a header line"), Succeeded()); - EXPECT_THAT_ERROR(Handler.handleBodyChunk("body"), - Failed()); -} - -TEST(BufferedHTTPResponseHandler, ZeroContentLength) { - BufferedHTTPResponseHandler Handler; - EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: 0"), Succeeded()); - EXPECT_NE(Handler.ResponseBuffer.Body, nullptr); - EXPECT_EQ(Handler.ResponseBuffer.Body->getBufferSize(), 0u); - - // All content should be rejected by the handler. - EXPECT_THAT_ERROR(Handler.handleBodyChunk("non-empty body content"), - Failed()); -} - -TEST(BufferedHTTPResponseHandler, MalformedContentLength) { - // Check that several invalid content lengths are ignored. - BufferedHTTPResponseHandler Handler; - EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr); - EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: fff"), - Succeeded()); - EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr); - - EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: "), - Succeeded()); - EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr); - - using namespace std::string_literals; - EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: \0\0\0"s), - Succeeded()); - EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr); - - EXPECT_THAT_ERROR(Handler.handleHeaderLine("Content-Length: -11"), - Succeeded()); - EXPECT_EQ(Handler.ResponseBuffer.Body, nullptr); - - // All content should be rejected by the handler because no valid - // Content-Length header has been received. - EXPECT_THAT_ERROR(Handler.handleBodyChunk("non-empty body content"), - Failed()); -} - -#ifdef LLVM_ENABLE_CURL - -TEST(HTTPClient, isAvailable) { EXPECT_TRUE(HTTPClient::isAvailable()); } - -#endif