Improve rsa dfi http error handling and logging

Author: pnoltes

bundles/remote_services/remote_service_admin_dfi/src/remote_service_admin_dfi.c

@@ -140,6 +140,16 @@ static const char *no_content_response_headers =
 
 static const unsigned int DEFAULT_TIMEOUT = 0;
 
+static void rsa_send_http_error(struct mg_connection *conn, int code, const char *msg);
+
+#define RSA_LOG_REQUEST_ERROR(rsa, conn, fmt, ...) \
+    do { \
+        const struct mg_request_info *ri = mg_get_request_info(conn); \
+        const char *client_ip = ri->remote_addr ? ri->remote_addr : "unknown"; \
+        celix_logHelper_log((rsa)->loghelper, CELIX_LOG_LEVEL_ERROR, \
+            "[%s %s from %s] " fmt, ri->request_method, ri->request_uri, client_ip, ##__VA_ARGS__); \
+    } while(0)
+
 static int remoteServiceAdmin_callback(struct mg_connection *conn);
 static celix_status_t remoteServiceAdmin_createEndpointDescription(remote_service_admin_t *admin, service_reference_pt reference, celix_properties_t *props, char *interface, endpoint_description_t **description);
 static celix_status_t remoteServiceAdmin_send(void *handle, endpoint_description_t *endpointDescription, char *request, celix_properties_t *metadata, char **reply);
@@ -150,6 +160,18 @@ static size_t remoteServiceAdmin_write(void *contents, size_t size, size_t nmemb
 static void remoteServiceAdmin_setupStopExportsThread(remote_service_admin_t* admin);
 static void remoteServiceAdmin_teardownStopExportsThread(remote_service_admin_t* admin);
 
+static void rsa_send_http_error(struct mg_connection *conn, int code, const char *msg) {
+    const char *reason = "Error";
+    switch (code) {
+        case 400: reason = "Bad Request"; break;
+        case 404: reason = "Not Found"; break;
+        case 413: reason = "Payload Too Large"; break;
+        case 500: reason = "Internal Server Error"; break;
+        case 503: reason = "Service Unavailable"; break;
+    }
+    mg_printf(conn, "HTTP/1.1 %d %s\r\nContent-Type: text/plain\r\nContent-Length: %zu\r\n\r\n%s", code, reason, strlen(msg), msg);
+}
+
 static void remoteServiceAdmin_curlshare_lock(CURL *handle, curl_lock_data data, curl_lock_access laccess, void *userptr)
 {
     (void)handle;
@@ -469,62 +491,67 @@ static int remoteServiceAdmin_callback(struct mg_connection *conn) {
 
         if (strncmp(request_info->request_uri, "/service/", 9) == 0 && strcmp("POST", request_info->request_method) == 0) {
 
-            // uri = /services/myservice/call
+            // uri = /service/{serviceId}/{rest}
             const char *uri = request_info->request_uri;
-            // rest = myservice/call
+            const char *rest = uri + 9;
 
-            const char *rest = uri+9;
             char *interfaceStart = strchr(rest, '/');
-            int pos = interfaceStart - rest;
-            char service[pos+1];
-            strncpy(service, rest, pos);
-            service[pos] = '\0';
-            unsigned long serviceId = strtoul(service,NULL,10);
-
-            for (int i = 0; i < request_info->num_headers; i++) {
-                struct mg_header header = request_info->http_headers[i];
-                if (strncmp(header.name, "X-RSA-Metadata-", 15) == 0) {
-                    if (metadata == NULL) {
-                        metadata = celix_properties_create();
+            int pos = interfaceStart ? (int)(interfaceStart - rest) : -1;
+            if (interfaceStart == NULL || pos <= 0) {
+                RSA_LOG_REQUEST_ERROR(rsa, conn, "Invalid service URI format: %s", uri);
+                rsa_send_http_error(conn, 400, "Invalid service URI format");
+                result = 1;
+            } else {
+                char service[pos + 1];
+                    strncpy(service, rest, pos);
+                    service[pos] = '\0';
+                    unsigned long serviceId = strtoul(service, NULL, 10);
+
+                    for (int i = 0; i < request_info->num_headers; i++) {
+                        struct mg_header header = request_info->http_headers[i];
+                        if (strncmp(header.name, "X-RSA-Metadata-", 15) == 0) {
+                            if (metadata == NULL) {
+                                metadata = celix_properties_create();
+                            }
+                            celix_properties_set(metadata, header.name + 15, header.value);
+                        }
                     }
-                    celix_properties_set(metadata, header.name + 15, header.value);
-                }
-            }
 
-            celixThreadRwlock_readLock(&rsa->exportedServicesLock);
-
-            //find endpoint
-            hash_map_iterator_pt iter = hashMapIterator_create(rsa->exportedServices);
-            while (hashMapIterator_hasNext(iter)) {
-                hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
-                celix_array_list_t *exports = hashMapEntry_getValue(entry);
-                int expIt = 0;
-                for (expIt = 0; expIt < celix_arrayList_size(exports); expIt++) {
-                    export_registration_t *check = celix_arrayList_get(exports, expIt);
-                    export_reference_t * ref = NULL;
-                    exportRegistration_getExportReference(check, &ref);
-                    endpoint_description_t * checkEndpoint = NULL;
-                    exportReference_getExportedEndpoint(ref, &checkEndpoint);
-                    if (serviceId == checkEndpoint->serviceId) {
-                        export = check;
-                        free(ref);
-                        break;
+                    celixThreadRwlock_readLock(&rsa->exportedServicesLock);
+
+                    //find endpoint
+                    hash_map_iterator_pt iter = hashMapIterator_create(rsa->exportedServices);
+                    while (hashMapIterator_hasNext(iter)) {
+                        hash_map_entry_pt entry = hashMapIterator_nextEntry(iter);
+                        celix_array_list_t *exports = hashMapEntry_getValue(entry);
+                        int expIt = 0;
+                        for (expIt = 0; expIt < celix_arrayList_size(exports); expIt++) {
+                            export_registration_t *check = celix_arrayList_get(exports, expIt);
+                            export_reference_t * ref = NULL;
+                            exportRegistration_getExportReference(check, &ref);
+                            endpoint_description_t * checkEndpoint = NULL;
+                            exportReference_getExportedEndpoint(ref, &checkEndpoint);
+                            if (serviceId == checkEndpoint->serviceId) {
+                                export = check;
+                                free(ref);
+                                break;
+                            }
+                            free(ref);
+                        }
                     }
-                    free(ref);
-                }
-            }
-            hashMapIterator_destroy(iter);
+                    hashMapIterator_destroy(iter);
 
-            if (export != NULL) {
-                exportRegistration_increaseUsage(export);
-            } else {
-                result = 0;
-                RSA_LOG_WARNING(rsa, "No export registration found for service id %lu", serviceId);
+                    if (export != NULL) {
+                        exportRegistration_increaseUsage(export);
+                    } else {
+                        RSA_LOG_REQUEST_ERROR(rsa, conn, "No export registration found for service id %lu", serviceId);
+                        rsa_send_http_error(conn, 404, "Service not found");
+                        result = 1;
+                    }
+                    celixThreadRwlock_unlock(&rsa->exportedServicesLock);
             }
-            celixThreadRwlock_unlock(&rsa->exportedServicesLock);
         }
 
-
         if (export != NULL) {
             uint64_t datalength = request_info->content_length;
             char* data = malloc(datalength + 1);