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 2026/10/10 17:59:18 UTC
(celix) branch feature/harden-remote-shell-loop created (now 0b909e601)
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a change to branch feature/harden-remote-shell-loop
in repository https://gitbox.apache.org/repos/asf/celix.git
at 0b909e601 Improve read loops for http admin, rsa and remote shell
This branch includes the following new commits:
new 0b909e601 Improve read loops for http admin, rsa and remote shell
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: Improve read loops for http admin, rsa and remote shell
Posted by pn...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/harden-remote-shell-loop
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 0b909e60120aaf994a1ad89fee5ea1d4548b3d8b
Author: Pepijn Noltes <pn...@apache.org>
AuthorDate: Sat Oct 10 19:57:21 2026 +0200
Improve read loops for http admin, rsa and remote shell
---
CHANGES.md | 1 +
bundles/http_admin/http_admin/src/http_admin.c | 123 ++++++++-------------
.../src/remote_service_admin_dfi.c | 71 +++++++-----
bundles/shell/remote_shell/src/remote_shell.c | 30 +++--
4 files changed, 112 insertions(+), 113 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 3eec41940..a9883c4ec 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -123,6 +123,7 @@ limitations under the License.
- Improved `INSTALL_RPATH` to use `$ORIGIN` during installation.
- Corrected bundle update behavior when updating bundles from different sources.
- Enhanced `libcurl` initialization procedures and made `libcurl` optional.
+- HTTP Admin, Remote Service Admin (DFI) and Remote Shell now keep reading until the full request body / command is received.
## Fixes
diff --git a/bundles/http_admin/http_admin/src/http_admin.c b/bundles/http_admin/http_admin/src/http_admin.c
index 7db8e1ea5..769120199 100755
--- a/bundles/http_admin/http_admin/src/http_admin.c
+++ b/bundles/http_admin/http_admin/src/http_admin.c
@@ -56,6 +56,7 @@ typedef struct http_alias {
//Local function prototypes
static int http_request_handle(struct mg_connection *connection);
+static bool httpAdmin_readRequestBody(struct mg_connection *connection, long long content_length, char **bufferOut, size_t *bytesReadOut);
static void httpAdmin_updateInfoSvc(http_admin_manager_t *admin);
static void createAliasesSymlink(const char *aliases, const char *admin_root, const char *bundle_root, long bundle_id, celix_array_list_t *alias_list);
static bool aliasList_containsAlias(celix_array_list_t *alias_list, const char *alias);
@@ -173,6 +174,34 @@ void http_admin_removeHttpService(void *handle, void *svc CELIX_UNUSED, const ce
}
}
+static bool httpAdmin_readRequestBody(struct mg_connection* connection, long long contentLength, char** bufferOut, size_t* bytesReadOut) {
+ char* buffer = NULL;
+ size_t totalBytesRead = 0;
+ bool success = true;
+ if (contentLength > 0) {
+ size_t length = (contentLength > INT_MAX) ? INT_MAX : (size_t)contentLength;
+ buffer = malloc(length + 1);
+ if (buffer == NULL) {
+ success = false;
+ } else {
+ while (totalBytesRead < length) {
+ int read = mg_read(connection, buffer + totalBytesRead, length - totalBytesRead);
+ if (read <= 0) {
+ success = false;
+ break;
+ }
+ totalBytesRead += (size_t)read;
+ }
+ if (success) {
+ buffer[totalBytesRead] = '\0';
+ }
+ }
+ }
+ *bufferOut = buffer;
+ *bytesReadOut = totalBytesRead;
+ return success;
+}
+
int http_request_handle(struct mg_connection *connection) {
int ret_status = 400; //Default bad request
@@ -208,59 +237,32 @@ int http_request_handle(struct mg_connection *connection) {
}
} else if (strcmp("POST", ri->request_method) == 0) {
if (httpSvc->doPost != NULL) {
- int bytes_read = 0;
- bool no_data = false;
char *rcv_buf = NULL;
-
- if (ri->content_length > 0) {
- int content_size = (ri->content_length > INT_MAX ? INT_MAX : (int) ri->content_length);
- rcv_buf = malloc((size_t) content_size + 1);
- rcv_buf[content_size] = '\0';
- bytes_read = mg_read(connection, rcv_buf, (size_t) content_size);
- } else {
- no_data = true;
- }
-
- if (bytes_read > 0 || no_data) {
- ret_status = httpSvc->doPost(httpSvc->handle, connection, rcv_buf, (size_t) bytes_read);
+ size_t bytesRead = 0;
+ bool success = httpAdmin_readRequestBody(connection, ri->content_length, &rcv_buf, &bytesRead);
+ if (success) {
+ ret_status = httpSvc->doPost(httpSvc->handle, connection, rcv_buf, bytesRead);
} else {
mg_send_http_error(connection, 400, "%s", "Bad request");
ret_status = 400; //Bad Request, failed to read data
}
-
- if (rcv_buf != NULL) {
- free(rcv_buf);
- }
+ free(rcv_buf);
} else {
ret_status = 0; //Let civetweb handle the request
}
} else if (strcmp("PUT", ri->request_method) == 0) {
if (httpSvc->doPut != NULL) {
- int bytes_read = 0;
- bool no_data = false;
char *rcv_buf = NULL;
-
- if (ri->content_length > 0) {
- int content_size = (ri->content_length > INT_MAX ? INT_MAX : (int) ri->content_length);
- rcv_buf = malloc((size_t) content_size + 1);
- rcv_buf[content_size] = '\0';
- bytes_read = mg_read(connection, rcv_buf, (size_t) content_size);
- } else {
- no_data = true;
- }
-
- if (bytes_read > 0 || no_data) {
- ret_status = httpSvc->doPut(httpSvc->handle, connection, req_uri, rcv_buf,
- (size_t) bytes_read);
+ size_t bytesRead = 0;
+ bool success = httpAdmin_readRequestBody(connection, ri->content_length, &rcv_buf, &bytesRead);
+ if (success) {
+ ret_status = httpSvc->doPut(httpSvc->handle, connection, req_uri, rcv_buf, bytesRead);
} else {
mg_send_http_error(connection, 400, "%s", "Bad request");
ret_status = 400; //Bad Request, failed to read data
}
-
- if (rcv_buf != NULL) {
- free(rcv_buf);
- }
+ free(rcv_buf);
} else {
ret_status = 0; //Let civetweb handle the request
}
@@ -272,29 +274,16 @@ int http_request_handle(struct mg_connection *connection) {
}
} else if (strcmp("TRACE", ri->request_method) == 0) {
if (httpSvc->doTrace != NULL) {
- int bytes_read = 0;
- bool no_data = false;
char *rcv_buf = NULL;
-
- if (ri->content_length > 0) {
- int content_size = (ri->content_length > INT_MAX ? INT_MAX : (int) ri->content_length);
- rcv_buf = malloc((size_t) content_size + 1);
- rcv_buf[content_size] = '\0';
- bytes_read = mg_read(connection, rcv_buf, (size_t) content_size);
- } else {
- no_data = true;
- }
-
- if (bytes_read > 0 || no_data) {
- ret_status = httpSvc->doTrace(httpSvc->handle, connection, rcv_buf, (size_t) bytes_read);
+ size_t bytesRead = 0;
+ bool success = httpAdmin_readRequestBody(connection, ri->content_length, &rcv_buf, &bytesRead);
+ if (success) {
+ ret_status = httpSvc->doTrace(httpSvc->handle, connection, rcv_buf, bytesRead);
} else {
mg_send_http_error(connection, 400, "%s", "Bad request");
ret_status = 400; //Bad Request, failed to read data
}
-
- if (rcv_buf != NULL) {
- free(rcv_buf);
- }
+ free(rcv_buf);
} else {
ret_status = 0; //Let civetweb handle the request
}
@@ -306,30 +295,16 @@ int http_request_handle(struct mg_connection *connection) {
}
} else if (strcmp("PATCH", ri->request_method) == 0) {
if (httpSvc->doPatch != NULL) {
- int bytes_read = 0;
- bool no_data = false;
char *rcv_buf = NULL;
-
- if (ri->content_length > 0) {
- int content_size = (ri->content_length > INT_MAX ? INT_MAX : (int) ri->content_length);
- rcv_buf = malloc((size_t) content_size + 1);
- rcv_buf[content_size] = '\0';
- bytes_read = mg_read(connection, rcv_buf, (size_t) content_size);
- } else {
- no_data = true;
- }
-
- if (bytes_read > 0 || no_data) {
- ret_status = httpSvc->doPatch(httpSvc->handle, connection, req_uri, rcv_buf,
- (size_t) bytes_read);
+ size_t bytesRead = 0;
+ bool success = httpAdmin_readRequestBody(connection, ri->content_length, &rcv_buf, &bytesRead);
+ if (success) {
+ ret_status = httpSvc->doPatch(httpSvc->handle, connection, req_uri, rcv_buf, bytesRead);
} else {
mg_send_http_error(connection, 400, "%s", "Bad request");
ret_status = 400; //Bad Request, failed to read data
}
-
- if (rcv_buf != NULL) {
- free(rcv_buf);
- }
+ free(rcv_buf);
} else {
ret_status = 0; //Let civetweb handle the request
}
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 5c02b5bbe..6f3521595 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
@@ -528,41 +528,54 @@ static int remoteServiceAdmin_callback(struct mg_connection *conn) {
if (export != NULL) {
uint64_t datalength = request_info->content_length;
char* data = malloc(datalength + 1);
- mg_read(conn, data, datalength);
- data[datalength] = '\0';
-
- char *response = NULL;
- int responceLength = 0;
- int rc = exportRegistration_call(export, data, -1, &metadata, &response, &responceLength);
- if (rc != CELIX_SUCCESS) {
- RSA_LOG_ERROR(rsa, "Error trying to invoke remove service, got error %i\n", rc);
+ size_t readLen = 0;
+ while (readLen < datalength) {
+ int this_read = mg_read(conn, data + readLen, (size_t)(datalength - readLen));
+ if (this_read <= 0) {
+ break;
+ }
+ readLen += (size_t)this_read;
}
+ data[readLen] = '\0';
- if (rc == CELIX_SUCCESS && response != NULL) {
- mg_write(conn, data_response_headers, strlen(data_response_headers));
-
- 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");
+ if (readLen < datalength) {
+ RSA_LOG_ERROR(rsa, "Error while reading request body, received %zu of %llu bytes", readLen, (unsigned long long)datalength);
+ mg_send_http_error(conn, 400, "%s", "Bad request");
+ result = 400;
+ } else {
+ char *response = NULL;
+ int responceLength = 0;
+ int rc = exportRegistration_call(export, data, -1, &metadata, &response, &responceLength);
+ if (rc != CELIX_SUCCESS) {
+ RSA_LOG_ERROR(rsa, "Error trying to invoke remove service, got error %i\n", rc);
}
- while (bytesLeft > 0) {
- int send = mg_write(conn, bufLoc, strlen(bufLoc));
- if (send > 0) {
- bytesLeft -= send;
- bufLoc += send;
- } else {
- RSA_LOG_ERROR(rsa, "Error sending response: %s", strerror(errno));
- break;
+
+ if (rc == CELIX_SUCCESS && response != NULL) {
+ mg_write(conn, data_response_headers, strlen(data_response_headers));
+
+ 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, bufLoc, strlen(bufLoc));
+ 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));
+ free(response);
+ } else {
+ mg_write(conn, no_content_response_headers, strlen(no_content_response_headers));
+ }
+ result = 1;
}
- result = 1;
free(data);
exportRegistration_decreaseUsage(export);
diff --git a/bundles/shell/remote_shell/src/remote_shell.c b/bundles/shell/remote_shell/src/remote_shell.c
index d7261774b..0b3aba4a0 100644
--- a/bundles/shell/remote_shell/src/remote_shell.c
+++ b/bundles/shell/remote_shell/src/remote_shell.c
@@ -151,10 +151,13 @@ celix_status_t remoteShell_stopConnections(remote_shell_pt instance) {
void *remoteShell_connection_run(void *data) {
celix_status_t status = CELIX_SUCCESS;
connection_pt connection = data;
- size_t len;
+ ssize_t len;
int result;
struct timeval timeout; /* Timeout for select */
+ char buff[COMMAND_BUFF_SIZE];
+ size_t used = 0;
+
int fd = fileno(connection->socketStream);
connection->threadRunning = true;
@@ -172,28 +175,35 @@ void *remoteShell_connection_run(void *data) {
/* The socket_fd has data available to be read */
if (result > 0 && FD_ISSET(fd, &connection->pollset)) {
- char buff[COMMAND_BUFF_SIZE];
-
- len = recv(fd, buff, COMMAND_BUFF_SIZE - 1, 0);
+ len = recv(fd, buff + used, COMMAND_BUFF_SIZE - 1 - used, 0);
if (len > 0) {
- if (len < COMMAND_BUFF_SIZE) {
- celix_status_t commandStatus = CELIX_SUCCESS;
- buff[len] = '\0';
+ used += (size_t)len;
+ char *newline = NULL;
+ while (status == CELIX_SUCCESS && (newline = memchr(buff, '\n', used)) != NULL) {
+ size_t consumed = (size_t)(newline - buff) + 1;
+ *newline = '\0';
- commandStatus = remoteShell_connection_execute(connection, buff);
+ celix_status_t commandStatus = remoteShell_connection_execute(connection, buff);
if (commandStatus == CELIX_SUCCESS) {
remoteShell_connection_print(connection, RS_PROMPT);
} else if (commandStatus == CELIX_FILE_IO_EXCEPTION) {
//exit command
+ status = CELIX_FILE_IO_EXCEPTION;
break;
} else { //error
remoteShell_connection_print(connection, RS_ERROR);
remoteShell_connection_print(connection, RS_PROMPT);
}
- } else {
- celix_logHelper_log(*connection->parent->loghelper, CELIX_LOG_LEVEL_ERROR, "REMOTE_SHELL: Error while retrieving data");
+ memmove(buff, buff + consumed, used - consumed);
+ used -= consumed;
+ }
+
+ if (status == CELIX_SUCCESS && used >= COMMAND_BUFF_SIZE - 1) {
+ //Buffer is full without a newline, so no complete command could be read.
+ celix_logHelper_log(*connection->parent->loghelper, CELIX_LOG_LEVEL_ERROR, "REMOTE_SHELL: Received data without a newline, dropping data");
+ used = 0;
}
} else if (len == 0) {
celix_logHelper_log(*connection->parent->loghelper, CELIX_LOG_LEVEL_INFO, "REMOTE_SHELL: Connection closed by peer");