You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celix.apache.org by pn...@apache.org on 2021/01/21 17:09:45 UTC
[celix] branch hotfix/rsa_response_has_a_max created (now 261adf8)
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a change to branch hotfix/rsa_response_has_a_max
in repository https://gitbox.apache.org/repos/asf/celix.git.
at 261adf8 Adds test and improvements for remote service calls with a large payload and response.
This branch includes the following new commits:
new 261adf8 Adds test and improvements for remote service calls with a large payload and response.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[celix] 01/01: Adds test and improvements for remote service calls
with a large payload and response.
Posted by pn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch hotfix/rsa_response_has_a_max
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 261adf891776ea2c770cfaeae938a2f26e8e35ac
Author: Pepijn Noltes <pe...@gmail.com>
AuthorDate: Thu Jan 21 18:09:31 2021 +0100
Adds test and improvements for remote service calls with a large payload and response.
---
.../src/remote_example_impl.c | 2 +-
.../gtest/src/tst_activator.c | 24 +++++++--
.../src/export_registration_dfi.c | 6 ++-
.../src/import_registration_dfi.c | 2 -
.../src/remote_service_admin_dfi.c | 57 +++++++++++++---------
libs/utils/include/celix_utils.h | 2 +-
6 files changed, 61 insertions(+), 32 deletions(-)
diff --git a/bundles/remote_services/examples/remote_example_service/src/remote_example_impl.c b/bundles/remote_services/examples/remote_example_service/src/remote_example_impl.c
index 1439e11..542f4c6 100644
--- a/bundles/remote_services/examples/remote_example_service/src/remote_example_impl.c
+++ b/bundles/remote_services/examples/remote_example_service/src/remote_example_impl.c
@@ -96,7 +96,7 @@ int remoteExample_setName1(remote_example_impl_t* impl, char *n, char **out) {
free(impl->name);
}
impl->name = n;
- *out = strndup(impl->name, 1024 * 1024);
+ *out = celix_utils_strdup(n);
pthread_mutex_unlock(&impl->mutex);
return 0;
}
diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
index 0f1cbb9..a61dc0f 100644
--- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
+++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
@@ -135,12 +135,30 @@ static bool bndTestRemoteString(void *handle) {
pthread_mutex_lock(&act->mutex);
if (act->remoteExample != NULL) {
//test string Call with taking ownership
- char *tmp = strndup("test1", 1024);
+
+ //test with a large very large string to verify mg_write limits.
+ int testLength = 1024 * 1024 * 5; //5mb
+ char *buf = NULL;
+ size_t bufLen = 0;
+ FILE* stream = open_memstream(&buf, &bufLen);
+ for (int i =0; i < testLength; i++) {
+ fputc('A', stream);
+ }
+ fputc('\0', stream);
+ fclose(stream);
char *result = NULL;
- TIMED_EXPR(act->remoteExample->setName1(act->remoteExample->handle, tmp, &result));
+ TIMED_EXPR(act->remoteExample->setName1(act->remoteExample->handle, buf, &result));
printf("Call setName1 took %f ms\n", diff);
//note setName1 should take ownership of tmp, so no free(tmp) needed.
- ok = strncmp("test1", result, 1024) == 0;
+ ok = strncmp("AAAA", result, 4) == 0;
+ if (ok) {
+ ok = strlen(result) == testLength;
+ if (!ok) {
+ fprintf(stderr, "result length is not currect. expected %i, but len is %li\n", testLength, strlen(result));
+ }
+ } else {
+ fprintf(stderr, "result does not start with AAAA\n");
+ }
free(result);
} else {
fprintf(stderr, "remote example service not available");
diff --git a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c
index ec6cf9e..f63c39a 100644
--- a/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c
+++ b/bundles/remote_services/remote_service_admin_dfi/src/export_registration_dfi.c
@@ -156,6 +156,7 @@ void exportRegistration_waitTillNotUsed(export_registration_t *export) {
celix_status_t exportRegistration_call(export_registration_t *export, char *data, int datalength, celix_properties_t *metadata, char **responseOut, int *responseLength) {
int status = CELIX_SUCCESS;
+ char* response = NULL;
*responseLength = -1;
json_error_t error;
json_t *js_request = json_loads(data, 0, &error);
@@ -166,7 +167,7 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data
if (cont) {
celixThreadMutex_lock(&export->mutex);
if (export->active && export->service != NULL) {
- status = jsonRpc_call(export->intf, export->service, data, responseOut);
+ status = jsonRpc_call(export->intf, export->service, data, &response);
} else if (!export->active) {
status = CELIX_ILLEGAL_STATE;
celix_logHelper_warning(export->helper, "Cannot call an inactive service export");
@@ -178,13 +179,14 @@ celix_status_t exportRegistration_call(export_registration_t *export, char *data
remoteInterceptorHandler_invokePostExportCall(export->interceptorsHandler, export->exportReference.endpoint->properties, sig, metadata);
}
+ *responseOut = response;
//printf("calling for '%s'\n");
if (export->logFile != NULL) {
static int callCount = 0;
char *name = NULL;
dynInterface_getName(export->intf, &name);
- fprintf(export->logFile, "REMOTE CALL %i\n\tservice=%s\n\tservice_id=%s\n\trequest_payload=%s\n\tstatus=%i\n", callCount, name, export->servId, data, status);
+ fprintf(export->logFile, "REMOTE CALL %i\n\tservice=%s\n\tservice_id=%s\n\trequest_payload=%s\n\trequest_response=%s\n\tstatus=%i\n", callCount, name, export->servId, data, response, status);
fflush(export->logFile);
callCount += 1;
}
diff --git a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c
index 64c0d27..55f0e05 100644
--- a/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c
+++ b/bundles/remote_services/remote_service_admin_dfi/src/import_registration_dfi.c
@@ -18,11 +18,9 @@
*/
#include <stdlib.h>
-#include <jansson.h>
#include <json_rpc.h>
#include <assert.h>
#include "version.h"
-#include "json_serializer.h"
#include "dyn_interface.h"
#include "import_registration.h"
#include "import_registration_dfi.h"
diff --git a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c
index beeeb5f..5a64f62 100644
--- a/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c
+++ b/bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c
@@ -27,6 +27,7 @@
#include <string.h>
#include <uuid/uuid.h>
#include <curl/curl.h>
+#include <limits.h>
#include <jansson.h>
#include "json_serializer.h"
@@ -96,14 +97,15 @@ struct remote_service_admin {
pthread_mutex_t curlMutexDns;
};
-struct post {
+struct celix_post_data {
const char *readptr;
size_t size;
size_t read;
};
-struct get {
- char *writeptr;
+struct celix_get_data_reply {
+ FILE* stream;
+ char* buf;
size_t size;
};
@@ -505,7 +507,24 @@ static int remoteServiceAdmin_callback(struct mg_connection *conn) {
if (rc == CELIX_SUCCESS && response != NULL) {
mg_write(conn, data_response_headers, strlen(data_response_headers));
- mg_write(conn, response, strlen(response));
+
+ char *bufLoc = response;
+ size_t bytesLeft = strlen(response);
+ if (bytesLeft > INT_MAX) {
+ //NOTE arcording to civetweb mg_write, there is a limit on mg_write for INT_MAX.
+ RSA_LOG_WARNING(rsa, "nr of bytes to send for a remote call is > INT_MAX, this can lead to issues\n");
+ }
+ while (bytesLeft > 0) {
+ int send = mg_write(conn, response, strlen(response));
+ if (send > 0) {
+ bytesLeft -= send;
+ bufLoc += send;
+ } else {
+ RSA_LOG_ERROR(rsa, "Error sending response: %s", strerror(errno));
+ break;
+ }
+ }
+
free(response);
} else {
mg_write(conn, no_content_response_headers, strlen(no_content_response_headers));
@@ -869,14 +888,15 @@ celix_status_t remoteServiceAdmin_removeImportedService(remote_service_admin_t *
static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply, int* replyStatus) {
remote_service_admin_t * rsa = handle;
- struct post post;
+ struct celix_post_data post;
post.readptr = request;
post.size = strlen(request);
post.read = 0;
- struct get get;
+ struct celix_get_data_reply get;
+ get.buf = NULL;
get.size = 0;
- get.writeptr = NULL;
+ get.stream = open_memstream(&get.buf, &get.size);
const char *serviceUrl = celix_properties_get(endpointDescription->properties, (char*) RSA_DFI_ENDPOINT_URL, NULL);
char url[256];
@@ -935,7 +955,9 @@ static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description
//celix_logHelper_log(rsa->loghelper, CELIX_LOG_LEVEL_DEBUG, "RSA: Performing curl post\n");
res = curl_easy_perform(curl);
- *reply = get.writeptr;
+ fputc('\0', get.stream);
+ fclose(get.stream);
+ *reply = get.buf;
*replyStatus = res;
curl_easy_cleanup(curl);
@@ -946,7 +968,7 @@ static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description
}
static size_t remoteServiceAdmin_readCallback(void *voidBuffer, size_t size, size_t nmemb, void *userp) {
- struct post *post = userp;
+ struct celix_post_data *post = userp;
size_t buffSize = size * nmemb;
size_t readSize = post->size - post->read;
if (readSize > buffSize) {
@@ -959,20 +981,9 @@ static size_t remoteServiceAdmin_readCallback(void *voidBuffer, size_t size, siz
}
static size_t remoteServiceAdmin_write(void *contents, size_t size, size_t nmemb, void *userp) {
- size_t realsize = size * nmemb;
- struct get *mem = (struct get *)userp;
-
- mem->writeptr =malloc(realsize + 1);
- if (mem->writeptr == NULL) {
- /* out of memory! */
- fprintf(stderr, "not enough memory (malloc returned NULL)");
- return 0;
- } else {
- memcpy(&(mem->writeptr[mem->size]), contents, realsize);
- mem->size += realsize;
- mem->writeptr[mem->size] = 0;
- return realsize;
- }
+ struct celix_get_data_reply *get = userp;
+ fwrite(contents, size, nmemb, get->stream);
+ return size * nmemb;
}
diff --git a/libs/utils/include/celix_utils.h b/libs/utils/include/celix_utils.h
index d39e8e2..bac0d68 100644
--- a/libs/utils/include/celix_utils.h
+++ b/libs/utils/include/celix_utils.h
@@ -27,7 +27,7 @@ extern "C" {
#include <time.h>
#include <stdbool.h>
-#define CELIX_UTILS_MAX_STRLEN 1024*1024*10
+#define CELIX_UTILS_MAX_STRLEN 1024*1024*1024
/**
* Creates a copy of a provided string.