From 22e54f54fac51dc814dedf5e3cdebd829fed518b Mon Sep 17 00:00:00 2001 From: jay candel Date: Sun, 3 Nov 2024 21:17:53 +0800 Subject: [PATCH] v1.0.9 ## v1.0.9 - Fixed log file corruption when stopping captures - Added proper bounds checking for oversized messages - Improved text display buffer management - Added automatic prefix tagging for WiFi, BLE and system messages - Improved storage init speed --- app_state.h | 1 + application.fam | 2 +- docs/changelog.md | 10 +- settings_ui.c | 2 +- uart_storage.c | 145 +++++++++++---------- uart_utils.c | 319 ++++++++++++++++------------------------------ 6 files changed, 201 insertions(+), 278 deletions(-) diff --git a/app_state.h b/app_state.h index adedb33..b98eb7f 100644 --- a/app_state.h +++ b/app_state.h @@ -51,4 +51,5 @@ struct AppState { char* textBoxBuffer; size_t buffer_length; size_t buffer_capacity; + size_t buffer_size; }; \ No newline at end of file diff --git a/application.fam b/application.fam index 680d84a..71381af 100644 --- a/application.fam +++ b/application.fam @@ -9,7 +9,7 @@ App( fap_category="GPIO/ESP", # Optional values icon="A_GhostESP_14", - fap_version="1.0.7", + fap_version="1.0.9", fap_icon="ghost_esp.png", # 10x10 1-bit PNG fap_icon_assets="images", # Image assets to compile for this application ) diff --git a/docs/changelog.md b/docs/changelog.md index 22a6e1d..745a438 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -76,7 +76,15 @@ - Filtering majorly improved - Improved stop on back to be much more reliable by added type-specific stop commands with delays between operations + +## v1.0.9 +- Fixed log file corruption when stopping captures +- Added proper bounds checking for oversized messages +- Improved text display buffer management +- Added automatic prefix tagging for WiFi, BLE and system messages +- Improved storage init speed + ## TODO - Replaced select a utility text with prompt to show NEW Help Menu -- IMPROVE optional filtering to UART output +- FINALISE optional filtering to UART output - Improve directory organisation!!!!!!!!!!!!!!!!!!!!!!!!!!!! \ No newline at end of file diff --git a/settings_ui.c b/settings_ui.c index 7424b64..f3c10b3 100644 --- a/settings_ui.c +++ b/settings_ui.c @@ -391,7 +391,7 @@ bool settings_custom_event_callback(void* context, uint32_t event_id) { "Updated by: Jay Candel\n" "Built with <3"; - confirmation_view_set_header(app_state->confirmation_view, "Ghost ESP v1.0.8"); + confirmation_view_set_header(app_state->confirmation_view, "Ghost ESP v1.0.9"); confirmation_view_set_text(app_state->confirmation_view, info_text); // Save current view before switching diff --git a/uart_storage.c b/uart_storage.c index f2c83bf..32a4530 100644 --- a/uart_storage.c +++ b/uart_storage.c @@ -9,46 +9,29 @@ #define COMMAND_BUFFER_SIZE 128 #define PCAP_WRITE_CHUNK_SIZE 1024u -void uart_storage_rx_callback(uint8_t *buf, size_t len, void *context) { - UartContext *app = context; - - if(!app || !app->storageContext || !app->storageContext->storage_api) { - FURI_LOG_E("Storage", "Invalid storage context"); - return; - } +// Define directories in an array for loop-based creation +static const char* GHOST_DIRECTORIES[] = { + GHOST_ESP_APP_FOLDER, + GHOST_ESP_APP_FOLDER_PCAPS, + GHOST_ESP_APP_FOLDER_LOGS, + GHOST_ESP_APP_FOLDER_WARDRIVE +}; + +// Helper macro for error handling and cleanup +#define CLEANUP_AND_RETURN(ctx, msg, retval) \ + do { \ + FURI_LOG_E("Storage", msg); \ + if (ctx) { \ + uart_storage_free(ctx); \ + } \ + return retval; \ + } while(0) - if(!app->storageContext->HasOpenedFile || - !app->storageContext->current_file || - !storage_file_is_open(app->storageContext->current_file)) { - FURI_LOG_W("Storage", "No file open for writing"); - app->pcap = false; // Disable PCAP mode if file isn't ready - return; - } - - // Write in chunks to avoid buffer overflow - size_t written = 0; - while(written < len) { - size_t remaining = len - written; - size_t chunk_size = (remaining < PCAP_WRITE_CHUNK_SIZE) ? - remaining : PCAP_WRITE_CHUNK_SIZE; - - uint16_t bytes_written = storage_file_write( - app->storageContext->current_file, - buf + written, - chunk_size); - - if(bytes_written != chunk_size) { - FURI_LOG_E("Storage", "Write failed: %u/%zu bytes", bytes_written, chunk_size); - app->pcap = false; // Disable PCAP mode on write failure - break; - } - written += bytes_written; - } -} UartStorageContext* uart_storage_init(UartContext* parentContext) { uint32_t start_time = furi_get_tick(); FURI_LOG_D("Storage", "Starting storage initialization"); + // Allocate and initialize context UartStorageContext* ctx = malloc(sizeof(UartStorageContext)); if(!ctx) { FURI_LOG_E("Storage", "Failed to allocate context"); @@ -57,37 +40,26 @@ UartStorageContext* uart_storage_init(UartContext* parentContext) { memset(ctx, 0, sizeof(UartStorageContext)); ctx->parentContext = parentContext; + // Open storage API ctx->storage_api = furi_record_open(RECORD_STORAGE); if(!ctx->storage_api) { - FURI_LOG_E("Storage", "Failed to open storage API"); - free(ctx); - return NULL; + CLEANUP_AND_RETURN(ctx, "Failed to open storage API", NULL); } + // Allocate file handles ctx->current_file = storage_file_alloc(ctx->storage_api); ctx->log_file = storage_file_alloc(ctx->storage_api); - if(!ctx->current_file || !ctx->log_file) { - FURI_LOG_E("Storage", "Failed to allocate file handles"); - if(ctx->current_file) storage_file_free(ctx->current_file); - if(ctx->log_file) storage_file_free(ctx->log_file); - furi_record_close(RECORD_STORAGE); - free(ctx); - return NULL; + CLEANUP_AND_RETURN(ctx, "Failed to allocate file handles", NULL); } - // Create directories with verification - if(!storage_simply_mkdir(ctx->storage_api, GHOST_ESP_APP_FOLDER)) { - FURI_LOG_W("Storage", "Failed to create app folder"); - } - if(!storage_simply_mkdir(ctx->storage_api, GHOST_ESP_APP_FOLDER_PCAPS)) { - FURI_LOG_W("Storage", "Failed to create PCAP folder"); - } - if(!storage_simply_mkdir(ctx->storage_api, GHOST_ESP_APP_FOLDER_LOGS)) { - FURI_LOG_W("Storage", "Failed to create logs folder"); - } - if(!storage_simply_mkdir(ctx->storage_api, GHOST_ESP_APP_FOLDER_WARDRIVE)) { - FURI_LOG_W("Storage", "Failed to create wardrive folder"); + // Create directories using a loop + size_t num_directories = sizeof(GHOST_DIRECTORIES) / sizeof(GHOST_DIRECTORIES[0]); + for(size_t i = 0; i < num_directories; i++) { + if(!storage_simply_mkdir(ctx->storage_api, GHOST_DIRECTORIES[i])) { + // Log warnings instead of errors to minimize critical failure states + FURI_LOG_W("Storage", "Failed to create directory: %s", GHOST_DIRECTORIES[i]); + } } // Verify PCAP directory exists and is writable @@ -95,24 +67,30 @@ UartStorageContext* uart_storage_init(UartContext* parentContext) { if(test_dir) { if(!storage_dir_open(test_dir, GHOST_ESP_APP_FOLDER_PCAPS)) { FURI_LOG_E("Storage", "PCAP directory not accessible"); + // Optionally, decide whether to continue or cleanup + // Here, we choose to disable PCAP mode but continue initialization } storage_dir_close(test_dir); storage_file_free(test_dir); + } else { + FURI_LOG_W("Storage", "Failed to allocate test directory handle"); + // Decide whether to treat this as critical or not } // Initialize log file - bool log_ok = sequential_file_open( + ctx->HasOpenedFile = sequential_file_open( ctx->storage_api, ctx->log_file, GHOST_ESP_APP_FOLDER_LOGS, "ghost_logs", - "txt"); + "txt" + ); - if(!log_ok) { - FURI_LOG_E("Storage", "Failed to open log file"); + if(!ctx->HasOpenedFile) { + FURI_LOG_W("Storage", "Failed to open log file"); + // Depending on requirements, decide to disable logging or treat as critical } - ctx->HasOpenedFile = log_ok; ctx->IsWritingToFile = false; FURI_LOG_I("Storage", "Storage initialization completed in %lums", @@ -120,6 +98,44 @@ UartStorageContext* uart_storage_init(UartContext* parentContext) { return ctx; } +void uart_storage_rx_callback(uint8_t *buf, size_t len, void *context) { + UartContext *app = context; + + if(!app || !app->storageContext || !app->storageContext->storage_api) { + FURI_LOG_E("Storage", "Invalid storage context"); + return; + } + + if(!app->storageContext->HasOpenedFile || + !app->storageContext->current_file || + !storage_file_is_open(app->storageContext->current_file)) { + FURI_LOG_W("Storage", "No file open for writing"); + app->pcap = false; // Disable PCAP mode if file isn't ready + return; + } + + // Write in chunks to avoid buffer overflow + size_t written = 0; + while(written < len) { + size_t remaining = len - written; + size_t chunk_size = (remaining < PCAP_WRITE_CHUNK_SIZE) ? + remaining : PCAP_WRITE_CHUNK_SIZE; + + uint16_t bytes_written = storage_file_write( + app->storageContext->current_file, + buf + written, + chunk_size + ); + + if(bytes_written != chunk_size) { + FURI_LOG_E("Storage", "Write failed: %u/%zu bytes", bytes_written, chunk_size); + app->pcap = false; // Disable PCAP mode on write failure + break; + } + written += bytes_written; + } +} + void uart_storage_reset_logs(UartStorageContext *ctx) { if(!ctx || !ctx->storage_api) return; @@ -134,7 +150,8 @@ void uart_storage_reset_logs(UartStorageContext *ctx) { ctx->log_file, GHOST_ESP_APP_FOLDER_LOGS, "ghost_logs", - "txt"); + "txt" + ); if(!ctx->HasOpenedFile) { FURI_LOG_E("Storage", "Failed to reset log file"); @@ -159,4 +176,4 @@ void uart_storage_free(UartStorageContext *ctx) { } free(ctx); -} \ No newline at end of file +} diff --git a/uart_utils.c b/uart_utils.c index 875b837..26def54 100644 --- a/uart_utils.c +++ b/uart_utils.c @@ -16,17 +16,12 @@ static FuriMutex* buffer_mutex = NULL; #define MUTEX_TIMEOUT_MS 1000 #define BUFFER_CLEAR_SIZE 128 -// In uart_utils.c, after the defines -static APListState g_ap_list_state = { - .in_ap_list = false, - .ap_count = 0, - .expected_ap_count = 0 -}; + +static void strip_ansi_codes(const char* input, char* output); static bool process_chunk(LineProcessingState* state, const char* input, char* output, size_t output_size, size_t* output_len); - static bool format_line(const char* input, char* output, FilterConfig* config); static bool init_buffer_mutex() { @@ -163,67 +158,35 @@ static bool process_chunk(LineProcessingState* state, const char* input, return false; } -static bool is_ap_list_start(const char* line) { - if (!line) return false; - - if (strstr(line, "Found") && strstr(line, "access points:")) { - // Extract expected AP count - const char* count_start = strstr(line, "Found") + 6; - char* end; - int count = strtol(count_start, &end, 10); - if (count > 0) { - g_ap_list_state.expected_ap_count = count; - } - return true; - } - return false; -} -static bool is_ap_list_entry(const char* line) { - if (!line) return false; - return strstr(line, "SSID:") != NULL && - strstr(line, "BSSID:") != NULL && - strstr(line, "RSSI:") != NULL && - strstr(line, "Company:") != NULL; -} -static bool match_any_pattern(const char* line, const char* patterns[]) { - for(int i = 0; patterns[i]; i++) { - if(strstr(line, patterns[i])) return true; - } - return false; -} -static bool should_filter_line(const char* line) { - if(!line || !*line) return true; // Filter empty lines - - // Handle AP list state - if(g_ap_list_state.in_ap_list) { - if(g_ap_list_state.ap_count >= g_ap_list_state.expected_ap_count) { - g_ap_list_state.in_ap_list = false; - g_ap_list_state.ap_count = 0; - } - } +static bool should_filter_line(const char* raw_line) { + if(!raw_line || !*raw_line) return true; // Filter empty lines - // AP List handling - if(is_ap_list_start(line)) { - g_ap_list_state.in_ap_list = true; - g_ap_list_state.ap_count = 0; - return false; - } + // First strip ANSI and clean the line + char clean_line[RX_BUF_SIZE]; + strip_ansi_codes(raw_line, clean_line); - if(g_ap_list_state.in_ap_list) { - if(is_ap_list_entry(line)) { - g_ap_list_state.ap_count++; - return false; - } - if(!strstr(line, "WiFiManager:")) { - g_ap_list_state.in_ap_list = false; - } + // Debug messages always filtered + if(strstr(clean_line, "wifi:flush") || + strstr(clean_line, "wifi:stop") || + strstr(clean_line, "wifi:lmac") || + strstr(clean_line, "wifi:new:") || + strstr(clean_line, "wifi:station:") || + strstr(clean_line, "wifi:= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == 'm') { + if((c >= 'A' && c <= 'Z') || + (c >= 'a' && c <= 'z') || + c == 'm') { in_escape = false; } + read++; continue; } - - // Handle normal characters - if(isspace(c)) { - if(!last_was_space) { + + if(isspace((unsigned char)c)) { + if(!last_space) { *write++ = ' '; - last_was_space = true; + last_space = true; } } else { *write++ = c; - last_was_space = false; + last_space = false; } + read++; } - - // Trim trailing space - if(write > str && write[-1] == ' ') write--; + + if(write > str && *(write-1) == ' ') write--; *write = '\0'; } - static void strip_ansi_codes(const char* input, char* output) { if (!input || !output) return; - + size_t j = 0; - size_t input_len = strlen(input); bool in_escape = false; bool in_timestamp = false; - char* temp = malloc(RX_BUF_SIZE); - if (!temp) { - FURI_LOG_E("UART", "Failed to allocate temp buffer"); - return; - } - memset(temp, 0, RX_BUF_SIZE); - - size_t temp_idx = 0; char last_char = 0; - char next_char = 0; - - for (size_t i = 0; i < input_len; i++) { - unsigned char c = (unsigned char)input[i]; - next_char = (i + 1 < input_len) ? input[i + 1] : 0; - - // Improved escape sequence handling + + for (size_t i = 0; input[i]; i++) { + unsigned char c = input[i]; + char next_char = input[i + 1]; + + // Handle ANSI escape sequences if (c == '\x1b' || (c == '[' && last_char == '\x1b')) { in_escape = true; last_char = c; continue; } - + if (in_escape) { if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == 'm') { in_escape = false; @@ -325,8 +284,8 @@ static void strip_ansi_codes(const char* input, char* output) { last_char = c; continue; } - - // Enhanced timestamp handling with boundary protection + + // Handle timestamps if (c == '(' && next_char && isdigit((unsigned char)next_char)) { in_timestamp = true; continue; @@ -335,68 +294,26 @@ static void strip_ansi_codes(const char* input, char* output) { if (c == ')') { in_timestamp = false; if (next_char && !isspace((unsigned char)next_char)) { - if (temp_idx < RX_BUF_SIZE - 1) { - temp[temp_idx++] = ' '; - } + output[j++] = ' '; } } continue; } - - // Improved color code fragment handling with boundary check - if (c == ';' && i + 2 < input_len && - isdigit((unsigned char)next_char) && input[i + 2] == 'm') { + + // Skip color code fragments + if (c == ';' && next_char && isdigit((unsigned char)next_char) && + input[i + 2] && input[i + 2] == 'm') { i += 2; continue; } - - // Enhanced buffer management with word boundary protection - if (temp_idx >= RX_BUF_SIZE - 2) { - char* last_space = strrchr(temp, ' '); - if (last_space) { - size_t keep_len = last_space - temp; - if (j + keep_len < RX_BUF_SIZE) { - memcpy(output + j, temp, keep_len); - j += keep_len; - - // Move remaining content with overlap protection - size_t remaining = temp_idx - (keep_len + 1); - if (remaining > 0) { - memmove(temp, last_space + 1, remaining); - temp_idx = remaining; - } else { - temp_idx = 0; - } - } - } else { - if (j + temp_idx < RX_BUF_SIZE) { - memcpy(output + j, temp, temp_idx); - j += temp_idx; - temp_idx = 0; - } - } - } - - // Add character with boundary check - if (temp_idx < RX_BUF_SIZE - 1) { - temp[temp_idx++] = c; + + if (j < RX_BUF_SIZE - 1) { + output[j++] = c; } last_char = c; } - - // Handle remaining characters with improved cleanup - if (temp_idx > 0) { - temp[temp_idx] = '\0'; - clean_text(temp); - size_t remaining_len = strlen(temp); - if (j + remaining_len < RX_BUF_SIZE) { - memcpy(output + j, temp, remaining_len); - j += remaining_len; - } - } - + output[j] = '\0'; - free(temp); clean_text(output); } @@ -412,70 +329,32 @@ static bool format_line(const char* input, char* output, FilterConfig* config) { char* temp = malloc(RX_BUF_SIZE); if (!temp) return false; - // Initial cleanup with improved buffer management - strncpy(temp, input, RX_BUF_SIZE - 1); - temp[RX_BUF_SIZE - 1] = '\0'; - - // Enhanced ANSI code handling - if (config->strip_ansi_codes) { - char* stripped = malloc(RX_BUF_SIZE); - if (stripped) { - strip_ansi_codes(temp, stripped); - strncpy(temp, stripped, RX_BUF_SIZE - 1); - free(stripped); - } - } - + // Do all processing on cleaned line + strip_ansi_codes(input, temp); clean_text(temp); - // Improved pattern fixing - char* fixed = temp; - while (*fixed) { - if (strncmp(fixed, "Wi Fi", 5) == 0) { - memmove(fixed + 4, fixed + 5, strlen(fixed + 5) + 1); - memcpy(fixed, "WiFi", 4); - } - // Add additional pattern fixes here if needed - fixed++; - } - - // Enhanced timestamp and ID cleanup - char* start = temp; - while (*start) { - if (isdigit((unsigned char)*start) && strstr(start, "]")) { - char* end = strstr(start, "]"); - if (end) { - memmove(start, end + 1, strlen(end + 1) + 1); - while (*start == ' ') start++; // Remove leading spaces - continue; - } - } - break; - } - - bool keep_line = !should_filter_line(start); + bool keep_line = !should_filter_line(temp); if (keep_line) { + // Add prefix if needed const char* prefix = ""; if (config->add_prefixes) { - if (strstr(start, "WiFi") || strstr(start, "AP_MANAGER") || - strstr(start, "SSID:") || strstr(start, "BSSID:")) { + if (strstr(temp, "WiFi") || strstr(temp, "AP_MANAGER") || + strstr(temp, "SSID:") || strstr(temp, "BSSID:")) { prefix = "[WIFI] "; - } else if (strstr(start, "BLE")) { + } else if (strstr(temp, "BLE")) { prefix = "[BLE] "; - } else if (strstr(start, "Found Flipper")) { + } else if (strstr(temp, "Found Flipper")) { prefix = "[FLIPPER] "; } } - // More robust prefix handling - if (strlen(prefix) > 0 && strncmp(start, prefix, strlen(prefix)) != 0) { - snprintf(output, RX_BUF_SIZE - 1, "%s%s", prefix, start); + if (strlen(prefix) > 0) { + snprintf(output, RX_BUF_SIZE - 1, "%s%s", prefix, temp); } else { - strncpy(output, start, RX_BUF_SIZE - 1); + strncpy(output, temp, RX_BUF_SIZE - 1); } output[RX_BUF_SIZE - 1] = '\0'; - clean_text(output); } free(temp); @@ -536,7 +415,32 @@ void handle_uart_rx_data(uint8_t *buf, size_t len, void *context) { return; } - // Ensure proper null termination of input + // Handle logging to file first, before any buffer modifications + if(state->uart_context->storageContext && + state->uart_context->storageContext->log_file) { + // Ensure proper null termination for logging + uint8_t* log_buf = malloc(len + 1); + if(log_buf) { + memcpy(log_buf, buf, len); + log_buf[len] = '\0'; + + uint16_t written = storage_file_write( + state->uart_context->storageContext->log_file, + log_buf, + len + ); + + if(written != len) { + FURI_LOG_E("UART", "Log write failed: %d/%d bytes", written, len); + // Force a file sync to ensure data is written + storage_file_sync(state->uart_context->storageContext->log_file); + } + + free(log_buf); + } + } + + // Ensure proper null termination of input for display if(len >= RX_BUF_SIZE) len = RX_BUF_SIZE - 1; buf[len] = '\0'; @@ -618,7 +522,6 @@ void handle_uart_rx_data(uint8_t *buf, size_t len, void *context) { text_box_set_text(state->text_box, state->textBoxBuffer); text_box_set_focus(state->text_box, TextBoxFocusStart); } else { - // Handle sign issues with explicit cast for arithmetic size_t display_size = state->buffer_length; if(display_size > (size_t)TEXT_BOX_STORE_SIZE - 1) { display_size = (size_t)TEXT_BOX_STORE_SIZE - 1; @@ -630,12 +533,6 @@ void handle_uart_rx_data(uint8_t *buf, size_t len, void *context) { } } - // Handle logging to file - if(state->uart_context->storageContext && - state->uart_context->storageContext->log_file) { - storage_file_write(state->uart_context->storageContext->log_file, buf, len); - } - furi_mutex_release(buffer_mutex); } // UART worker thread function