bakaid commented on a change in pull request #610: MINIFICPP-814 - Fixed
ListenHTTP and HTTPClient bugs, created tests f…
URL: https://github.com/apache/nifi-minifi-cpp/pull/610#discussion_r305281550
##########
File path: extensions/http-curl/client/HTTPClient.cpp
##########
@@ -196,7 +255,7 @@ std::string HTTPClient::escape(std::string
string_to_escape) {
void HTTPClient::setPostFields(std::string input) {
curl_easy_setopt(http_session_, CURLOPT_POSTFIELDSIZE, input.length());
- curl_easy_setopt(http_session_, CURLOPT_POSTFIELDS, input.c_str());
+ curl_easy_setopt(http_session_, CURLOPT_COPYPOSTFIELDS, input.c_str());
Review comment:
@phrocker Actually we can't assume that std::strings are COW (indeed they
explicitly can't be anymore since C++11). This means that if SSO is not in
effect, then the heap containing the string will be copied and then freed when
the temporary copy of the string setPostFields is called with is destructed,
immediately resulting in a heap use-after-free.
In the specific case if the string is short enough, meaning SSO is in
effect, and the calling function calls both setPostFields and then submit,
meaning that even though destructed, the temporary copy will still be on stack,
then we avert a user-after free. If the HTTPClient object is stored beyond
that, which is a usual case, then even this case will result in
stack-use-after-return or stack-buffer-overflow.
On the issue of re-copying on every submission: I don't see evidence of
that. Once we set the CURLOPT_COPYPOSTFIELDS, the easy handle itself will store
that string until we set a new one, or call curl_easy_reset or
curl_easy_cleanup the handle. Multiple curl_easy_performs (so multiple submits)
do no change that. So our internal copy of the post string is already there: it
is in the easy handle.
This function could be optimized if our parameter was const& (we would avoid
one copy): I am not sure if changing that counts as an API change, if not, I
would be happy to do it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
