diff --git a/google/cloud/internal/curl_impl.cc b/google/cloud/internal/curl_impl.cc index 107210aa0c25c..b1db29c57ad9b 100644 --- a/google/cloud/internal/curl_impl.cc +++ b/google/cloud/internal/curl_impl.cc @@ -487,6 +487,22 @@ std::size_t CurlImpl::HeaderCallback(absl::Span response) { response.size()); } +class CurlImpl::ReadFunctionAbortGuard { + public: + explicit ReadFunctionAbortGuard(CurlImpl& impl) : impl_(impl) {} + ~ReadFunctionAbortGuard() { + // If curl_closed_ is true, then the handle has already been recycled and + // attempting to set an option on it will error. + if (!impl_.curl_closed_) { + impl_.handle_.SetOptionUnchecked(CURLOPT_READFUNCTION, + &ReadFunctionAbort); + } + } + + private: + CurlImpl& impl_; +}; + Status CurlImpl::MakeRequestImpl(RestContext& context) { TRACE_STATE() << ", url_=" << url_; @@ -509,6 +525,11 @@ Status CurlImpl::MakeRequestImpl(RestContext& context) { handle_.SetOptionUnchecked(CURLOPT_HTTP_VERSION, VersionToCurlCode(http_version_)); + // All data in the WriteVector should be written after ReadImpl returns unless + // an error, typically a timeout, has occurred. Use ReadFunctionAbortGuard to + // leverage RAII to instruct curl to not attempt to send anymore data on this + // handle regardless if an error or exception is encountered. + ReadFunctionAbortGuard guard(*this); auto error = curl_multi_add_handle(multi_.get(), handle_.handle_.get()); // This indicates that we are using the API incorrectly. The application @@ -524,18 +545,7 @@ Status CurlImpl::MakeRequestImpl(RestContext& context) { // thus make available the status_code and headers. Any response data // should be put into the spill buffer, which makes them available for // subsequent calls to Read() after the headers have been extracted. - auto read_status = ReadImpl(context, {}).status(); - - // All data in the WriteVector should have been written by now, unless an - // error, typically a timeout, has occurred. Instruct curl to not attempt to - // send anymore data on this handle. If curl_closed_ is true, then the handle - // has already been recycled and attempting to set an option on it will error. - if (!curl_closed_) { - status = handle_.SetOption(CURLOPT_READFUNCTION, &ReadFunctionAbort); - if (!status.ok()) return OnTransferError(context, std::move(status)); - } - - return read_status; + return ReadImpl(context, {}).status(); } StatusOr CurlImpl::ReadImpl(RestContext& context, diff --git a/google/cloud/internal/curl_impl.h b/google/cloud/internal/curl_impl.h index 59added888618..ce9c8a18a994b 100644 --- a/google/cloud/internal/curl_impl.h +++ b/google/cloud/internal/curl_impl.h @@ -107,6 +107,7 @@ class CurlImpl { std::size_t HeaderCallback(absl::Span response); private: + class ReadFunctionAbortGuard; Status MakeRequestImpl(RestContext& context); StatusOr ReadImpl(RestContext& context, absl::Span output);