From a58638ced21b3eda9f8e8449616730e232490881 Mon Sep 17 00:00:00 2001 From: Ken Van Hoeylandt Date: Thu, 3 Sep 2026 23:17:03 +0200 Subject: [PATCH] Refactor execution logic --- Modules/app-module/include/app/exec.h | 13 -- Modules/app-module/include/app/manager.h | 47 +++++ .../app-module/private/app/private/ledger.h | 1 + Modules/app-module/source/exec.cpp | 111 ++--------- Modules/app-module/source/manager.cpp | 65 +++++-- Modules/app-module/tests/source/exec_test.cpp | 81 ++++++++ .../app-module/tests/source/manager_test.cpp | 177 ++++++++++++++++++ Tactility/Source/app/files/View.cpp | 11 +- 8 files changed, 382 insertions(+), 124 deletions(-) diff --git a/Modules/app-module/include/app/exec.h b/Modules/app-module/include/app/exec.h index 13d9660cd..8732ca56e 100644 --- a/Modules/app-module/include/app/exec.h +++ b/Modules/app-module/include/app/exec.h @@ -1,8 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 #pragma once -#include - #include #include @@ -41,17 +39,6 @@ bool app_exec_path_allowed(const char* file_path); */ bool app_exec_is_executable_path(const char* path); -/** - * Runs @a path directly, without it having to be pre-registered as an app (e.g. via - * app_manager_add() or app_install()), for a file a user picked in a file browser. Internally - * registers (app_manager_add(), APP_MANIFEST_FLAG_HIDDEN so it never shows up in app-browsing - * UIs) a manifest for @a path the first time it's run, reusing that same registration on - * subsequent calls for the same path, then starts it (app_manager_start_with_parameters()). - * @retval ERROR_NOT_ALLOWED app_exec_is_executable_path(path) is false - * @retval ERROR_NONE on success - */ -error_t app_exec_run_path(const char* path, int argc, const char* const argv[], AppInstanceId* out_app_instance_id); - #ifdef __cplusplus } #endif diff --git a/Modules/app-module/include/app/manager.h b/Modules/app-module/include/app/manager.h index 002629290..b87169445 100644 --- a/Modules/app-module/include/app/manager.h +++ b/Modules/app-module/include/app/manager.h @@ -128,6 +128,53 @@ error_t app_manager_start_with_streams(const char* id, const struct AppStreamBin */ error_t app_manager_start_for_result_with_streams(const char* id, AppInstanceId parent_instance_id, int argc, const char* const argv[], const struct AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id); +/** + * Starts an app directly from @a location, without it having to be pre-registered via + * app_manager_add() first. Independent of the id-based app_manager_start*() family: performs no + * checks of its own beyond what AppLoaderApi::load() itself rejects. Not manifest lookup, and + * not whether @a location should be allowed to run (see app_exec_is_executable_path(), + * app/exec.h, for that): callers that need a check must run it themselves before calling this. + * @param[in] stack stack allocation config for the app's task; all-zero uses the scheduler's + * default depth/capability, same as an AppManifest that leaves AppManifest::stack zeroed + * @param[in] argv see app_manager_start_with_parameters() + * @retval ERROR_NOT_FOUND no AppLoaderApi is registered for @a location.type + * @retval ERROR_NONE on success + */ +error_t app_manager_start_location(struct AppLocation location, struct AppStackConfig stack, int argc, const char* const argv[], AppInstanceId* out_app_instance_id); + +/** + * Same as app_manager_start_location(), but as a modal child of @a parent_instance_id - see + * app_manager_start_for_result()'s own doc for the result-delivery contract. Still no manifest + * and no check of its own beyond what AppLoaderApi::load() itself rejects. + * @param[in] argv see app_manager_start_with_parameters() + * @retval ERROR_NOT_FOUND no AppLoaderApi is registered for @a location.type + * @retval ERROR_NONE on success + */ +error_t app_manager_start_location_for_result(struct AppLocation location, struct AppStackConfig stack, AppInstanceId parent_instance_id, int argc, const char* const argv[], AppInstanceId* out_app_instance_id); + +/** + * Same as app_manager_start_location(), but installs @a bindings into the new instance's fd + * table before its task begins executing - see app_manager_start_with_streams()'s own doc for + * stream ownership. + * @param[in] bindings see app_manager_start_with_streams() + * @retval ERROR_NOT_FOUND no AppLoaderApi is registered for @a location.type + * @retval ERROR_OUT_OF_RANGE a binding's producer_fd is out of range + * @retval ERROR_RESOURCE a binding's event_group has no free bits left to claim + * @retval ERROR_NONE on success + */ +error_t app_manager_start_location_with_streams(struct AppLocation location, struct AppStackConfig stack, const struct AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id); + +/** + * Combines app_manager_start_location_for_result() and app_manager_start_location_with_streams(). + * @param[in] argv see app_manager_start_with_parameters() + * @param[in] bindings see app_manager_start_with_streams() + * @retval ERROR_NOT_FOUND no AppLoaderApi is registered for @a location.type + * @retval ERROR_OUT_OF_RANGE a binding's producer_fd is out of range + * @retval ERROR_RESOURCE a binding's event_group has no free bits left to claim + * @retval ERROR_NONE on success + */ +error_t app_manager_start_location_for_result_with_streams(struct AppLocation location, struct AppStackConfig stack, AppInstanceId parent_instance_id, int argc, const char* const argv[], const struct AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id); + /** * Stop an app instance permanently. Emits APP_EVENT_CLOSE and bound-waits for its task to exit * if it was running. diff --git a/Modules/app-module/private/app/private/ledger.h b/Modules/app-module/private/app/private/ledger.h index 52efa5aeb..712924fcc 100644 --- a/Modules/app-module/private/app/private/ledger.h +++ b/Modules/app-module/private/app/private/ledger.h @@ -38,6 +38,7 @@ struct AppCompletionSignal { /** A registered/running app instance, as tracked internally by app-module. */ struct AppInstanceRecord { uint32_t id; + /** NULL for an instance started via app_manager_start_location() (no manifest involved). */ const AppManifest* manifest; AppInstanceState state; /** The FreeRTOS task currently executing AppLoaderApi::run() for this instance; NULL when not running. */ diff --git a/Modules/app-module/source/exec.cpp b/Modules/app-module/source/exec.cpp index 224fa9fad..24f994437 100644 --- a/Modules/app-module/source/exec.cpp +++ b/Modules/app-module/source/exec.cpp @@ -5,8 +5,6 @@ #include #include -#include -#include #include #include @@ -17,12 +15,10 @@ #include #include -#include +#include #include -#include #include #include -#include constexpr auto* TAG = "app_exec"; @@ -81,6 +77,19 @@ bool has_dotdot_segment(const std::string& path) { return false; } +// Resolves @a absolute_path through every symlink on its way (POSIX realpath()), so a symlink +// sitting inside an allowed directory can't point at a target outside it. is_under_directory() +// only ever sees where a path physically ends up, never the lexical path a caller wrote. Falls +// back to @a absolute_path unresolved if realpath() fails (e.g. nothing exists there yet): a +// nonexistent path can't be dlopen()ed either way, so comparing it unresolved is still safe. +std::string resolve_physical(const std::string& absolute_path) { + char resolved[PATH_MAX]; + if (realpath(absolute_path.c_str(), resolved) == nullptr) { + return absolute_path; + } + return std::string(resolved); +} + // True when @a file_path lives strictly under @a dir (a direct or nested child). Never equal to // @a dir itself: a bare directory is never itself an executable candidate. bool is_under_directory(const std::string& file_path, const std::string& dir) { @@ -98,49 +107,6 @@ const AppLoaderApi* find_path_loader_api() { return static_cast(service_instance_get_data(instance)); } -// region Ad hoc "run this path directly" registry: owns the AppManifest (and its id/name/path -// strings) that app_manager's own ledger only keeps a non-owning pointer to (see -// app_manager_add()'s contract; install.cpp's InstallRegistry exists for the same reason). -// Keyed by path so re-running the same file reuses its manifest instead of re-adding it. - -struct RunRecord { - std::string id; - std::string name; - std::string path; - AppManifest manifest {}; -}; - -struct RunRegistry { - std::unordered_map> apps; - Mutex mutex {}; - - RunRegistry() { mutex_construct(&mutex); } -}; - -RunRegistry& run_registry() { - static RunRegistry registry; - return registry; -} - -std::string last_path_segment(const std::string& path) { - auto index = path.find_last_of('/'); - return index == std::string::npos ? path : path.substr(index + 1); -} - -// A short, stable-per-path id that fits AppManifest::id's APP_ID_LENGTH limit regardless of how -// long @a path is. FNV-1a's collision odds are irrelevant at the scale of "distinct executables -// one user runs from the Files app". -std::string make_run_id(const std::string& path) { - uint32_t hash = 2166136261u; - for (unsigned char c: path) { - hash ^= c; - hash *= 16777619u; - } - char id[16]; - std::snprintf(id, sizeof(id), "run.%08x", static_cast(hash)); - return id; -} - } // namespace extern "C" { @@ -193,12 +159,16 @@ bool app_exec_path_allowed(const char* file_path) { if (has_dotdot_segment(path)) { return false; } + std::string resolved_path = resolve_physical(path); auto& reg = registry(); mutex_lock(®.mutex); bool allowed = false; for (size_t i = 0; i < reg.count; i++) { - if (is_under_directory(path, reg.paths[i])) { + // Resolved at check time, not at app_exec_path_add() time: the registered directory + // itself may not exist yet when it's registered (e.g. before the first app_install()). + std::string resolved_dir = resolve_physical(reg.paths[i]); + if (is_under_directory(resolved_path, resolved_dir)) { allowed = true; break; } @@ -217,49 +187,6 @@ bool app_exec_is_executable_path(const char* path) { return loader->is_executable(location); } -error_t app_exec_run_path(const char* path, int argc, const char* const argv[], AppInstanceId* out_app_instance_id) { - if (!app_exec_is_executable_path(path)) { - return ERROR_NOT_ALLOWED; - } - - std::string path_str(path); - auto& reg = run_registry(); - mutex_lock(®.mutex); - - auto iterator = reg.apps.find(path_str); - std::string id; - if (iterator != reg.apps.end()) { - id = iterator->second->id; - mutex_unlock(®.mutex); - } else { - auto record = std::make_unique(); - record->id = make_run_id(path_str); - record->name = last_path_segment(path_str); - record->path = path_str; - record->manifest = AppManifest { - .id = record->id.c_str(), - .name = record->name.c_str(), - .category = APP_CATEGORY_USER, - .location = { APP_LOCATION_PATH, const_cast(record->path.c_str()) }, - .flags = APP_MANIFEST_FLAG_HIDDEN, - .stack = { .depth = 0, .desired_memory_capability = 0 }, - }; - - error_t add_result = app_manager_add(&record->manifest); - if (add_result != ERROR_NONE) { - mutex_unlock(®.mutex); - LOG_E(TAG, "Failed to register %s: %s", path, error_to_string(add_result)); - return add_result; - } - - id = record->id; - reg.apps[path_str] = std::move(record); - mutex_unlock(®.mutex); - } - - return app_manager_start_with_parameters(id.c_str(), argc, argv, out_app_instance_id); -} - void app_exec_register_default_paths() { char data_root[192]; if (paths_get_data_path(data_root, sizeof(data_root)) == ERROR_NONE) { diff --git a/Modules/app-module/source/manager.cpp b/Modules/app-module/source/manager.cpp index 020b0d3f3..ed1931f84 100644 --- a/Modules/app-module/source/manager.cpp +++ b/Modules/app-module/source/manager.cpp @@ -92,8 +92,10 @@ char** copy_arguments(int argc, const char* const argv[]) { // app_scheduler_start() frees it on any failure path, and the spawned task frees it once its // run() returns. @a bindings (@a binding_count entries, may be NULL/0) are subscribed into the // new instance's fd table before app_scheduler_start() is called, so they're in place before its -// task begins executing (see app_manager_start_with_streams()). -error_t start_internal(const char* id, AppInstanceId parent_instance_id, int argc, char* argv[], const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) { +// task begins executing (see app_manager_start_with_streams()). @a manifest may be NULL, for a +// location-based start with no manifest at all (see app_manager_start_location()); it's stored +// on the ledger record as-is and never dereferenced here beyond the log line below. +error_t start_internal(const AppManifest* manifest, AppLocation location, AppStackConfig stack, AppInstanceId parent_instance_id, int argc, char* argv[], const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) { if (binding_count != 0 && bindings == nullptr) { app_ledger_free_arguments(argc, argv); return ERROR_INVALID_ARGUMENT; @@ -102,14 +104,6 @@ error_t start_internal(const char* id, AppInstanceId parent_instance_id, int arg auto& ledger = app_ledger(); mutex_lock(&ledger.mutex); - auto manifest_iterator = ledger.manifests.find(id); - if (manifest_iterator == ledger.manifests.end()) { - mutex_unlock(&ledger.mutex); - app_ledger_free_arguments(argc, argv); - return ERROR_NOT_FOUND; - } - const AppManifest* manifest = manifest_iterator->second; - AppInstanceId target_id = ledger.next_instance_id++; AppInstanceRecord record { .id = target_id, .manifest = manifest, .state = APP_INSTANCE_STATE_STARTING, .task = nullptr }; record.parent_id = parent_instance_id; @@ -120,7 +114,7 @@ error_t start_internal(const char* id, AppInstanceId parent_instance_id, int arg app_fd_table_construct(&ledger.instances[target_id].fd_table); mutex_unlock(&ledger.mutex); - LOG_I(TAG, "[instance %d] starting %s with parent %d", target_id, manifest->id, parent_instance_id); + LOG_I(TAG, "[instance %d] starting %s with parent %d", target_id, manifest != nullptr ? manifest->id : "", parent_instance_id); for (size_t i = 0; i < binding_count; i++) { error_t bind_result = app_stream_subscribe(bindings[i].stream, bindings[i].buffer, bindings[i].buffer_capacity, bindings[i].event_group, target_id, bindings[i].producer_fd); @@ -141,7 +135,7 @@ error_t start_internal(const char* id, AppInstanceId parent_instance_id, int arg } } - error_t error = app_scheduler_start(target_id, manifest->location, manifest->stack, argc, argv); + error_t error = app_scheduler_start(target_id, location, stack, argc, argv); if (error != ERROR_NONE) { // Every binding succeeded before app_scheduler_start() failed. Unsubscribe all of them, // same reasoning as the bind-failure path above. @@ -160,26 +154,60 @@ error_t start_internal(const char* id, AppInstanceId parent_instance_id, int arg return ERROR_NONE; } +// Looks @a id up in the manifest registry, then delegates to start_internal(). The only path +// that requires a registered manifest; app_manager_start_location() bypasses this entirely. +error_t start_internal_by_id(const char* id, AppInstanceId parent_instance_id, int argc, char* argv[], const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) { + auto& ledger = app_ledger(); + + mutex_lock(&ledger.mutex); + auto manifest_iterator = ledger.manifests.find(id); + if (manifest_iterator == ledger.manifests.end()) { + mutex_unlock(&ledger.mutex); + app_ledger_free_arguments(argc, argv); + return ERROR_NOT_FOUND; + } + const AppManifest* manifest = manifest_iterator->second; + mutex_unlock(&ledger.mutex); + + return start_internal(manifest, manifest->location, manifest->stack, parent_instance_id, argc, argv, bindings, binding_count, out_app_instance_id); +} + } // namespace error_t app_manager_start(const char* id, AppInstanceId* out_app_instance_id) { - return start_internal(id, 0, 0, nullptr, nullptr, 0, out_app_instance_id); + return start_internal_by_id(id, 0, 0, nullptr, nullptr, 0, out_app_instance_id); } error_t app_manager_start_with_parameters(const char* id, int argc, const char* const argv[], AppInstanceId* out_app_instance_id) { - return start_internal(id, 0, argc, copy_arguments(argc, argv), nullptr, 0, out_app_instance_id); + return start_internal_by_id(id, 0, argc, copy_arguments(argc, argv), nullptr, 0, out_app_instance_id); } error_t app_manager_start_for_result(const char* id, AppInstanceId parent_instance_id, int argc, const char* const argv[], AppInstanceId* out_app_instance_id) { - return start_internal(id, parent_instance_id, argc, copy_arguments(argc, argv), nullptr, 0, out_app_instance_id); + return start_internal_by_id(id, parent_instance_id, argc, copy_arguments(argc, argv), nullptr, 0, out_app_instance_id); } error_t app_manager_start_with_streams(const char* id, const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) { - return start_internal(id, 0, 0, nullptr, bindings, binding_count, out_app_instance_id); + return start_internal_by_id(id, 0, 0, nullptr, bindings, binding_count, out_app_instance_id); } error_t app_manager_start_for_result_with_streams(const char* id, AppInstanceId parent_instance_id, int argc, const char* const argv[], const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) { - return start_internal(id, parent_instance_id, argc, copy_arguments(argc, argv), bindings, binding_count, out_app_instance_id); + return start_internal_by_id(id, parent_instance_id, argc, copy_arguments(argc, argv), bindings, binding_count, out_app_instance_id); +} + +error_t app_manager_start_location(AppLocation location, AppStackConfig stack, int argc, const char* const argv[], AppInstanceId* out_app_instance_id) { + return start_internal(nullptr, location, stack, 0, argc, copy_arguments(argc, argv), nullptr, 0, out_app_instance_id); +} + +error_t app_manager_start_location_for_result(AppLocation location, AppStackConfig stack, AppInstanceId parent_instance_id, int argc, const char* const argv[], AppInstanceId* out_app_instance_id) { + return start_internal(nullptr, location, stack, parent_instance_id, argc, copy_arguments(argc, argv), nullptr, 0, out_app_instance_id); +} + +error_t app_manager_start_location_with_streams(AppLocation location, AppStackConfig stack, const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) { + return start_internal(nullptr, location, stack, 0, 0, nullptr, bindings, binding_count, out_app_instance_id); +} + +error_t app_manager_start_location_for_result_with_streams(AppLocation location, AppStackConfig stack, AppInstanceId parent_instance_id, int argc, const char* const argv[], const AppStreamBinding* bindings, size_t binding_count, AppInstanceId* out_app_instance_id) { + return start_internal(nullptr, location, stack, parent_instance_id, argc, copy_arguments(argc, argv), bindings, binding_count, out_app_instance_id); } error_t app_manager_stop(AppInstanceId app_instance_id) { @@ -230,7 +258,8 @@ error_t app_manager_get_topmost_app_id(char* buffer, size_t buffer_size) { auto& ledger = app_ledger(); mutex_lock(&ledger.mutex); auto iterator = ledger.instances.find(topmost_id); - const char* app_id = (iterator != ledger.instances.end()) ? iterator->second.manifest->id : nullptr; + const AppManifest* manifest = (iterator != ledger.instances.end()) ? iterator->second.manifest : nullptr; + const char* app_id = manifest != nullptr ? manifest->id : nullptr; mutex_unlock(&ledger.mutex); if (app_id == nullptr) { diff --git a/Modules/app-module/tests/source/exec_test.cpp b/Modules/app-module/tests/source/exec_test.cpp index abd8f10d7..618ffdfc7 100644 --- a/Modules/app-module/tests/source/exec_test.cpp +++ b/Modules/app-module/tests/source/exec_test.cpp @@ -10,7 +10,9 @@ #include #include +#include #include +#include #include extern ServiceManifest app_internal_loader_service_manifest; @@ -30,6 +32,47 @@ int32_t fake_entry(int, char*[]) { return 0; } +// Creates /{bin,outside} plus a real file at outside/target.so, for the symlink-escape +// tests below. Caller removes everything via remove_symlink_fixture() once done. +struct SymlinkFixture { + std::string root; + std::string bin_dir; + std::string outside_dir; + std::string outside_target; + + bool create() { + char temp_template[] = "/tmp/exec-test-symlink-XXXXXX"; + char* temp_dir = mkdtemp(temp_template); + if (temp_dir == nullptr) { + return false; + } + root = temp_dir; + bin_dir = root + "/bin"; + outside_dir = root + "/outside"; + outside_target = outside_dir + "/target.so"; + + if (mkdir(bin_dir.c_str(), 0755) != 0 || mkdir(outside_dir.c_str(), 0755) != 0) { + return false; + } + FILE* file = fopen(outside_target.c_str(), "w"); + if (file == nullptr) { + return false; + } + fclose(file); + return true; + } +}; + +void remove_symlink_fixture(const SymlinkFixture& fixture) { + unlink((fixture.bin_dir + "/escape.so").c_str()); + unlink((fixture.bin_dir + "/inside.so").c_str()); + unlink((fixture.bin_dir + "/real.so").c_str()); + unlink(fixture.outside_target.c_str()); + rmdir(fixture.bin_dir.c_str()); + rmdir(fixture.outside_dir.c_str()); + rmdir(fixture.root.c_str()); +} + } // namespace TEST_CASE("app_exec_path_add/app_exec_path_allowed accept nested children of a registered directory") { @@ -104,6 +147,44 @@ TEST_CASE("app_exec_path_allowed resolves a relative incoming path to absolute, app_exec_path_remove(absolute_dir.c_str()); } +TEST_CASE("app_exec_path_allowed rejects a symlink inside a registered directory that resolves outside it") { + SymlinkFixture fixture; + REQUIRE(fixture.create()); + + std::string escape_link = fixture.bin_dir + "/escape.so"; + REQUIRE_EQ(symlink(fixture.outside_target.c_str(), escape_link.c_str()), 0); + + REQUIRE_EQ(app_exec_path_add(fixture.bin_dir.c_str()), ERROR_NONE); + + // Lexically "escape.so" is inside bin_dir, but it physically resolves to outside_dir. Must + // be rejected, or the posix loader would dlopen() a binary the caller never allowed. + CHECK_FALSE(app_exec_path_allowed(escape_link.c_str())); + + app_exec_path_remove(fixture.bin_dir.c_str()); + remove_symlink_fixture(fixture); +} + +TEST_CASE("app_exec_path_allowed accepts a real file, and a symlink that stays inside, a registered directory") { + SymlinkFixture fixture; + REQUIRE(fixture.create()); + + std::string real_file = fixture.bin_dir + "/real.so"; + FILE* file = fopen(real_file.c_str(), "w"); + REQUIRE(file != nullptr); + fclose(file); + + std::string inside_link = fixture.bin_dir + "/inside.so"; + REQUIRE_EQ(symlink(real_file.c_str(), inside_link.c_str()), 0); + + REQUIRE_EQ(app_exec_path_add(fixture.bin_dir.c_str()), ERROR_NONE); + + CHECK(app_exec_path_allowed(real_file.c_str())); + CHECK(app_exec_path_allowed(inside_link.c_str())); + + app_exec_path_remove(fixture.bin_dir.c_str()); + remove_symlink_fixture(fixture); +} + TEST_CASE("app_exec_path_add returns ERROR_OUT_OF_MEMORY once the registry is full") { char path[32]; int added = 0; diff --git a/Modules/app-module/tests/source/manager_test.cpp b/Modules/app-module/tests/source/manager_test.cpp index 9dc8fca11..5dfd8a8dd 100644 --- a/Modules/app-module/tests/source/manager_test.cpp +++ b/Modules/app-module/tests/source/manager_test.cpp @@ -1,9 +1,11 @@ #include "doctest.h" #include +#include #include #include #include +#include #include @@ -173,6 +175,22 @@ bool wait_for_arguments_stashed(uint32_t timeout_ms) { return arguments_stashed.load(std::memory_order_acquire); } +// Writes a fixed string to its own stdout then returns 7 as its result, for the +// app_manager_start_location_with_streams()/_for_result_with_streams() tests: proves a binding +// installed before the task starts actually reaches the app's own app_io_write() calls. +int32_t stream_writer_app_main(int, char*[]) { + const char message[] = "loc"; + size_t sent = 0; + while (sent < sizeof(message) - 1) { + ssize_t written = app_io_write(STDOUT_FILENO, message + sent, sizeof(message) - 1 - sent); + if (written < 0) { + break; + } + sent += static_cast(written); + } + return 7; +} + } // namespace TEST_CASE("app_manager_start activates an app instance, app_manager_stop terminates it") { @@ -507,3 +525,162 @@ TEST_CASE("app_manager_get_topmost_app_id returns BUFFER_OVERFLOW for a too-smal app_manager_stop(id); app_manager_remove("test.app.top_overflow"); } + +TEST_CASE("app_manager_start_location runs a location with no manifest at all, and reports no topmost app id for it") { + ensure_fake_loader_registered(); + + AppLocation location { APP_LOCATION_PATH, nullptr }; + uint32_t instance_id = 0; + REQUIRE_EQ(app_manager_start_location(location, AppStackConfig {}, 0, nullptr, &instance_id), ERROR_NONE); + CHECK(wait_for_state(instance_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + CHECK_EQ(topmost_instance_id(), instance_id); + char buffer[64]; + // No manifest to report an id from - same NOT_FOUND a caller already sees for "nothing active". + CHECK_EQ(app_manager_get_topmost_app_id(buffer, sizeof(buffer)), ERROR_NOT_FOUND); + + CHECK_EQ(app_manager_stop(instance_id), ERROR_NONE); + CHECK_EQ(app_manager_get_state(instance_id), APP_INSTANCE_STATE_STOPPED); +} + +TEST_CASE("app_manager_start_location doesn't disturb app_manager_get_topmost_app_id for a normal manifest-backed app started afterward") { + ensure_fake_loader_registered(); + + AppLocation location { APP_LOCATION_PATH, nullptr }; + uint32_t unregistered_id = 0; + REQUIRE_EQ(app_manager_start_location(location, AppStackConfig {}, 0, nullptr, &unregistered_id), ERROR_NONE); + CHECK(wait_for_state(unregistered_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + AppManifest manifest { "test.app.location.after", "After", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&manifest), ERROR_NONE); + uint32_t registered_id = 0; + REQUIRE_EQ(app_manager_start("test.app.location.after", ®istered_id), ERROR_NONE); + CHECK(wait_for_state(registered_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + char buffer[64]; + CHECK_EQ(app_manager_get_topmost_app_id(buffer, sizeof(buffer)), ERROR_NONE); + CHECK_EQ(std::string(buffer), "test.app.location.after"); + + app_manager_stop(unregistered_id); + app_manager_stop(registered_id); + app_manager_remove("test.app.location.after"); +} + +TEST_CASE("app_manager_start_location_for_result delivers APP_EVENT_RESULT to the parent, with no manifest for the child either") { + ensure_fake_loader_registered(); + + AppManifest parent_manifest { "test.app.location.parent", "Parent", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&parent_manifest), ERROR_NONE); + + uint32_t parent_id = 0; + REQUIRE_EQ(app_manager_start("test.app.location.parent", &parent_id), ERROR_NONE); + CHECK(wait_for_state(parent_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + TaskEventGroup parent_event_group {}; + task_event_group_construct(&parent_event_group); + + AppEventSubscription parent_sub {}; + REQUIRE_EQ(app_event_subscribe_with_app_id(&parent_sub, &parent_event_group, parent_id), ERROR_NONE); + + AppLocation location { APP_LOCATION_PATH, nullptr }; + const char* argv[] = { "42" }; // fake_run's single-arg shortcut - returns 42 immediately + uint32_t child_id = 0; + REQUIRE_EQ(app_manager_start_location_for_result(location, AppStackConfig {}, parent_id, 1, argv, &child_id), ERROR_NONE); + + REQUIRE_EQ(task_event_group_wait(&parent_event_group, parent_sub.bit, false, nullptr, pdMS_TO_TICKS(2000)), ERROR_NONE); + AppEvent event {}; + REQUIRE_EQ(app_event_poll(&parent_sub, &event), ERROR_NONE); + CHECK_EQ(event.type, APP_EVENT_RESULT); + CHECK_EQ(event.result.launch_id, child_id); + CHECK_EQ(event.result.result, 42); + + app_event_unsubscribe(&parent_sub); + task_event_group_destruct(&parent_event_group); + app_manager_stop(child_id); + app_manager_stop(parent_id); + app_manager_remove("test.app.location.parent"); +} + +TEST_CASE("app_manager_start_location_with_streams pipes a manifest-less child's app_io_write() calls into a parent-owned AppStream") { + ensure_memory_loader_registered(); + + AppLocation location { APP_LOCATION_MEMORY, reinterpret_cast(stream_writer_app_main) }; + + TaskEventGroup event_group {}; + task_event_group_construct(&event_group); + + uint8_t storage[64]; + AppStream child_stdout {}; + AppStreamBinding binding { STDOUT_FILENO, &child_stdout, storage, sizeof(storage), &event_group }; + + AppInstanceId child_id = 0; + REQUIRE_EQ(app_manager_start_location_with_streams(location, AppStackConfig {}, &binding, 1, &child_id), ERROR_NONE); + + std::vector received; + while (app_stream_await(&child_stdout, APP_FILE_WAIT_READABLE, pdMS_TO_TICKS(1000)) == ERROR_NONE) { + uint8_t chunk[16]; + size_t n = app_stream_read(&child_stdout, chunk, sizeof(chunk)); + if (n == 0) { + break; // EOF + } + received.insert(received.end(), chunk, chunk + n); + } + + REQUIRE_EQ(received.size(), 3u); + CHECK_EQ(std::memcmp(received.data(), "loc", 3), 0); + + REQUIRE(wait_for_state(child_id, APP_INSTANCE_STATE_STOPPED, 1000)); + app_stream_unsubscribe(&child_stdout); + task_event_group_destruct(&event_group); +} + +TEST_CASE("app_manager_start_location_for_result_with_streams delivers both the stream data and the APP_EVENT_RESULT") { + ensure_fake_loader_registered(); + ensure_memory_loader_registered(); + + AppManifest parent_manifest { "test.app.location.parent_streams", "Parent", APP_CATEGORY_USER, { APP_LOCATION_PATH, nullptr } }; + REQUIRE_EQ(app_manager_add(&parent_manifest), ERROR_NONE); + + uint32_t parent_id = 0; + REQUIRE_EQ(app_manager_start("test.app.location.parent_streams", &parent_id), ERROR_NONE); + CHECK(wait_for_state(parent_id, APP_INSTANCE_STATE_ACTIVE, 1000)); + + TaskEventGroup parent_event_group {}; + task_event_group_construct(&parent_event_group); + AppEventSubscription parent_sub {}; + REQUIRE_EQ(app_event_subscribe_with_app_id(&parent_sub, &parent_event_group, parent_id), ERROR_NONE); + + uint8_t storage[64]; + AppStream child_stdout {}; + AppStreamBinding binding { STDOUT_FILENO, &child_stdout, storage, sizeof(storage), &parent_event_group }; + + AppLocation location { APP_LOCATION_MEMORY, reinterpret_cast(stream_writer_app_main) }; + uint32_t child_id = 0; + REQUIRE_EQ(app_manager_start_location_for_result_with_streams(location, AppStackConfig {}, parent_id, 0, nullptr, &binding, 1, &child_id), ERROR_NONE); + + std::vector received; + while (app_stream_await(&child_stdout, APP_FILE_WAIT_READABLE, pdMS_TO_TICKS(1000)) == ERROR_NONE) { + uint8_t chunk[16]; + size_t n = app_stream_read(&child_stdout, chunk, sizeof(chunk)); + if (n == 0) { + break; // EOF + } + received.insert(received.end(), chunk, chunk + n); + } + REQUIRE_EQ(received.size(), 3u); + CHECK_EQ(std::memcmp(received.data(), "loc", 3), 0); + + REQUIRE_EQ(task_event_group_wait(&parent_event_group, parent_sub.bit, false, nullptr, pdMS_TO_TICKS(2000)), ERROR_NONE); + AppEvent event {}; + REQUIRE_EQ(app_event_poll(&parent_sub, &event), ERROR_NONE); + CHECK_EQ(event.type, APP_EVENT_RESULT); + CHECK_EQ(event.result.launch_id, child_id); + CHECK_EQ(event.result.result, 7); + + app_stream_unsubscribe(&child_stdout); + app_event_unsubscribe(&parent_sub); + task_event_group_destruct(&parent_event_group); + app_manager_stop(child_id); + app_manager_stop(parent_id); + app_manager_remove("test.app.location.parent_streams"); +} diff --git a/Tactility/Source/app/files/View.cpp b/Tactility/Source/app/files/View.cpp index 4cfdf5126..1c23e78cf 100644 --- a/Tactility/Source/app/files/View.cpp +++ b/Tactility/Source/app/files/View.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include #include @@ -209,8 +210,16 @@ void View::viewFile(const std::string& path, const std::string& filename) { void View::runFile(const std::string& file_path) { LOG_I(TAG, "Running %s", file_path.c_str()); + + if (!app_exec_is_executable_path(file_path.c_str())) { + LOG_W(TAG, "Not executable: %s", file_path.c_str()); + alertdialog::start(appInstanceId, "Run failed", "Could not run \"" + file::getLastPathSegment(file_path) + "\"."); + return; + } + + AppLocation location { APP_LOCATION_PATH, const_cast(file_path.c_str()) }; AppInstanceId instance_id = 0; - if (app_exec_run_path(file_path.c_str(), 0, nullptr, &instance_id) != ERROR_NONE) { + if (app_manager_start_location(location, AppStackConfig {}, 0, nullptr, &instance_id) != ERROR_NONE) { LOG_W(TAG, "Failed to run %s", file_path.c_str()); alertdialog::start(appInstanceId, "Run failed", "Could not run \"" + file::getLastPathSegment(file_path) + "\"."); }