Skip to content

Conversation

@ottigeda
Copy link
Contributor

DataSink::done was not assigned in write_request.
Therefore it was not possible to call "done" in an content provider.
I expected that should be the way to go, in case of failure to provide the content.
As an example, this could happen because the file to provide has vanished from disk in mean time.
I tried to implement it the same way as in other places, I hope it makes sense to you ?

@ottigeda ottigeda force-pushed the add-missing-done branch from e0e9e58 to 7141adf Compare May 11, 2020 11:50
@yhirose
Copy link
Owner

yhirose commented May 11, 2020

@ottigeda, thank you for your contribution! The code looks almost good to me. Could you handle the following two things? Then, I'll merge it. Thanks!

  1. Change data_sink.is_writable implementation as below
    data_sink.is_writable = [&](void) {
      return strm.is_writable() && written_length >= 0;
    };
  1. Apply the same change to the following section

cpp-httplib/httplib.h

Lines 4138 to 4160 in ba685db

#ifdef CPPHTTPLIB_ZLIB_SUPPORT
if (compress_) {
if (content_provider) {
size_t offset = 0;
DataSink data_sink;
data_sink.write = [&](const char *data, size_t data_len) {
req.body.append(data, data_len);
offset += data_len;
};
data_sink.is_writable = [&](void) { return true; };
while (offset < content_length) {
content_provider(offset, content_length - offset, data_sink);
}
} else {
req.body = body;
}
if (!detail::compress(req.body)) { return nullptr; }
req.headers.emplace("Content-Encoding", "gzip");
} else
#endif

@ottigeda ottigeda force-pushed the add-missing-done branch from c9a90b3 to 28c07a1 Compare May 13, 2020 14:06
@ottigeda
Copy link
Contributor Author

Is it that, what you wanted to have in addition ?

@yhirose yhirose closed this in 2c0613f May 14, 2020
@yhirose
Copy link
Owner

yhirose commented May 14, 2020

@ottigeda, thanks for the update. I have been pondering over this change, and came to remember that done is only used for stopping chunked transfer data stream. This is not for canceling or aborting the streaming process.

So instead of implementing done method in this case. I have rather added 'boolean return value' to ContentProvider type for this purpose. If we want to cancel the streaming process, the functor should return false. It actually corresponds to the behavior of ContentReceiver.

This is a breaking change to the ContentProvider interface. But it's necessary for the API consistency.

Sorry that I couldn't use your changes. But thank you very much for bringing this matter!

@ottigeda ottigeda deleted the add-missing-done branch May 14, 2020 07:46
@ottigeda
Copy link
Contributor Author

No problem, your alternative solution is perfectly fine with me. Thanks a lot!

ExclusiveOrange pushed a commit to ExclusiveOrange/cpp-httplib-exor that referenced this pull request May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants