From 1145e94e4879e290b7b0fd5774c28d7ef173375f Mon Sep 17 00:00:00 2001 From: Ben Dodson Date: Tue, 25 Aug 2026 12:26:01 -0700 Subject: [PATCH 1/2] fix(debugger): address review feedback --- npm_modules/cli/src/debugger/server.spec.ts | 20 +++++++++++ npm_modules/cli/src/debugger/server.ts | 24 +++++++++++--- .../test/IRenderedVirtualNodeData.spec.ts | 33 +++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/npm_modules/cli/src/debugger/server.spec.ts b/npm_modules/cli/src/debugger/server.spec.ts index ba7a153a..0582e76b 100644 --- a/npm_modules/cli/src/debugger/server.spec.ts +++ b/npm_modules/cli/src/debugger/server.spec.ts @@ -524,6 +524,26 @@ describe('debugger server', () => { expect(responseBody.logs.map(log => log.message)).toEqual(['exact application']); }); + it('does not expose absolute log paths in filesystem error responses', async () => { + const logsDirectory = path.join(assetRoot, 'missing', 'logs'); + const consoleWarn = spyOn(console, 'warn'); + debuggerServer = await startDebuggerServer({ + assetRoot, + host: '127.0.0.1', + logsDirectory, + port: await getFreePort(), + strictPort: true, + }); + + const result = await request(new URL('/api/runtime-logs', debuggerServer.url).toString(), GET_REQUEST_OPTIONS); + const responseBody = JSON.parse(result.body) as { error: string }; + + expect(result.statusCode).toBe(500); + expect(responseBody.error).toBe('A local filesystem operation failed. See the debugger server output for details.'); + expect(result.body).not.toContain(logsDirectory); + expect(consoleWarn).toHaveBeenCalledWith(jasmine.stringContaining(logsDirectory)); + }); + it('reads the logs directory through the shared YAML config parser', async () => { const testHome = path.join(assetRoot, 'home'); const logsDirectory = path.join(testHome, 'logs#debug'); diff --git a/npm_modules/cli/src/debugger/server.ts b/npm_modules/cli/src/debugger/server.ts index b02c1b5a..1e948e22 100644 --- a/npm_modules/cli/src/debugger/server.ts +++ b/npm_modules/cli/src/debugger/server.ts @@ -254,6 +254,22 @@ function errorPayload(error: unknown): { error: string } { }; } +function isFileSystemError(error: unknown): error is NodeJS.ErrnoException { + if (!(error instanceof Error)) return false; + const fileSystemError = error as NodeJS.ErrnoException; + return ( + typeof fileSystemError.code === 'string' && + typeof fileSystemError.path === 'string' && + typeof fileSystemError.syscall === 'string' + ); +} + +function clientErrorPayload(error: unknown): { error: string } { + if (!isFileSystemError(error)) return errorPayload(error); + console.warn(`Debugger filesystem error: ${errorPayload(error).error}`); + return { error: 'A local filesystem operation failed. See the debugger server output for details.' }; +} + function isValidSnapshotBase64(value: string): boolean { if (!value || value.length % 4 === 1) return false; const firstPaddingIndex = value.indexOf('='); @@ -841,7 +857,7 @@ async function streamRuntimeLogs( if (nextLogs.length > 0) sendSse(response, 'logs', { logs: nextLogs }); } catch (error) { - sendSse(response, 'stream-error', errorPayload(error)); + sendSse(response, 'stream-error', clientErrorPayload(error)); } } @@ -899,7 +915,7 @@ async function collectClientContexts( try { contexts = await conn.listContexts(client.client_id); } catch (error) { - contextError = errorPayload(error).error; + contextError = clientErrorPayload(error).error; } } @@ -938,7 +954,7 @@ async function inspectPort(port: number): Promise<{ portName: portName(port), connected: false, clients: [], - error: errorPayload(error).error, + error: clientErrorPayload(error).error, }; } } @@ -1375,7 +1391,7 @@ async function handleApi(request: IncomingMessage, response: ServerResponse, url sendJson(response, 404, { error: `Unknown API route ${url.pathname}` }); } catch (error) { - sendJson(response, error instanceof ApiRequestError ? error.statusCode : 500, errorPayload(error)); + sendJson(response, error instanceof ApiRequestError ? error.statusCode : 500, clientErrorPayload(error)); } } diff --git a/src/valdi_modules/src/valdi/valdi_core/test/IRenderedVirtualNodeData.spec.ts b/src/valdi_modules/src/valdi/valdi_core/test/IRenderedVirtualNodeData.spec.ts index 6419fdba..5c5a0cf6 100644 --- a/src/valdi_modules/src/valdi/valdi_core/test/IRenderedVirtualNodeData.spec.ts +++ b/src/valdi_modules/src/valdi/valdi_core/test/IRenderedVirtualNodeData.spec.ts @@ -190,6 +190,39 @@ describe('IRenderedVirtualNodeData', () => { expect(data.component?.viewModel).toContain('Set('); }); + it('replaces values at the maximum component debug depth with an omission marker', () => { + const data = createDetailedData( + createComponentNode( + { + level1: { level2: { level3: { level4: 'too-deep' } } }, + sibling: 'visible', + }, + {}, + ), + ); + + expect(data.component?.viewModel).toContain('level4: ...'); + expect(data.component?.viewModel).not.toContain('too-deep'); + expect(data.component?.viewModel).toContain('sibling: "visible"'); + }); + + it('limits arrays and objects to 50 serialized items with omission markers', () => { + const items = Array.from({ length: 52 }, (_value, index) => `item-${index}`); + const properties: Record = {}; + for (let index = 0; index < 52; index++) { + properties[`property${index}`] = index; + } + + const data = createDetailedData(createComponentNode({ items, properties }, {})); + + expect(data.component?.viewModel).toContain('"item-49"'); + expect(data.component?.viewModel).not.toContain('"item-50"'); + expect(data.component?.viewModel).toContain('... 2 more item(s) ...'); + expect(data.component?.viewModel).toContain('property49: 49'); + expect(data.component?.viewModel).not.toContain('property50: 50'); + expect(data.component?.viewModel).toContain('... more properties ...'); + }); + it('truncates one serialized component field to its character cap', () => { const data = createDetailedData(createComponentNode('x'.repeat(70_000), {})); From 10af426aee32c56efc476c4714c089b4d0c9944e Mon Sep 17 00:00:00 2001 From: Ben Dodson Date: Tue, 25 Aug 2026 23:29:03 -0700 Subject: [PATCH 2/2] feat(debugger): support explicit debugger ports --- valdi/BUILD.bazel | 1 + .../valdi/support/AppBootstrapActivity.kt | 48 ++++++ .../snapchat/client/valdi/NativeBridge.java | 1 + valdi/src/valdi/android/NativeBridge.cpp | 9 + valdi/src/valdi/android/NativeBridge.hpp | 1 + .../valdi/android/RuntimeManagerWrapper.cpp | 3 +- .../SCValdiBootstrappingAppDelegate.m | 4 +- valdi/src/valdi/ios/SCValdiRuntimeManager.mm | 22 ++- valdi/src/valdi/macos/SCValdiRuntime.mm | 3 +- .../runtime/Debugger/DebuggerService.cpp | 48 ++++++ .../runtime/Debugger/DebuggerService.hpp | 15 ++ valdi/src/valdi/runtime/RuntimeManager.cpp | 60 ++++++- valdi/src/valdi/runtime/RuntimeManager.hpp | 14 ++ .../ValdiStandaloneRuntime.cpp | 3 +- .../valdi/swift/SwiftValdiRuntimeManager.mm | 6 +- valdi/test/ios/SCValdiRuntimeTests.mm | 133 +++++++++++++++ .../java/support/AppBootstrapActivityTest.kt | 161 ++++++++++++++++++ valdi/test/runtime/DebuggerService_tests.cpp | 143 ++++++++++++++++ valdi/test/runtime/RuntimeManager_tests.cpp | 85 +++++++++ .../ios/valdi_core/SCValdiConfiguration.h | 25 +++ .../ios/valdi_core/SCValdiConfiguration.m | 9 + .../SCValdiRuntimeManagerProtocol.h | 2 + 22 files changed, 785 insertions(+), 11 deletions(-) create mode 100644 valdi/test/java/support/AppBootstrapActivityTest.kt create mode 100644 valdi/test/runtime/RuntimeManager_tests.cpp diff --git a/valdi/BUILD.bazel b/valdi/BUILD.bazel index c3136c30..2e89e1b0 100644 --- a/valdi/BUILD.bazel +++ b/valdi/BUILD.bazel @@ -598,6 +598,7 @@ kt_jvm_test( "@android_mvn//:org_junit_vintage_junit_vintage_engine", ], deps = [ + ":valdi_android_support", ":valdi_android_test_support", ":valdi_java", "//src/valdi_modules/src/valdi/valdi_test:valdi_test_kt", diff --git a/valdi/src/android_support/java/com/snap/valdi/support/AppBootstrapActivity.kt b/valdi/src/android_support/java/com/snap/valdi/support/AppBootstrapActivity.kt index 01867097..08e216e9 100644 --- a/valdi/src/android_support/java/com/snap/valdi/support/AppBootstrapActivity.kt +++ b/valdi/src/android_support/java/com/snap/valdi/support/AppBootstrapActivity.kt @@ -1,6 +1,9 @@ package com.snap.valdi.support +import android.content.Intent +import android.content.pm.ApplicationInfo import android.os.Bundle +import android.util.Log import android.view.View import androidx.appcompat.app.AppCompatActivity @@ -12,6 +15,12 @@ import com.snap.valdi.ValdiRuntimeManager import com.snap.valdi.ValdiRuntime import com.snap.valdi.utils.Disposable import com.snap.valdi.support.DefaultNavigator +import com.snapchat.client.valdi.NativeBridge + +/** Intent extra used to select the Valdi debugger port for a debuggable application. */ +const val VALDI_DEBUGGER_PORT_INTENT_EXTRA = "com.snap.valdi.DEBUGGER_PORT" + +private const val VALDI_LOG_TAG = "Valdi" /** This class implements an Android activity where the root view @@ -53,8 +62,29 @@ abstract class AppBootstrapActivity: AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + val debuggerPort = requestedValdiDebuggerPort( + intent, + applicationInfo.flags and ApplicationInfo.FLAG_DEBUGGABLE != 0, + ) + loadNativeLibrary() + if (debuggerPort != null) { + // This process-wide override intentionally persists for this debug host process. When the intent + // omits the extra, this block is skipped so externally configured environment state remains unchanged. + setDebuggerPortEnvironment(debuggerPort) + } + + bootstrapValdiRuntime() + } + + protected open fun loadNativeLibrary() { System.loadLibrary(getNativeLibName()) + } + protected open fun setDebuggerPortEnvironment(debuggerPort: Int) { + NativeBridge.setDebuggerPortEnvironment(debuggerPort) + } + + protected open fun bootstrapValdiRuntime() { createRuntimeManager() this.rootView = createAppRootView() setContentView(this.rootView) @@ -106,3 +136,21 @@ abstract class AppBootstrapActivity: AppCompatActivity() { } } } + +internal fun requestedValdiDebuggerPort(intent: Intent?, debuggable: Boolean): Int? { + if (!debuggable || intent == null || !intent.hasExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA)) { + return null + } + + val port = intent.extras?.get(VALDI_DEBUGGER_PORT_INTENT_EXTRA) as? Int + if (port == null || port !in 1..65535) { + Log.w( + VALDI_LOG_TAG, + "Ignoring invalid Valdi debugger port from intent extra " + + "$VALDI_DEBUGGER_PORT_INTENT_EXTRA: (expected an integer in 1...65535)", + ) + return null + } + + return port +} diff --git a/valdi/src/java/com/snapchat/client/valdi/NativeBridge.java b/valdi/src/java/com/snapchat/client/valdi/NativeBridge.java index f0f692fe..487b9391 100644 --- a/valdi/src/java/com/snapchat/client/valdi/NativeBridge.java +++ b/valdi/src/java/com/snapchat/client/valdi/NativeBridge.java @@ -4,6 +4,7 @@ public class NativeBridge { public static native int getBuildOptions(); + public static native void setDebuggerPortEnvironment(int debuggerPort); public static native long createRuntimeManager(Object mainThreadDispatcher, Object snapDrawingFrameScheduler, Object viewManager, diff --git a/valdi/src/valdi/android/NativeBridge.cpp b/valdi/src/valdi/android/NativeBridge.cpp index f9388d9d..5636f14d 100644 --- a/valdi/src/valdi/android/NativeBridge.cpp +++ b/valdi/src/valdi/android/NativeBridge.cpp @@ -62,6 +62,8 @@ #endif #include +#include +#include inline ValdiAndroid::RuntimeWrapper* getRuntimeWrapper(jlong handle) { return reinterpret_cast(handle); @@ -148,6 +150,12 @@ jint ValdiAndroid::NativeBridge::getBuildOptions(fbjni::alias_ref return static_cast(buildOptions); } +void ValdiAndroid::NativeBridge::setDebuggerPortEnvironment(fbjni::alias_ref /* clazz */, + jint debuggerPort) { + const auto debuggerPortString = std::to_string(debuggerPort); + setenv("VALDI_DEBUGGER_PORT", debuggerPortString.c_str(), 1); +} + jlong ValdiAndroid::NativeBridge::createRuntimeManager( // NOLINT fbjni::alias_ref /* clazz */, // NOLINT jobject mainThreadDispatcher, @@ -2530,6 +2538,7 @@ jlong ValdiAndroid::NativeBridge::snapDrawingGetMaxRenderTargetSize(fbjni::alias void ValdiAndroid::NativeBridge::registerNatives() { javaClassStatic()->registerNatives({ makeNativeMethod("getBuildOptions", ValdiAndroid::NativeBridge::getBuildOptions), + makeNativeMethod("setDebuggerPortEnvironment", ValdiAndroid::NativeBridge::setDebuggerPortEnvironment), makeNativeMethod("getAllRuntimeAttachedObjects", ValdiAndroid::NativeBridge::getAllRuntimeAttachedObjects), makeNativeMethod("prepareRenderBackend", ValdiAndroid::NativeBridge::prepareRenderBackend), makeNativeMethod("emitRuntimeManagerInitMetrics", ValdiAndroid::NativeBridge::emitRuntimeManagerInitMetrics), diff --git a/valdi/src/valdi/android/NativeBridge.hpp b/valdi/src/valdi/android/NativeBridge.hpp index 208b896b..18afbc51 100644 --- a/valdi/src/valdi/android/NativeBridge.hpp +++ b/valdi/src/valdi/android/NativeBridge.hpp @@ -16,6 +16,7 @@ class NativeBridge : public fbjni::JavaClass { static constexpr auto kJavaDescriptor = "Lcom/snapchat/client/valdi/NativeBridge;"; static jint getBuildOptions(fbjni::alias_ref clazz); + static void setDebuggerPortEnvironment(fbjni::alias_ref clazz, jint debuggerPort); static jobject getAllRuntimeAttachedObjects(fbjni::alias_ref clazz, jlong runtimeManagerHandle); diff --git a/valdi/src/valdi/android/RuntimeManagerWrapper.cpp b/valdi/src/valdi/android/RuntimeManagerWrapper.cpp index ba92805d..1d28de4b 100644 --- a/valdi/src/valdi/android/RuntimeManagerWrapper.cpp +++ b/valdi/src/valdi/android/RuntimeManagerWrapper.cpp @@ -104,7 +104,8 @@ RuntimeManagerWrapper::RuntimeManagerWrapper(JavaEnv env, _logger, /* enableDebuggerService */ true, /* disableHotReloader */ false, - /* isStandalone */ false); + /* isStandalone */ false, + std::nullopt); _runtimeManager->postInit(); _runtimeManager->setKeepDebuggerServiceOnPause(static_cast(keepDebuggerServiceOnPause)); _runtimeManager->setApplicationId(_applicationId); diff --git a/valdi/src/valdi/ios/Bootstrap/SCValdiBootstrappingAppDelegate.m b/valdi/src/valdi/ios/Bootstrap/SCValdiBootstrappingAppDelegate.m index c0c3135b..9182f0e6 100644 --- a/valdi/src/valdi/ios/Bootstrap/SCValdiBootstrappingAppDelegate.m +++ b/valdi/src/valdi/ios/Bootstrap/SCValdiBootstrappingAppDelegate.m @@ -14,10 +14,12 @@ @implementation SCValdiBootstrappingAppDelegate { - (BOOL)application:(UIApplication*)application didFinishLaunchingWithOptions:(NSDictionary*)launchOptions { - _runtimeManager = [[SCValdiRuntimeManager alloc] init]; + _runtimeManager = [SCValdiRuntimeManager new]; [_runtimeManager updateConfiguration:^(SCValdiConfiguration* configuration) { configuration.allowDarkMode = YES; + // Bootstrap apps are local-development hosts, so keep their previous hot-reload behavior explicit. + configuration.enableDebuggerService = YES; }]; id runtime = _runtimeManager.mainRuntime; diff --git a/valdi/src/valdi/ios/SCValdiRuntimeManager.mm b/valdi/src/valdi/ios/SCValdiRuntimeManager.mm index 5730c106..fa8f2c1f 100644 --- a/valdi/src/valdi/ios/SCValdiRuntimeManager.mm +++ b/valdi/src/valdi/ios/SCValdiRuntimeManager.mm @@ -88,6 +88,20 @@ #endif } +static std::optional SCValdiResolveDebuggerServicePort(SCValdiConfiguration* configuration) { + NSInteger debuggerServicePort = configuration.debuggerServicePort; + if (debuggerServicePort == 0) { + return std::nullopt; + } + + if (debuggerServicePort < 0 || debuggerServicePort > 65535) { + SCLogValdiWarning(@"Ignoring invalid Valdi debugger service port: (expected 1...65535)"); + return std::nullopt; + } + + return static_cast(debuggerServicePort); +} + static void updateRuntimeManagersArray(void (^callback)(NSMutableArray *runtimeManagers)) { static dispatch_once_t onceToken; static NSMutableArray *kAllRuntimeManagers; @@ -204,6 +218,7 @@ - (void)_initializeIfNeeded _diskCache = Valdi::makeShared(resolveDocumentsDirectory()); id keychainStore = SCValdiCreateKeychainStore(); + SCValdiConfiguration *configuration = [self _getOrCreateConfiguration]; _cppInstance = Valdi::makeShared(mainThreadDispatcher, [self _javaScriptBridge], @@ -213,9 +228,10 @@ - (void)_initializeIfNeeded Valdi::PlatformTypeIOS, Valdi::ThreadQoSClassMax, logger, - /* enableDebuggerService */ true, - /* disableHotReloader */ false, - /* isStandalone */ false); + configuration.enableDebuggerService, + configuration.disableHotReloader, + /* isStandalone */ false, + SCValdiResolveDebuggerServicePort(configuration)); _cppInstance->postInit(); NSString *bundleIdentifier = [NSBundle mainBundle].bundleIdentifier; _cppInstance->setApplicationId(ValdiIOS::StringFromNSString(bundleIdentifier)); diff --git a/valdi/src/valdi/macos/SCValdiRuntime.mm b/valdi/src/valdi/macos/SCValdiRuntime.mm index ae219310..6d1f9f23 100644 --- a/valdi/src/valdi/macos/SCValdiRuntime.mm +++ b/valdi/src/valdi/macos/SCValdiRuntime.mm @@ -117,7 +117,8 @@ - (instancetype)initWithUsingTemporaryCachesDirectory:(BOOL)usingTemporaryCaches Valdi::strongSmallRef(&Valdi::ConsoleLogger::getLogger()), /* enableDebuggerService */ true, /* disableHotReloader */ false, - /* isStandalone */ true); + /* isStandalone */ true, + std::nullopt); _runtimeManager->postInit(); _runtimeManager->applicationDidResume(); _runtimeManager->registerBytesAssetLoader(cachesImageCache); diff --git a/valdi/src/valdi/runtime/Debugger/DebuggerService.cpp b/valdi/src/valdi/runtime/Debugger/DebuggerService.cpp index cea0e2a1..48a13ae5 100644 --- a/valdi/src/valdi/runtime/Debugger/DebuggerService.cpp +++ b/valdi/src/valdi/runtime/Debugger/DebuggerService.cpp @@ -18,6 +18,11 @@ #include "valdi_core/cpp/Utils/ContainerUtils.hpp" +#include +#include +#include +#include + namespace Valdi { class DaemonClientTCPDataSender : public DaemonClientDataSender { @@ -138,6 +143,10 @@ void DebuggerService::stop() { }); } +uint32_t DebuggerService::getConfiguredPort() const { + return _debuggerPort; +} + uint16_t DebuggerService::getBoundPort() { uint16_t boundPort = 0; _dispatchQueue->sync([&]() { @@ -316,4 +325,43 @@ uint32_t DebuggerService::resolveDebuggerPort(bool isStandalone) { return isStandalone ? kStandaloneDebuggerPort : kMobileDebuggerPort; } +DebuggerPortResolution DebuggerService::resolveDebuggerPortWithDiagnostics( + bool isStandalone, + std::optional requestedPort) { + DebuggerPortResolution resolution{ + resolveDebuggerPort(isStandalone), + std::nullopt, + std::nullopt, + }; + + if (requestedPort.has_value()) { + if (requestedPort.value() > 0 && requestedPort.value() <= 65535) { + resolution.port = requestedPort.value(); + return resolution; + } + resolution.rejectedRequestedPort = requestedPort; + } + + const char* overridePort = std::getenv("VALDI_DEBUGGER_PORT"); + if (overridePort != nullptr) { + uint32_t parsedPort = 0; + const char* overridePortEnd = overridePort + std::strlen(overridePort); + const auto parseResult = std::from_chars(overridePort, overridePortEnd, parsedPort); + if (parseResult.ec == std::errc() && parseResult.ptr == overridePortEnd && parsedPort > 0 && + parsedPort <= 65535) { + resolution.port = parsedPort; + return resolution; + } + + if (parseResult.ec == std::errc::result_out_of_range || + (parseResult.ec == std::errc() && parseResult.ptr == overridePortEnd)) { + resolution.environmentError = DebuggerPortEnvironmentError::OutOfRange; + } else { + resolution.environmentError = DebuggerPortEnvironmentError::Malformed; + } + } + + return resolution; +} + } // namespace Valdi diff --git a/valdi/src/valdi/runtime/Debugger/DebuggerService.hpp b/valdi/src/valdi/runtime/Debugger/DebuggerService.hpp index 52c757cc..198a76ef 100644 --- a/valdi/src/valdi/runtime/Debugger/DebuggerService.hpp +++ b/valdi/src/valdi/runtime/Debugger/DebuggerService.hpp @@ -18,6 +18,7 @@ #include "utils/platform/BuildOptions.hpp" +#include #include namespace snap::valdi { @@ -36,6 +37,17 @@ class Runtime; class IDebuggerServiceListener; +enum class DebuggerPortEnvironmentError { + Malformed, + OutOfRange, +}; + +struct DebuggerPortResolution { + uint32_t port; + std::optional rejectedRequestedPort; + std::optional environmentError; +}; + class DebuggerService : public SharedPtrRefCountable, protected ITCPServerListener, public DaemonClientListener, @@ -67,9 +79,12 @@ class DebuggerService : public SharedPtrRefCountable, StringBox getApplicationId() const; void setApplicationId(const StringBox& applicationId); + uint32_t getConfiguredPort() const; uint16_t getBoundPort(); static uint32_t resolveDebuggerPort(bool isStandalone); + static DebuggerPortResolution resolveDebuggerPortWithDiagnostics(bool isStandalone, + std::optional requestedPort); protected: // TCPServerListener diff --git a/valdi/src/valdi/runtime/RuntimeManager.cpp b/valdi/src/valdi/runtime/RuntimeManager.cpp index 5be9a801..3a9d719c 100644 --- a/valdi/src/valdi/runtime/RuntimeManager.cpp +++ b/valdi/src/valdi/runtime/RuntimeManager.cpp @@ -43,6 +43,7 @@ namespace Valdi { Shared createDebuggerService(bool enableDebuggerService, bool disableHotReloader, bool isStandalone, + std::optional debuggerPort, PlatformType platformType, const Shared& runtimeMessageHandler, const Ref& logger) { @@ -65,10 +66,25 @@ Shared createDebuggerService(bool enableDebuggerService, platform = snap::valdi_core::Platform::Ios; break; } - auto debuggerPort = DebuggerService::resolveDebuggerPort(isStandalone); + auto debuggerPortResolution = + DebuggerService::resolveDebuggerPortWithDiagnostics(isStandalone, debuggerPort); + if (debuggerPortResolution.rejectedRequestedPort.has_value()) { + VALDI_WARN(*logger, + "Ignoring invalid debugger port from requestedPort: (expected 1...65535); " + "trying VALDI_DEBUGGER_PORT, then the platform default."); + } + if (debuggerPortResolution.environmentError.has_value()) { + VALDI_WARN(*logger, + "Ignoring invalid debugger port from VALDI_DEBUGGER_PORT: (expected " + "1...65535); using platform default {}.", + debuggerPortResolution.environmentError.value() == DebuggerPortEnvironmentError::OutOfRange + ? "out-of-range" + : "malformed", + debuggerPortResolution.port); + } auto debuggerService = Valdi::makeShared( - runtimeMessageHandler, platform, debuggerPort, disableHotReloader, logger); + runtimeMessageHandler, platform, debuggerPortResolution.port, disableHotReloader, logger); debuggerService->postInit(); return debuggerService.toShared(); } else { @@ -117,10 +133,41 @@ RuntimeManager::RuntimeManager(const Ref& mainThreadDispa bool enableDebuggerService, bool disableHotReloader, bool isStandalone) + : RuntimeManager(mainThreadDispatcher, + jsBridge, + diskCache, + std::move(keychain), + runtimeMessageHandler, + platformType, + jsThreadQoS, + logger, + enableDebuggerService, + disableHotReloader, + isStandalone, + std::nullopt) {} + +RuntimeManager::RuntimeManager(const Ref& mainThreadDispatcher, + IJavaScriptBridge* jsBridge, + const Ref& diskCache, + Shared keychain, + const Shared& runtimeMessageHandler, + PlatformType platformType, + ThreadQoSClass jsThreadQoS, + const Ref& logger, + bool enableDebuggerService, + bool disableHotReloader, + bool isStandalone, + std::optional debuggerPort) : _initStopWatch(std::make_shared()), _yogaConfig(Valdi::Yoga::createConfig(0)), _debuggerService(createDebuggerService( - enableDebuggerService, disableHotReloader, isStandalone, platformType, runtimeMessageHandler, logger)), + enableDebuggerService, + disableHotReloader, + isStandalone, + debuggerPort, + platformType, + runtimeMessageHandler, + logger)), _deferredGCTask(DispatchQueue::TaskIDNull), _mainThreadManager(makeShared(mainThreadDispatcher)), _assetLoaderManager(makeShared()), @@ -457,6 +504,13 @@ bool RuntimeManager::debuggerServiceEnabled() const { return _debuggerService != nullptr; } +std::optional RuntimeManager::getDebuggerServicePort() const { + if (_debuggerService == nullptr) { + return std::nullopt; + } + return _debuggerService->getConfiguredPort(); +} + void RuntimeManager::setUserSession(const StringBox& userId) { if (userId.isEmpty()) { _userSession.set(nullptr); diff --git a/valdi/src/valdi/runtime/RuntimeManager.hpp b/valdi/src/valdi/runtime/RuntimeManager.hpp index 2c376fe2..88f1f850 100644 --- a/valdi/src/valdi/runtime/RuntimeManager.hpp +++ b/valdi/src/valdi/runtime/RuntimeManager.hpp @@ -19,6 +19,7 @@ #include "valdi_core/cpp/Utils/Mutex.hpp" #include "valdi_core/cpp/Utils/Shared.hpp" #include +#include #include struct YGConfig; @@ -74,6 +75,18 @@ class RuntimeManager : public ValdiObject, protected ColorPaletteManagerListener bool enableDebuggerService, bool disableHotReloader, bool isStandalone); + RuntimeManager(const Ref& mainThreadDispatcher, + IJavaScriptBridge* jsBridge, + const Ref& diskCache, + Shared keychain, + const Shared& runtimeMessageHandler, + PlatformType platformType, + ThreadQoSClass jsThreadQoS, + const Ref& logger, + bool enableDebuggerService, + bool disableHotReloader, + bool isStandalone, + std::optional debuggerPort); ~RuntimeManager() override; void postInit(); @@ -112,6 +125,7 @@ class RuntimeManager : public ValdiObject, protected ColorPaletteManagerListener void applicationWillTerminate(); bool debuggerServiceEnabled() const; + std::optional getDebuggerServicePort() const; void setUserSession(const StringBox& userId); void setApplicationId(const StringBox& applicationId); diff --git a/valdi/src/valdi/standalone_runtime/ValdiStandaloneRuntime.cpp b/valdi/src/valdi/standalone_runtime/ValdiStandaloneRuntime.cpp index cc948a81..b926bc3b 100644 --- a/valdi/src/valdi/standalone_runtime/ValdiStandaloneRuntime.cpp +++ b/valdi/src/valdi/standalone_runtime/ValdiStandaloneRuntime.cpp @@ -403,7 +403,8 @@ Ref ValdiStandaloneRuntime::create(bool enableDebuggerSe Valdi::strongSmallRef(&ConsoleLogger::getLogger()), enableDebuggerService, disableHotReloader, - /* isStandalone */ true); + /* isStandalone */ true, + std::nullopt); runtimeManager->postInit(); runtimeManager->registerBytesAssetLoader(diskCache); auto shouldWaitForHotReload = enableDebuggerService && !disableHotReloader; diff --git a/valdi/src/valdi/swift/SwiftValdiRuntimeManager.mm b/valdi/src/valdi/swift/SwiftValdiRuntimeManager.mm index 7206eddb..92dd5c22 100644 --- a/valdi/src/valdi/swift/SwiftValdiRuntimeManager.mm +++ b/valdi/src/valdi/swift/SwiftValdiRuntimeManager.mm @@ -10,7 +10,11 @@ @implementation SwiftValdiRuntimeManager - (id)createRuntimeManager { if (!self.runtimeManager) { - self.runtimeManager = [[SCValdiRuntimeManager alloc] init]; + self.runtimeManager = [SCValdiRuntimeManager new]; + [self.runtimeManager updateConfiguration:^(SCValdiConfiguration* configuration) { + // Preserve Snap's default-on development-host behavior; direct hosts can opt out through configuration. + configuration.enableDebuggerService = YES; + }]; } return self.runtimeManager; } diff --git a/valdi/test/ios/SCValdiRuntimeTests.mm b/valdi/test/ios/SCValdiRuntimeTests.mm index 40db97a1..69342ebe 100644 --- a/valdi/test/ios/SCValdiRuntimeTests.mm +++ b/valdi/test/ios/SCValdiRuntimeTests.mm @@ -18,11 +18,14 @@ #import "valdi/ios/Text/SCValdiCustomUnderlineStyle.h" #import "valdi/ios/Gestures/SCValdiGestureRecognizers.h" #import "valdi/ios/Utils/SCValdiImageFilter.h" +#import "valdi/runtime/Debugger/DebuggerService.hpp" +#import "valdi/runtime/RuntimeManager.hpp" #import "valdi/runtime/Utils/AsyncGroup.hpp" #import "valdi_core/cpp/Threading/DispatchQueue.hpp" #import "valdi_core/cpp/Threading/GCDDispatchQueue.hpp" #import "valdi_core/SCValdiScrollView.h" #import "valdi_core/SCValdiRootView.h" +#import "valdi_core/SCValdiSharedLogger.h" #import "valdi_core/UIView+ValdiBase.h" #import @@ -54,6 +57,56 @@ - (void)onRender @end +@interface SCValdiCapturingLogger: NSObject + +- (void)reset; +- (NSArray *)capturedMessages; + +@end + +@implementation SCValdiCapturingLogger { + NSMutableArray *_messages; +} + +- (instancetype)init +{ + self = [super init]; + if (self) { + _messages = [NSMutableArray array]; + } + return self; +} + +- (BOOL)isLogEnabledForLevel:(SCValdiLoggerLevel)level +{ + (void)level; + return YES; +} + +- (void)outputLog:(NSString *)log forLevel:(SCValdiLoggerLevel)level +{ + (void)level; + @synchronized (self) { + [_messages addObject:log]; + } +} + +- (void)reset +{ + @synchronized (self) { + [_messages removeAllObjects]; + } +} + +- (NSArray *)capturedMessages +{ + @synchronized (self) { + return [_messages copy]; + } +} + +@end + @interface SCValdiTextField (SCValdiRuntimeTests) - (void)valdi_setFontAttributes:(SCValdiFontAttributes *)fontAttributes; @@ -106,6 +159,86 @@ - (void)tearDown self.runtimeManager = nil; } +- (Valdi::RuntimeManager *)_cppRuntimeManagerForRuntimeManager:(SCValdiRuntimeManager *)runtimeManager +{ + (void)runtimeManager.mainRuntime; + return static_cast(runtimeManager.cppInstance); +} + +- (void)testDirectRuntimeManagerPreservesDebuggerServiceDefault +{ + Valdi::RuntimeManager *runtimeManagerCpp = [self _cppRuntimeManagerForRuntimeManager:self.runtimeManager]; + + XCTAssertNotEqual(nullptr, runtimeManagerCpp); + XCTAssertEqual(Valdi::kDebuggerServiceEnabled, runtimeManagerCpp->debuggerServiceEnabled()); +} + +- (void)testDirectRuntimeManagerCanExplicitlyDisableDebuggerService +{ + SCValdiRuntimeManager *runtimeManager = [SCValdiRuntimeManager new]; + [runtimeManager updateConfiguration:^(SCValdiConfiguration *configuration) { + configuration.enableDebuggerService = NO; + }]; + + Valdi::RuntimeManager *runtimeManagerCpp = [self _cppRuntimeManagerForRuntimeManager:runtimeManager]; + + XCTAssertNotEqual(nullptr, runtimeManagerCpp); + XCTAssertFalse(runtimeManagerCpp->debuggerServiceEnabled()); +} + +- (void)testDirectRuntimeManagerAcceptsExplicitDebuggerServicePort +{ + SCValdiRuntimeManager *runtimeManager = [SCValdiRuntimeManager new]; + [runtimeManager updateConfiguration:^(SCValdiConfiguration *configuration) { + configuration.debuggerServicePort = 13702; + }]; + + Valdi::RuntimeManager *runtimeManagerCpp = [self _cppRuntimeManagerForRuntimeManager:runtimeManager]; + + XCTAssertNotEqual(nullptr, runtimeManagerCpp); + XCTAssertEqual(Valdi::kDebuggerServiceEnabled, runtimeManagerCpp->debuggerServiceEnabled()); + std::optional configuredPort = runtimeManagerCpp->getDebuggerServicePort(); + if (Valdi::kDebuggerServiceEnabled) { + XCTAssertTrue(configuredPort.has_value()); + XCTAssertEqual((uint32_t)13702, configuredPort.value_or(0)); + } else { + XCTAssertFalse(configuredPort.has_value()); + } +} + +- (void)testInvalidExplicitDebuggerServicePortWarningIsValueRedacted +{ + id previousLogger = SCValdiGetSharedLogger(); + SCValdiCapturingLogger *logger = [SCValdiCapturingLogger new]; + SCValdiSetSharedLogger(logger); + + @try { + for (NSNumber *invalidPort in @[@(-1), @(65536)]) { + [logger reset]; + @autoreleasepool { + SCValdiRuntimeManager *runtimeManager = [SCValdiRuntimeManager new]; + [runtimeManager updateConfiguration:^(SCValdiConfiguration *configuration) { + configuration.debuggerServicePort = invalidPort.integerValue; + }]; + (void)runtimeManager.mainRuntime; + } + + NSString *warning = nil; + for (NSString *message in [logger capturedMessages]) { + if ([message containsString:@"Ignoring invalid Valdi debugger service port"]) { + warning = message; + break; + } + } + XCTAssertNotNil(warning); + XCTAssertTrue([warning containsString:@""]); + XCTAssertFalse([warning containsString:invalidPort.stringValue]); + } + } @finally { + SCValdiSetSharedLogger(previousLogger); + } +} + - (void)testRelativeLineHeightScalesNaturalLineHeightViaMultiple { UIFont *font = [UIFont systemFontOfSize:14]; diff --git a/valdi/test/java/support/AppBootstrapActivityTest.kt b/valdi/test/java/support/AppBootstrapActivityTest.kt new file mode 100644 index 00000000..5c916b31 --- /dev/null +++ b/valdi/test/java/support/AppBootstrapActivityTest.kt @@ -0,0 +1,161 @@ +package com.snap.valdi.support + +import android.content.Intent +import android.content.pm.ApplicationInfo +import android.util.Log +import com.snap.valdi.views.ValdiRootView +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowLog + +class TestAppBootstrapActivity : AppBootstrapActivity() { + val lifecycleEvents = mutableListOf() + + override fun createRootView(bootstrapper: AppBootstrapper): ValdiRootView { + error("Runtime bootstrap is replaced by the test seam") + } + + protected override fun loadNativeLibrary() { + lifecycleEvents += "loadNativeLibrary" + } + + protected override fun setDebuggerPortEnvironment(debuggerPort: Int) { + lifecycleEvents += "setDebuggerPortEnvironment:$debuggerPort" + } + + protected override fun bootstrapValdiRuntime() { + lifecycleEvents += "bootstrapValdiRuntime" + } +} + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [19], manifest = Config.NONE) +class AppBootstrapActivityTest { + + @Before + fun clearLogs() { + ShadowLog.clear() + } + + @Test + fun debuggerPortOverrideIsAvailableOnlyToDebuggableApplications() { + val intent = Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 13644) + val wrongTypedIntent = Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, "secret-port-value") + + assertEquals(13644, invokeRequestedValdiDebuggerPort(intent, true)) + assertNull(invokeRequestedValdiDebuggerPort(intent, false)) + assertNull(invokeRequestedValdiDebuggerPort(wrongTypedIntent, false)) + assertTrue(valdiWarnings().isEmpty()) + } + + @Test + fun acceptsDebuggerPortBoundaries() { + assertEquals( + 1, + invokeRequestedValdiDebuggerPort( + Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 1), + true, + ), + ) + assertEquals( + 65535, + invokeRequestedValdiDebuggerPort( + Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 65535), + true, + ), + ) + } + + @Test + fun absentDebuggerPortOverrideDoesNotWarn() { + assertNull(invokeRequestedValdiDebuggerPort(null, true)) + assertNull(invokeRequestedValdiDebuggerPort(Intent(), true)) + + assertTrue(valdiWarnings().isEmpty()) + } + + @Test + fun invalidDebuggerPortOverridesWarnOnceWithRedactedValues() { + val invalidIntents = listOf( + Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, -1), + Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 0), + Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 65536), + Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, "secret-port-value"), + ) + + invalidIntents.forEach { intent -> + ShadowLog.clear() + + assertNull(invokeRequestedValdiDebuggerPort(intent, true)) + + val warnings = valdiWarnings() + assertEquals(1, warnings.size) + assertTrue(warnings.single().contains("")) + assertFalse(warnings.single().contains("secret-port-value")) + } + } + + @Test + fun onCreateAppliesValidDebuggerPortAfterLibraryLoadOnApi19() { + val activity = createActivity( + Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 13702), + debuggable = true, + ) + + assertEquals( + listOf("loadNativeLibrary", "setDebuggerPortEnvironment:13702", "bootstrapValdiRuntime"), + activity.lifecycleEvents, + ) + assertTrue(valdiWarnings().isEmpty()) + } + + @Test + fun onCreateNeverAppliesAbsentInvalidWrongTypedOrNonDebuggableOverrides() { + val cases = listOf( + Triple(Intent(), true, 0), + Triple(Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, -1), true, 1), + Triple(Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 65536), true, 1), + Triple(Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, "secret-port-value"), true, 1), + Triple(Intent().putExtra(VALDI_DEBUGGER_PORT_INTENT_EXTRA, 13702), false, 0), + ) + + cases.forEach { (intent, debuggable, expectedWarnings) -> + ShadowLog.clear() + + val activity = createActivity(intent, debuggable) + + assertEquals(listOf("loadNativeLibrary", "bootstrapValdiRuntime"), activity.lifecycleEvents) + assertEquals(expectedWarnings, valdiWarnings().size) + assertFalse(valdiWarnings().any { it.contains("secret-port-value") }) + } + } + + private fun valdiWarnings(): List = + ShadowLog.getLogsForTag("Valdi").filter { it.type == Log.WARN }.map { it.msg } + + private fun invokeRequestedValdiDebuggerPort(intent: Intent?, debuggable: Boolean): Int? = + Class.forName("com.snap.valdi.support.AppBootstrapActivityKt") + .getDeclaredMethod("requestedValdiDebuggerPort", Intent::class.java, Boolean::class.java) + .invoke(null, intent, debuggable) as Int? + + private fun createActivity(intent: Intent, debuggable: Boolean): TestAppBootstrapActivity { + val controller = Robolectric.buildActivity(TestAppBootstrapActivity::class.java, intent) + val activity = controller.get() + activity.setTheme(androidx.appcompat.R.style.Theme_AppCompat) + activity.applicationInfo.flags = if (debuggable) { + activity.applicationInfo.flags or ApplicationInfo.FLAG_DEBUGGABLE + } else { + activity.applicationInfo.flags and ApplicationInfo.FLAG_DEBUGGABLE.inv() + } + controller.create() + return activity + } +} diff --git a/valdi/test/runtime/DebuggerService_tests.cpp b/valdi/test/runtime/DebuggerService_tests.cpp index 1c5a8116..b8243b8d 100644 --- a/valdi/test/runtime/DebuggerService_tests.cpp +++ b/valdi/test/runtime/DebuggerService_tests.cpp @@ -22,6 +22,7 @@ #include "valdi_core/cpp/Utils/LoggerUtils.hpp" #include "valdi_core/cpp/Utils/Mutex.hpp" +#include #include using namespace Valdi; @@ -153,6 +154,36 @@ struct MockTCPClientListener : public ITCPClientListener, public MockListenerc_str(), 1); + } else { + unsetenv(_key); + } + } + + void set(const char* value) { + setenv(_key, value, 1); + } + + void unset() { + unsetenv(_key); + } + +private: + const char* _key; + std::optional _existingValue; +}; + struct DebuggerServiceWrapper { Ref listener; Shared service; @@ -223,6 +254,118 @@ TEST(DebuggerService, canConnectAndDisconnect) { client->disconnect(); } +TEST(DebuggerService, resolveDebuggerPortUsesPlatformDefaults) { + ScopedEnvironmentVariable debuggerPortEnv("VALDI_DEBUGGER_PORT"); + debuggerPortEnv.unset(); + + auto mobileResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(false, std::nullopt); + auto standaloneResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(true, std::nullopt); + + ASSERT_EQ(static_cast(13592), mobileResolution.port); + ASSERT_FALSE(mobileResolution.rejectedRequestedPort.has_value()); + ASSERT_FALSE(mobileResolution.environmentError.has_value()); + ASSERT_EQ(static_cast(13591), standaloneResolution.port); + ASSERT_FALSE(standaloneResolution.rejectedRequestedPort.has_value()); + ASSERT_FALSE(standaloneResolution.environmentError.has_value()); +} + +TEST(DebuggerService, legacyResolveDebuggerPortRemainsCallableAndUsesPlatformDefaults) { + ScopedEnvironmentVariable debuggerPortEnv("VALDI_DEBUGGER_PORT"); + debuggerPortEnv.set("14000"); + + uint32_t (*legacyResolver)(bool) = &DebuggerService::resolveDebuggerPort; + + ASSERT_EQ(static_cast(13592), legacyResolver(false)); + ASSERT_EQ(static_cast(13591), legacyResolver(true)); +} + +TEST(DebuggerService, resolveDebuggerPortUsesEnvironmentFallback) { + ScopedEnvironmentVariable debuggerPortEnv("VALDI_DEBUGGER_PORT"); + debuggerPortEnv.set("14000"); + + auto resolution = DebuggerService::resolveDebuggerPortWithDiagnostics(false, std::nullopt); + + ASSERT_EQ(static_cast(14000), resolution.port); + ASSERT_FALSE(resolution.rejectedRequestedPort.has_value()); + ASSERT_FALSE(resolution.environmentError.has_value()); +} + +TEST(DebuggerService, resolveDebuggerPortPrefersValidRequestedPort) { + ScopedEnvironmentVariable debuggerPortEnv("VALDI_DEBUGGER_PORT"); + debuggerPortEnv.set("not-a-port"); + + auto requestedResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(false, 13702); + auto minimumResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(false, 1); + auto maximumResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(true, 65535); + + ASSERT_EQ(static_cast(13702), requestedResolution.port); + ASSERT_FALSE(requestedResolution.rejectedRequestedPort.has_value()); + ASSERT_FALSE(requestedResolution.environmentError.has_value()); + ASSERT_EQ(static_cast(1), minimumResolution.port); + ASSERT_EQ(static_cast(65535), maximumResolution.port); +} + +TEST(DebuggerService, resolveDebuggerPortReportsInvalidRequestedPortBeforeEnvironmentFallback) { + ScopedEnvironmentVariable debuggerPortEnv("VALDI_DEBUGGER_PORT"); + debuggerPortEnv.set("14000"); + + auto zeroResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(false, 0); + auto outOfRangeResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(true, 65536); + + ASSERT_EQ(static_cast(14000), zeroResolution.port); + ASSERT_EQ(std::optional(0), zeroResolution.rejectedRequestedPort); + ASSERT_FALSE(zeroResolution.environmentError.has_value()); + ASSERT_EQ(static_cast(14000), outOfRangeResolution.port); + ASSERT_EQ(std::optional(65536), outOfRangeResolution.rejectedRequestedPort); + ASSERT_FALSE(outOfRangeResolution.environmentError.has_value()); +} + +TEST(DebuggerService, resolveDebuggerPortReportsAllInvalidSourcesBeforePlatformFallback) { + ScopedEnvironmentVariable debuggerPortEnv("VALDI_DEBUGGER_PORT"); + debuggerPortEnv.set("not-a-port"); + + auto resolution = DebuggerService::resolveDebuggerPortWithDiagnostics(false, 0); + + ASSERT_EQ(static_cast(13592), resolution.port); + ASSERT_EQ(std::optional(0), resolution.rejectedRequestedPort); + ASSERT_EQ(std::optional(DebuggerPortEnvironmentError::Malformed), + resolution.environmentError); +} + +TEST(DebuggerService, resolveDebuggerPortRejectsInvalidEnvironmentValues) { + ScopedEnvironmentVariable debuggerPortEnv("VALDI_DEBUGGER_PORT"); + struct InvalidEnvironmentPort { + const char* value; + DebuggerPortEnvironmentError expectedError; + }; + const std::vector invalidPorts = { + {"", DebuggerPortEnvironmentError::Malformed}, + {"-1", DebuggerPortEnvironmentError::Malformed}, + {"13702junk", DebuggerPortEnvironmentError::Malformed}, + {" 13702", DebuggerPortEnvironmentError::Malformed}, + {"13702 ", DebuggerPortEnvironmentError::Malformed}, + {"0", DebuggerPortEnvironmentError::OutOfRange}, + {"65536", DebuggerPortEnvironmentError::OutOfRange}, + {"4294967296", DebuggerPortEnvironmentError::OutOfRange}, + }; + + for (const auto& invalidPort : invalidPorts) { + debuggerPortEnv.set(invalidPort.value); + auto mobileResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(false, std::nullopt); + auto standaloneResolution = DebuggerService::resolveDebuggerPortWithDiagnostics(true, std::nullopt); + + ASSERT_EQ(static_cast(13592), mobileResolution.port) << invalidPort.value; + ASSERT_FALSE(mobileResolution.rejectedRequestedPort.has_value()) << invalidPort.value; + ASSERT_EQ(std::optional(invalidPort.expectedError), + mobileResolution.environmentError) + << invalidPort.value; + ASSERT_EQ(static_cast(13591), standaloneResolution.port) << invalidPort.value; + ASSERT_EQ(std::optional(invalidPort.expectedError), + standaloneResolution.environmentError) + << invalidPort.value; + } +} + TEST(DebuggerService, canReceiveUpdatedResources) { DebuggerServiceWrapper wrapper; diff --git a/valdi/test/runtime/RuntimeManager_tests.cpp b/valdi/test/runtime/RuntimeManager_tests.cpp new file mode 100644 index 00000000..432f5121 --- /dev/null +++ b/valdi/test/runtime/RuntimeManager_tests.cpp @@ -0,0 +1,85 @@ +// +// RuntimeManager_tests.cpp +// valdi-pc +// + +#include "valdi/runtime/Debugger/DebuggerService.hpp" +#include "valdi/runtime/RuntimeManager.hpp" +#include "valdi/standalone_runtime/InMemoryDiskCache.hpp" +#include "valdi/standalone_runtime/InMemoryKeychain.hpp" +#include "valdi_core/cpp/Utils/ConsoleLogger.hpp" +#include "valdi_core/cpp/Utils/LoggerUtils.hpp" +#include "valdi_test_utils.hpp" + +#include +#include +#include +#include +#include + +using namespace Valdi; + +namespace ValdiTest { + +#if SC_LOGGING_COMPILED_IN() +class CapturingLogger : public ILogger { +public: + void log(LogType type, std::string message) override { + entries.emplace_back(type, std::move(message)); + } + + std::vector> entries; +}; +#endif + +TEST(RuntimeManager, legacyConstructorRemainsCallable) { + auto mainQueue = makeShared(); + auto runtimeManager = makeShared( + mainQueue->createMainThreadDispatcher(), + nullptr, + makeShared(), + makeShared(), + nullptr, + PlatformTypeLinux, + ThreadQoSClassNormal, + strongSmallRef(&ConsoleLogger::getLogger()), + /* enableDebuggerService */ false, + /* disableHotReloader */ false, + /* isStandalone */ true); + + ASSERT_FALSE(runtimeManager->debuggerServiceEnabled()); +} + +#if SC_LOGGING_COMPILED_IN() +TEST(RuntimeManager, invalidExplicitDebuggerPortWarningIsValueRedacted) { + if (!kDebuggerServiceEnabled) { + GTEST_SKIP() << "Debugger service is compile-time disabled"; + } + + auto mainQueue = makeShared(); + auto logger = makeShared(); + auto runtimeManager = makeShared( + mainQueue->createMainThreadDispatcher(), + nullptr, + makeShared(), + makeShared(), + nullptr, + PlatformTypeLinux, + ThreadQoSClassNormal, + logger, + /* enableDebuggerService */ true, + /* disableHotReloader */ false, + /* isStandalone */ true, + /* debuggerPort */ 65536); + + ASSERT_TRUE(runtimeManager->debuggerServiceEnabled()); + const auto warning = std::find_if(logger->entries.begin(), logger->entries.end(), [](const auto& entry) { + return entry.first == LogTypeWarn && entry.second.find("requestedPort") != std::string::npos; + }); + ASSERT_NE(logger->entries.end(), warning); + EXPECT_NE(std::string::npos, warning->second.find("")); + EXPECT_EQ(std::string::npos, warning->second.find("65536")); +} +#endif + +} // namespace ValdiTest diff --git a/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.h b/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.h index 92297330..9d26b94a 100644 --- a/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.h +++ b/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.h @@ -69,6 +69,31 @@ typedef void (^SCValdiPerformHapticFeedbackBlock)(NSString* type); */ @property (assign, nonatomic) BOOL enableReferenceTracking; +/** + * Local-development-only debugger and hot-reload settings. + * + * These values are read when SCValdiRuntimeManager initializes its underlying + * RuntimeManager. Configure them through -[SCValdiRuntimeManager updateConfiguration:] + * before first accessing APIs that initialize the runtime, such as mainRuntime. + * The runtime still applies its existing compile-time debugger-service gate, + * so setting enableDebuggerService does not broaden release-build availability. + * Debugger service requests are enabled by default for compatibility with direct + * development hosts; hosts can set enableDebuggerService to NO to opt out. + */ +@property (assign, nonatomic) BOOL enableDebuggerService; +@property (assign, nonatomic) BOOL disableHotReloader; + +/** + * Optional port for the debugger / hot-reload service. + * + * Leave this as 0 to use VALDI_DEBUGGER_PORT when present, or Valdi's platform + * default when it is not. Valid explicit ports are in the range 1...65535. + * + * `valdi hotreload --port` currently targets simulator / localhost reloads. + * Physical-device USB auto-connectors still use Valdi's default mobile port. + */ +@property (assign, nonatomic) NSInteger debuggerServicePort; + /** * The currently selected JavaScript engine type. * In production, iOS uses the JavaScriptCore engine and Android uses QuickJS. diff --git a/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.m b/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.m index 7219fe19..c7d6a642 100644 --- a/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.m +++ b/valdi_core/src/valdi_core/ios/valdi_core/SCValdiConfiguration.m @@ -9,4 +9,13 @@ @implementation SCValdiConfiguration +- (instancetype)init +{ + self = [super init]; + if (self) { + self.enableDebuggerService = YES; + } + return self; +} + @end diff --git a/valdi_core/src/valdi_core/ios/valdi_core/SCValdiRuntimeManagerProtocol.h b/valdi_core/src/valdi_core/ios/valdi_core/SCValdiRuntimeManagerProtocol.h index 16d83ee6..df0368be 100644 --- a/valdi_core/src/valdi_core/ios/valdi_core/SCValdiRuntimeManagerProtocol.h +++ b/valdi_core/src/valdi_core/ios/valdi_core/SCValdiRuntimeManagerProtocol.h @@ -33,6 +33,8 @@ typedef void (^SCValdiRuntimeCreatedCallback)(id); /** The block will be provided with a SCValdiConfiguration instance that you can mutate and the runtime manager will apply this configuration after the block finishes executing. + Constructor-time settings such as debugger service configuration should be provided before + the underlying RuntimeManager is initialized. */ - (void)updateConfiguration:(void (^)(SCValdiConfiguration* configuration))updateBlock;