Skip to content

Commit

Permalink
impl(rest): set ReadFunctionAbort via RAII (#14617)
Browse files Browse the repository at this point in the history
  • Loading branch information
scotthart authored Aug 7, 2024
1 parent 9ccfa6a commit 7eff3dd
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 12 deletions.
34 changes: 22 additions & 12 deletions google/cloud/internal/curl_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,22 @@ std::size_t CurlImpl::HeaderCallback(absl::Span<char> 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_;

Expand All @@ -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
Expand All @@ -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<std::size_t> CurlImpl::ReadImpl(RestContext& context,
Expand Down
1 change: 1 addition & 0 deletions google/cloud/internal/curl_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ class CurlImpl {
std::size_t HeaderCallback(absl::Span<char> response);

private:
class ReadFunctionAbortGuard;
Status MakeRequestImpl(RestContext& context);
StatusOr<std::size_t> ReadImpl(RestContext& context, absl::Span<char> output);

Expand Down

0 comments on commit 7eff3dd

Please sign in to comment.