PengZheng commented on code in PR #851:
URL: https://github.com/apache/celix/pull/851#discussion_r4240776179
##########
bundles/http_admin/http_admin/src/http_admin.c:
##########
@@ -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) {
Review Comment:
`read == 0` usually means EOF, rather than an error.
> The function mg_read() receives data over an existing connection. The data
is handled as binary and is stored in a buffer whose address has been provided
as a parameter. The function returns the number of read bytes when successful,
the value 0 when the connection has been closed by peer and a negative value
when no more data could be read from the connection.
##########
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;
Review Comment:
According to official documentation:
> |**`content_length`**|`long long`| The content length of the request body.
This value can be -1 if no content length was provided. The request may still
have body data, but the server cannot determine the length until all data has
arrived (e.g. when the client closes the connection, or the final chunk of a
chunked request has been received). |
If every request body is required to have `content_length`, then we need to
check against -1.
Do we need to check against a reasonable upper bound to avoid allocating too
much memory?
##########
bundles/shell/remote_shell/src/remote_shell.c:
##########
@@ -172,27 +175,42 @@ 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);
- if (len < COMMAND_BUFF_SIZE) {
- celix_status_t commandStatus = CELIX_SUCCESS;
- buff[len] = '\0';
-
- 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
- break;
- } else { //error
-
remoteShell_connection_print(connection, RS_ERROR);
-
remoteShell_connection_print(connection, RS_PROMPT);
+ len = recv(fd, buff + used, COMMAND_BUFF_SIZE - 1 -
used, 0);
Review Comment:
This is still not robust enough. A common error breaking this is `EINTR`.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
