[C-API] Http transport doesn't correctly preserve the request body

[nirinchev]

When strings are small, they will be allocated on the stack rather than the heap, which means that a pointer to their .data() will become invalid when the string is moved. This seems to be what's happening after #5743 started preserving the request in the completion callback.

realm-core/src/realm/object-store/c_api/http.cpp

Lines 74 to 81 in 52b297a

	realm_http_request_t c_request{realm_http_request_method_e(request.method),
	request.url.c_str(),
	request.timeout_ms,
	c_headers.data(),
	c_headers.size(),
	request.body.data(),
	request.body.size()};
	auto completion_data = std::make_unique<HttpCompletionData>(std::move(request), std::move(completion_block));

Here, we construct a realm_http_request_t where the body points to request.body.data() but then we std::move the request to construct completion_data. When the request body is small, e.g. { "name": "a" }, the string will be copied and the old data will now be garbage. I imagine the same issue may affect the url, but it seems our urls are long enough for it not to be a problem.

[nirinchev]

cc @michael-wb - I've been chasing this for a while in dart and this is the best explanation I could come up with. My understanding of C++ is rather superficial though, so there may be something else at play.

[michael-wb]

@nirinchev - I am reworking this area due to requests from @RedBeard0531, so I will look into this issue as part of that.

[RedBeard0531]

@nirinchev's analysis is correct. You need to either copy the request into the CompletionData, or create the CompletionData first, and have the c request refer into its request members.

[sync-by-unito]

➤ Michael Wilkerson-Barker commented:

Updated send_request_to_server has been fixed in PR 5898
A new c_api test for the network transport has also been added.

[sync-by-unito]

➤ Michael Wilkerson-Barker commented:

Updated mechanism for the http C_API and added a test to verify operation