diff --git a/packages/react-native/android/src/main/java/com/bugsee/reactnative/BreadcrumbFilterBridge.java b/packages/react-native/android/src/main/java/com/bugsee/reactnative/BreadcrumbFilterBridge.java index d29b626c..55e7137c 100644 --- a/packages/react-native/android/src/main/java/com/bugsee/reactnative/BreadcrumbFilterBridge.java +++ b/packages/react-native/android/src/main/java/com/bugsee/reactnative/BreadcrumbFilterBridge.java @@ -119,6 +119,16 @@ static BreadcrumbFilterBridge shared() { private final ConcurrentHashMap pending = new ConcurrentHashMap<>(); private final AtomicLong ids = new AtomicLong(); private final AtomicBoolean installed = new AtomicBoolean(); + + /** + * The module whose JS has installed its filter with + * {@code setEnabled(true)}, or null. Compared to the attached sink by + * identity, so a stale module cannot arm the new one. A reload attaches a new module whose JS has + * not subscribed yet, while this bridge (and the SDK's filter) outlive + * the old runtime. Until JS arms it again, a request would be emitted to + * nobody, so it is dropped at once instead: fail closed, no wait. + */ + private final AtomicReference armedFor = new AtomicReference<>(); private final Object sdkLock = new Object(); /** * The id of the manual {@code addBreadcrumb} on this thread. Set only @@ -157,7 +167,31 @@ public void filter( }; void attach(@NonNull final Sink next) { - sink.set(next); + // Disarm before the sink is visible: a request in the gap must not + // reach a runtime that has not subscribed yet. See armedFor. + withSdkLock(() -> { + armedFor.set(null); + sink.set(next); + }); + } + + /** + * The module {@code caller}'s JS now answers requests. Ignored unless + * {@code caller} is still the attached module: a stale module's late + * enable must not arm the new runtime, which has not subscribed. + */ + void arm(@NonNull final Sink caller) { + if (sink.get() == caller) { + armedFor.set(caller); + } + } + + /** Arms for whichever module is attached. For tests, which have no module identity of their own. */ + void arm() { + final Sink current = sink.get(); + if (current != null) { + armedFor.set(current); + } } /** @@ -167,7 +201,10 @@ void attach(@NonNull final Sink next) { * callback is run with {@code null}, which discards the crumb. */ void detach(@NonNull final Sink current) { - sink.compareAndSet(current, null); + withSdkLock(() -> { + sink.compareAndSet(current, null); + armedFor.compareAndSet(current, null); + }); final List owned = new ArrayList<>(); for (final Pending item : pending.values()) { if (item.owner == current && pending.remove(item.id, item)) { @@ -196,8 +233,20 @@ void withSdkLock(@NonNull final Runnable body) { /** Installs the filter, or removes it. A second {@code true} is a no-op. */ void setEnabled(final boolean enabled) { + setEnabled(enabled, sink.get()); + } + + /** As above, on behalf of module {@code caller}; only the attached module can arm. */ + void setEnabled(final boolean enabled, final Sink caller) { withSdkLock(() -> { + if (sink.get() != caller) { + // A superseded module must neither arm nor disable the process-wide filter. + return; + } if (enabled) { + if (caller != null) { + arm(caller); + } if (installed.compareAndSet(false, true)) { Bugsee.setBreadcrumbFilter(filter); } @@ -265,7 +314,7 @@ void ask( @NonNull final Callback1 callback ) { final Sink current = sink.get(); - if (current == null) { + if (current == null || armedFor.get() != current) { drop(callback); return; } diff --git a/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java b/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java index f93ca049..713dcecd 100644 --- a/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java +++ b/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java @@ -145,10 +145,7 @@ public BugseeModule(final ReactApplicationContext context) { ReportHandlerBridge.shared().attach(this); DataRequestBridge.shared().attach(dataRequestSink, originTracker::currentOrigin); NetworkFilterBridge.shared().attach(this); - LogFilterBridge.shared().attach(this); - // The echo drop is installed even when the app never calls setLogFilter. - // setEnabled(false) is that pass-through; it does not remove the filter. - LogFilterBridge.shared().setEnabled(false); + LogFilterBridge.shared().attachModule(this); BreadcrumbFilterBridge.shared().attach(this); } @@ -659,7 +656,7 @@ public void onNetworkFilterRequest(@NonNull final String requestId, @NonNull fin */ @Override public boolean setNetworkFilterEnabled(final boolean enabled) { - NetworkFilterBridge.shared().setEnabled(enabled); + NetworkFilterBridge.shared().setEnabled(enabled, this); return true; } @@ -859,7 +856,7 @@ public void onLogFilterRequest(@NonNull final String requestId, @NonNull final S */ @Override public boolean setLogFilterEnabled(final boolean enabled) { - LogFilterBridge.shared().setEnabled(enabled); + LogFilterBridge.shared().setEnabled(enabled, this); return true; } @@ -896,7 +893,7 @@ public void onBreadcrumbFilterRequest( */ @Override public boolean setBreadcrumbFilterEnabled(final boolean enabled) { - BreadcrumbFilterBridge.shared().setEnabled(enabled); + BreadcrumbFilterBridge.shared().setEnabled(enabled, this); return true; } diff --git a/packages/react-native/android/src/main/java/com/bugsee/reactnative/LogFilterBridge.java b/packages/react-native/android/src/main/java/com/bugsee/reactnative/LogFilterBridge.java index d10054d6..ecdb38a9 100644 --- a/packages/react-native/android/src/main/java/com/bugsee/reactnative/LogFilterBridge.java +++ b/packages/react-native/android/src/main/java/com/bugsee/reactnative/LogFilterBridge.java @@ -119,6 +119,18 @@ static LogFilterBridge shared() { private final AtomicLong ids = new AtomicLong(); /** True only while the app has a log callback. False is the native pass-through. */ private final AtomicBoolean userFilter = new AtomicBoolean(); + /** Orders attach against setEnabled; see {@link #setEnabled(boolean, Sink)}. */ + private final Object stateLock = new Object(); + + /** + * The module whose JS has installed its filter with + * {@code setEnabled(true)}, or null. Compared to the attached sink by + * identity, so a stale module cannot arm the new one. A reload attaches a new module whose JS has + * not subscribed yet, while this bridge (and the SDK's filter) outlive + * the old runtime. Until JS arms it again, a request would be emitted to + * nobody, so it is dropped at once instead: fail closed, no wait. + */ + private final AtomicReference armedFor = new AtomicReference<>(); /** Production: one daemon thread forgets pending requests at {@link #BORROW_MS}. */ private LogFilterBridge() { @@ -162,7 +174,46 @@ public void filter( }; void attach(@NonNull final Sink next) { - sink.set(next); + // Disarm before the sink is visible: a request in the gap must not + // reach a runtime that has not subscribed yet. See armedFor. userFilter + // is deliberately kept: a redaction the app relied on must not silently + // stop applying across a reload. + synchronized (stateLock) { + armedFor.set(null); + sink.set(next); + } + } + + /** + * What a new module does at construction: attach, then put the echo drop + * in place even when the app never calls setLogFilter. Not + * {@code setEnabled(false)}: after a reload that would turn a redaction the + * app relied on into a silent pass-through. The attach disarms the bridge, + * so with a user filter still on, lines are dropped until the new runtime + * calls setLogFilter (re-arm) or clears it (pass-through). + */ + void attachModule(@NonNull final Sink next) { + attach(next); + ensureInstalled(); + } + + /** + * The module {@code caller}'s JS now answers requests. Ignored unless + * {@code caller} is still the attached module: a stale module's late + * enable must not arm the new runtime, which has not subscribed. + */ + void arm(@NonNull final Sink caller) { + if (sink.get() == caller) { + armedFor.set(caller); + } + } + + /** Arms for whichever module is attached. For tests, which have no module identity of their own. */ + void arm() { + final Sink current = sink.get(); + if (current != null) { + armedFor.set(current); + } } /** @@ -174,7 +225,10 @@ void attach(@NonNull final Sink next) { * forgotten and the callback is not touched. */ void detach(@NonNull final Sink current) { - sink.compareAndSet(current, null); + synchronized (stateLock) { + sink.compareAndSet(current, null); + armedFor.compareAndSet(current, null); + } final List owned = new ArrayList<>(); for (final Pending item : pending.values()) { if (item.owner == current && pending.remove(item.id, item)) { @@ -196,7 +250,24 @@ void detach(@NonNull final Sink current) { * {@link Bugsee#setLogEventFilter}. */ void setEnabled(final boolean enabled) { - userFilter.set(enabled); + setEnabled(enabled, sink.get()); + } + + /** As above, on behalf of module {@code caller}; only the attached module can arm. */ + void setEnabled(final boolean enabled, final Sink caller) { + // One lock for the identity check and the write, so a stale module + // cannot pass the check, pause, and switch off a filter the new + // module has since turned on. + synchronized (stateLock) { + if (sink.get() != caller) { + // A superseded module must neither arm nor disable the process-wide filter. + return; + } + if (enabled && caller != null) { + arm(caller); + } + userFilter.set(enabled); + } ensureInstalled(); } @@ -285,7 +356,7 @@ void ask( ) { final String line = event.getMessage(); final Sink current = sink.get(); - if (line == null || current == null) { + if (line == null || current == null || armedFor.get() != current) { drop(callback); return; } diff --git a/packages/react-native/android/src/main/java/com/bugsee/reactnative/NetworkFilterBridge.java b/packages/react-native/android/src/main/java/com/bugsee/reactnative/NetworkFilterBridge.java index 6c480874..32bf61e6 100644 --- a/packages/react-native/android/src/main/java/com/bugsee/reactnative/NetworkFilterBridge.java +++ b/packages/react-native/android/src/main/java/com/bugsee/reactnative/NetworkFilterBridge.java @@ -111,6 +111,18 @@ static NetworkFilterBridge shared() { private final ConcurrentHashMap pending = new ConcurrentHashMap<>(); private final AtomicLong ids = new AtomicLong(); private final AtomicBoolean installed = new AtomicBoolean(); + /** Orders attach against setEnabled; see {@link #setEnabled(boolean, Sink)}. */ + private final Object stateLock = new Object(); + + /** + * The module whose JS has installed its filter with + * {@code setEnabled(true)}, or null. Compared to the attached sink by + * identity, so a stale module cannot arm the new one. A reload attaches a new module whose JS has + * not subscribed yet, while this bridge (and the SDK's filter) outlive + * the old runtime. Until JS arms it again, a request would be emitted to + * nobody, so it is dropped at once instead: fail closed, no wait. + */ + private final AtomicReference armedFor = new AtomicReference<>(); /** Production: one daemon thread forgets pending requests at {@link #BORROW_MS}. */ private NetworkFilterBridge() { @@ -143,7 +155,31 @@ public void filter( }; void attach(@NonNull final Sink next) { - sink.set(next); + // Disarm before the sink is visible: a request in the gap must not + // reach a runtime that has not subscribed yet. See armedFor. + synchronized (stateLock) { + armedFor.set(null); + sink.set(next); + } + } + + /** + * The module {@code caller}'s JS now answers requests. Ignored unless + * {@code caller} is still the attached module: a stale module's late + * enable must not arm the new runtime, which has not subscribed. + */ + void arm(@NonNull final Sink caller) { + if (sink.get() == caller) { + armedFor.set(caller); + } + } + + /** Arms for whichever module is attached. For tests, which have no module identity of their own. */ + void arm() { + final Sink current = sink.get(); + if (current != null) { + armedFor.set(current); + } } /** @@ -152,7 +188,10 @@ void attach(@NonNull final Sink next) { * forgotten and the callback is not touched. */ void detach(@NonNull final Sink current) { - sink.compareAndSet(current, null); + synchronized (stateLock) { + sink.compareAndSet(current, null); + armedFor.compareAndSet(current, null); + } final List owned = new ArrayList<>(); for (final Pending item : pending.values()) { if (item.owner == current && pending.remove(item.id, item)) { @@ -169,14 +208,31 @@ void detach(@NonNull final Sink current) { /** Installs the filter, or removes it. A second {@code true} is a no-op. */ void setEnabled(final boolean enabled) { - if (enabled) { - if (installed.compareAndSet(false, true)) { - Bugsee.setNetworkEventFilter(filter); + setEnabled(enabled, sink.get()); + } + + /** As above, on behalf of module {@code caller}; only the attached module can arm. */ + void setEnabled(final boolean enabled, final Sink caller) { + // One lock for the identity check and the install, so a stale module + // cannot pass the check, pause, and remove a filter the new module + // has since relied on. + synchronized (stateLock) { + if (sink.get() != caller) { + // A superseded module must neither arm nor disable the process-wide filter. + return; + } + if (enabled) { + if (caller != null) { + arm(caller); + } + if (installed.compareAndSet(false, true)) { + Bugsee.setNetworkEventFilter(filter); + } + return; + } + if (installed.compareAndSet(true, false)) { + Bugsee.setNetworkEventFilter(null); } - return; - } - if (installed.compareAndSet(true, false)) { - Bugsee.setNetworkEventFilter(null); } } @@ -221,6 +277,11 @@ void ask( @NonNull final NetworkEvent event, @NonNull final Callback1 callback ) { + final Sink asked = sink.get(); + if (asked == null || armedFor.get() != asked) { + drop(callback); + return; + } // setNetworkEventFilter skips DefaultNetworkDataSanitizer. The // capture provider runs it only when no filter is installed, and // only when CaptureNetworkUseDefaultSanitizer is on (default true). @@ -243,10 +304,9 @@ void ask( drop(callback); return; } - final Sink current = sink.get(); // A null URL cannot tell a recycled entry (reset nulls the URL) from // the one we were asked about, so it is dropped rather than lent. - if (url == null || current == null) { + if (url == null) { drop(callback); return; } @@ -258,11 +318,11 @@ void ask( return; } final String requestId = Long.toString(ids.incrementAndGet()); - final Pending item = new Pending(requestId, event, id, url, callback, current); + final Pending item = new Pending(requestId, event, id, url, callback, asked); pending.put(requestId, item); try { item.deadline = scheduler.schedule(() -> forget(requestId), BORROW_MS); - current.onNetworkFilterRequest(requestId, eventJson); + asked.onNetworkFilterRequest(requestId, eventJson); } catch (final Throwable e) { if (pending.remove(requestId, item)) { cancel(item.deadline); diff --git a/packages/react-native/android/src/test/java/com/bugsee/reactnative/BreadcrumbFilterBridgeTest.java b/packages/react-native/android/src/test/java/com/bugsee/reactnative/BreadcrumbFilterBridgeTest.java index d801d442..6d8bc349 100644 --- a/packages/react-native/android/src/test/java/com/bugsee/reactnative/BreadcrumbFilterBridgeTest.java +++ b/packages/react-native/android/src/test/java/com/bugsee/reactnative/BreadcrumbFilterBridgeTest.java @@ -216,6 +216,7 @@ public void snapshotUsesGetValueAndOmitsUnsetKeys() throws Exception { assertEquals(2, Breadcrumb.Level.WARNING.ordinal()); final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.level = Breadcrumb.Level.WARNING; crumb.message = "secret"; @@ -236,6 +237,7 @@ public void snapshotUsesGetValueAndOmitsUnsetKeys() throws Exception { public void snapshotIncludesDataAndAStampedTimestamp() throws Exception { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.category = "ui"; crumb.message = "tap"; @@ -259,6 +261,7 @@ public void snapshotIncludesDataAndAStampedTimestamp() throws Exception { public void aReplyWritesGetValueNotTheOrdinal() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.level = Breadcrumb.Level.FATAL; final RecordingCallback callback = new RecordingCallback(); @@ -276,6 +279,7 @@ public void aReplyWritesGetValueNotTheOrdinal() { public void aNullReplyDropsWithoutPassingTheCrumbThrough() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; crumb.level = Breadcrumb.Level.ERROR; @@ -295,6 +299,7 @@ public void aNullReplyDropsWithoutPassingTheCrumbThrough() { public void aKeepRewritesTheNamedFieldsAndClearsDataWithNull() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.category = "ui"; crumb.message = "secret"; @@ -320,6 +325,7 @@ public void aKeepRewritesTheNamedFieldsAndClearsDataWithNull() { public void aReplyWritesData() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -339,6 +345,7 @@ public void aReplyWritesData() { public void aReplyThatDoesNotStickDropsTheCrumb() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.category = "secret"; crumb.stick = false; @@ -358,6 +365,7 @@ public void aReplyThatDoesNotStickDropsTheCrumb() { public void anIntegerLevelDropsTheCrumb() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.level = Breadcrumb.Level.INFO; final RecordingCallback callback = new RecordingCallback(); @@ -385,6 +393,7 @@ public void aFailedEmitDropsAndALaterReplyIsANoOp() { final RecordingSink sink = new RecordingSink(); sink.throwOnEmit = true; bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -401,6 +410,7 @@ public void aFailedEmitDropsAndALaterReplyIsANoOp() { public void detachDropsAPendingCrumbAndALaterReplyDoesNotWriteIt() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -420,6 +430,7 @@ public void detachDropsAPendingCrumbAndALaterReplyDoesNotWriteIt() { public void aSecondReplyIsANoOp() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -438,6 +449,7 @@ public void aSecondReplyIsANoOp() { public void dataThatCannotBeSerialisedDropsTheCrumb() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final Map data = new HashMap<>(); @@ -455,6 +467,7 @@ public void dataThatCannotBeSerialisedDropsTheCrumb() { public void anUnansweredCrumbIsDroppedAtTheDeadlineAndALateReplyIsANoOp() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -479,6 +492,7 @@ public void anUnansweredCrumbIsDroppedAtTheDeadlineAndALateReplyIsANoOp() { public void aReplyBeforeTheDeadlineCancelsItAndFiringLaterDoesNothing() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -497,6 +511,7 @@ public void aFailedEmitCancelsTheDeadline() { final RecordingSink sink = new RecordingSink(); sink.throwOnEmit = true; bridge.attach(sink); + bridge.arm(); final RecordingCallback callback = new RecordingCallback(); bridge.ask(new MutableCrumb(), callback); @@ -509,6 +524,7 @@ public void aFailedEmitCancelsTheDeadline() { public void detachCancelsTheDeadline() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final RecordingCallback callback = new RecordingCallback(); bridge.ask(new MutableCrumb(), callback); @@ -523,6 +539,7 @@ public void detachCancelsTheDeadline() { public void eachCrumbHasItsOwnDeadline() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final RecordingCallback first = new RecordingCallback(); final RecordingCallback second = new RecordingCallback(); bridge.ask(new MutableCrumb(), first); @@ -541,6 +558,7 @@ public void eachCrumbHasItsOwnDeadline() { public void aReplyAfterTheDeadlineIsRefusedEvenIfTheTimerHasNotRun() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -562,6 +580,7 @@ public void aReplyAfterTheDeadlineIsRefusedEvenIfTheTimerHasNotRun() { public void aReplyJustInsideTheDeadlineIsAccepted() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableCrumb crumb = new MutableCrumb(); crumb.message = "secret"; final RecordingCallback callback = new RecordingCallback(); @@ -574,4 +593,60 @@ public void aReplyJustInsideTheDeadlineIsAccepted() { assertEquals("redacted", crumb.message); assertSame(crumb, callback.last); } + + @Test + public void aNewModuleThatHasNotArmedDropsTheCrumbAtOnce() { + final RecordingSink old = new RecordingSink(); + bridge.attach(old); + bridge.arm(); + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + + final MutableCrumb crumb = new MutableCrumb(); + crumb.message = "secret"; + final RecordingCallback callback = new RecordingCallback(); + bridge.ask(crumb, callback); + + assertEquals(0, next.json.size()); + assertEquals(1, callback.runs); + assertTrue(callback.sawNull); + } + + @Test + public void armingAgainAsksTheNewModule() { + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + bridge.arm(); + + final MutableCrumb crumb = new MutableCrumb(); + crumb.message = "secret"; + final RecordingCallback callback = new RecordingCallback(); + bridge.ask(crumb, callback); + + assertEquals(1, next.json.size()); + assertEquals(0, callback.runs); + } + + /** A late enable from the module a reload replaced must not arm the new, unsubscribed runtime. */ + @Test + public void aStaleModulesEnableDoesNotArmTheNewModule() { + final RecordingSink old = new RecordingSink(); + bridge.attach(old); + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + bridge.arm(old); + + final MutableCrumb ev = new MutableCrumb(); + ev.message = "secret"; + final RecordingCallback dropped = new RecordingCallback(); + bridge.ask(ev, dropped); + assertEquals(0, next.json.size()); + assertEquals(1, dropped.runs); + + bridge.arm(next); + final RecordingCallback asked = new RecordingCallback(); + bridge.ask(ev, asked); + assertEquals(1, next.json.size()); + assertEquals(0, asked.runs); + } } diff --git a/packages/react-native/android/src/test/java/com/bugsee/reactnative/LogFilterBridgeTest.java b/packages/react-native/android/src/test/java/com/bugsee/reactnative/LogFilterBridgeTest.java index 84157ed5..935ba518 100644 --- a/packages/react-native/android/src/test/java/com/bugsee/reactnative/LogFilterBridgeTest.java +++ b/packages/react-native/android/src/test/java/com/bugsee/reactnative/LogFilterBridgeTest.java @@ -139,6 +139,7 @@ public LogLevel getLevel() { public void aReplyAfterTheMessageChangedDoesNotTouchTheEntry() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableLog event = new MutableLog("secret"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -158,6 +159,7 @@ public void aReplyAfterTheMessageChangedDoesNotTouchTheEntry() { public void detachingAnOldSinkDropsItsPendingWithoutTouchingARecycledEntry() { final RecordingSink stale = new RecordingSink(); bridge.attach(stale); + bridge.arm(); final MutableLog event = new MutableLog("secret"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -166,6 +168,7 @@ public void detachingAnOldSinkDropsItsPendingWithoutTouchingARecycledEntry() { event.message = "another line"; final RecordingSink current = new RecordingSink(); bridge.attach(current); + bridge.arm(); bridge.detach(stale); assertEquals(0, callback.runs); @@ -193,6 +196,7 @@ public void aReplyAfterTheDeadlineIsANoOpWhenTheMessageIsUnchanged() { assertTrue(LogFilterBridge.BORROW_MS < 10_000L); final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableLog event = new MutableLog("secret"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -214,6 +218,7 @@ public void aReplyAfterTheDeadlineIsANoOpWhenTheMessageIsUnchanged() { public void aDeadlineForgetsThePendingWithoutCallingTheCallback() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableLog event = new MutableLog("secret"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -231,6 +236,7 @@ public void aDeadlineForgetsThePendingWithoutCallingTheCallback() { public void aReplyWhileTheEntryIsStillTheOriginalWritesItBack() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableLog event = new MutableLog("secret"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -249,6 +255,7 @@ public void aReplyWhileTheEntryIsStillTheOriginalWritesItBack() { public void detachDropsALiveBorrowAndSkipsARecycledOne() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableLog live = new MutableLog("keep-or-drop"); final RecordingCallback liveCallback = new RecordingCallback(); bridge.ask(live, liveCallback); @@ -275,6 +282,7 @@ public void detachDropsALiveBorrowAndSkipsARecycledOne() { public void routeAsksTheChannelLineAndDropsItsLogcatEcho() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); bridge.setEnabled(true); bridge.noteEcho("hello"); final RecordingCallback channelCallback = new RecordingCallback(); @@ -313,6 +321,7 @@ public void routeAsksTheChannelLineAndDropsItsLogcatEcho() { public void anEchoFilteredBeforeTheChannelLineIsDroppedAndTheChannelLineAsked() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); bridge.setEnabled(true); bridge.noteEcho("dedup-raw n"); @@ -338,6 +347,7 @@ public void anEchoFilteredBeforeTheChannelLineIsDroppedAndTheChannelLineAsked() public void twoIdenticalConsoleLinesAreAskedTwiceAndBothEchoesDropped() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); bridge.setEnabled(true); bridge.noteEcho("same"); bridge.noteEcho("same"); @@ -369,6 +379,7 @@ public void twoIdenticalConsoleLinesAreAskedTwiceAndBothEchoesDropped() { public void anEchoFilteredBeforeItsNoteIsKept() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); bridge.setEnabled(true); final RecordingCallback echoCallback = new RecordingCallback(); @@ -460,4 +471,101 @@ public String getTag() { return tag; } } + + /** + * The reload case: the app had a log filter, the new module attaches but + * its JS has not called setLogFilter. Lines are dropped, never recorded + * unfiltered, and no request is left pending. + */ + @Test + public void afterAReloadAUserFilterStaysOnAndDropsUntilJsArmsAgain() { + final RecordingSink old = new RecordingSink(); + bridge.attach(old); + bridge.setEnabled(true); + final RecordingSink next = new RecordingSink(); + bridge.attachModule(next); + + final RecordingCallback dropped = new RecordingCallback(); + bridge.route(new MutableLog("secret"), dropped); + assertEquals(0, next.ids.size()); + assertEquals(1, dropped.runs); + assertTrue(dropped.sawNull); + assertEquals(0, scheduler.tasks.size()); + + bridge.setEnabled(true); + final RecordingCallback asked = new RecordingCallback(); + bridge.route(new MutableLog("secret"), asked); + assertEquals(1, next.ids.size()); + assertEquals(0, asked.runs); + } + + @Test + public void attachModuleInstallsTheEchoDropAndKeepsTheUserFilter() { + final RecordingSink old = new RecordingSink(); + bridge.attachModule(old); + bridge.setEnabled(true, old); + final RecordingSink next = new RecordingSink(); + bridge.attachModule(next); + + final RecordingCallback callback = new RecordingCallback(); + bridge.route(new MutableLog("secret"), callback); + assertTrue(callback.sawNull); + assertEquals(0, next.ids.size()); + } + + @Test + public void aStaleModulesDisableDoesNotClearTheNewModulesFilter() { + final RecordingSink old = new RecordingSink(); + bridge.attachModule(old); + final RecordingSink next = new RecordingSink(); + bridge.attachModule(next); + bridge.setEnabled(true, next); + + bridge.setEnabled(false, old); + + final RecordingCallback callback = new RecordingCallback(); + bridge.route(new MutableLog("secret"), callback); + assertEquals(1, next.ids.size()); + assertEquals(0, callback.runs); + } + + @Test + public void afterAReloadClearingTheFilterIsThePassThrough() { + final RecordingSink old = new RecordingSink(); + bridge.attach(old); + bridge.setEnabled(true); + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + + bridge.setEnabled(false); + final MutableLog event = new MutableLog("kept"); + final RecordingCallback callback = new RecordingCallback(); + bridge.route(event, callback); + + assertEquals(0, next.ids.size()); + assertEquals(1, callback.runs); + assertTrue(!callback.sawNull); + } + + /** A late enable from the module a reload replaced must not arm the new, unsubscribed runtime. */ + @Test + public void aStaleModulesEnableDoesNotArmTheNewModule() { + final RecordingSink old = new RecordingSink(); + bridge.attach(old); + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + bridge.setEnabled(true, old); + + final MutableLog ev = new MutableLog("secret"); + final RecordingCallback dropped = new RecordingCallback(); + bridge.ask(ev, dropped); + assertEquals(0, next.ids.size()); + assertEquals(1, dropped.runs); + + bridge.setEnabled(true, next); + final RecordingCallback asked = new RecordingCallback(); + bridge.ask(ev, asked); + assertEquals(1, next.ids.size()); + assertEquals(0, asked.runs); + } } diff --git a/packages/react-native/android/src/test/java/com/bugsee/reactnative/NetworkFilterBridgeTest.java b/packages/react-native/android/src/test/java/com/bugsee/reactnative/NetworkFilterBridgeTest.java index 588c9736..dea49718 100644 --- a/packages/react-native/android/src/test/java/com/bugsee/reactnative/NetworkFilterBridgeTest.java +++ b/packages/react-native/android/src/test/java/com/bugsee/reactnative/NetworkFilterBridgeTest.java @@ -233,6 +233,7 @@ public WebSocketEventType getWebSocketEventType() { public void aReplyAfterTheUrlChangedDoesNotTouchTheEntry() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableNetwork event = new MutableNetwork("https://secret.example/token"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -252,6 +253,7 @@ public void aReplyAfterTheUrlChangedDoesNotTouchTheEntry() { public void aDeadlineForgetsThePendingWithoutCallingTheCallback() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableNetwork event = new MutableNetwork("https://secret.example/token"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -268,6 +270,7 @@ public void aDeadlineForgetsThePendingWithoutCallingTheCallback() { public void aReplyWhileTheEntryIsStillTheOriginalWritesItBack() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableNetwork event = new MutableNetwork("https://secret.example/token"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -285,6 +288,7 @@ public void aReplyWhileTheEntryIsStillTheOriginalWritesItBack() { public void aNullReplyDropsALiveBorrow() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableNetwork event = new MutableNetwork("https://secret.example/token"); final RecordingCallback callback = new RecordingCallback(); bridge.ask(event, callback); @@ -301,6 +305,7 @@ public void aNullReplyDropsALiveBorrow() { public void detachDropsALiveBorrowAndSkipsARecycledOne() { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableNetwork live = new MutableNetwork("https://live.example"); final RecordingCallback liveCallback = new RecordingCallback(); bridge.ask(live, liveCallback); @@ -328,6 +333,7 @@ public void aDefaultSanitizerRedactsTokenBeforeTheUrlIsRemembered() throws Excep final NetworkFilterBridge sanitizing = new NetworkFilterBridge(localScheduler, () -> true); final RecordingSink sink = new RecordingSink(); sanitizing.attach(sink); + sanitizing.arm(); final MutableNetwork event = new MutableNetwork( "https://api.example/v1/items?token=bugsee-secret-token-value&ok=1" ); @@ -361,6 +367,7 @@ public void aDefaultSanitizerRedactsTokenBeforeTheUrlIsRemembered() throws Excep public void snapshotCarriesTheOverrideFlagAndAReplyCannotChangeIt() throws Exception { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableNetwork start = new MutableNetwork("https://api.example/v1/items"); final MutableNetwork supplement = new MutableNetwork("https://api.example/v1/items"); supplement.override = true; @@ -388,6 +395,7 @@ public void snapshotCarriesTheOverrideFlagAndAReplyCannotChangeIt() throws Excep public void jsonNullClearsErrorFieldsAndStatusText() throws Exception { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final MutableNetwork event = new MutableNetwork("https://api.example/v1/items"); event.errorDescription = "gateway"; event.errorShortMessage = "late"; @@ -416,6 +424,7 @@ public void jsonNullClearsErrorFieldsAndStatusText() throws Exception { public void aDisabledDefaultSanitizerLeavesTheTokenQuery() throws Exception { final RecordingSink sink = new RecordingSink(); bridge.attach(sink); + bridge.arm(); final String url = "https://api.example/v1/items?token=bugsee-secret-token-value&ok=1"; final MutableNetwork event = new MutableNetwork(url); bridge.ask(event, new RecordingCallback()); @@ -454,4 +463,61 @@ private static OptionsContainer options(final OptionRead read) { } ); } + + /** + * A reload attaches a new module whose JS has not installed its filter. + * The request is dropped at once: not emitted to nobody, not left to the + * 9s forget and the SDK's 10s timeout. + */ + @Test + public void aNewModuleThatHasNotArmedDropsAtOnceAndNothingIsPending() { + final RecordingSink old = new RecordingSink(); + bridge.attach(old); + bridge.arm(); + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + + final RecordingCallback callback = new RecordingCallback(); + bridge.ask(new MutableNetwork("https://secret.example/token"), callback); + + assertEquals(0, next.ids.size()); + assertEquals(1, callback.runs); + assertTrue(callback.sawNull); + assertEquals(0, scheduler.tasks.size()); + } + + @Test + public void armingAgainAsksTheNewModule() { + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + bridge.arm(); + + final RecordingCallback callback = new RecordingCallback(); + bridge.ask(new MutableNetwork("https://secret.example/token"), callback); + + assertEquals(1, next.ids.size()); + assertEquals(0, callback.runs); + } + + /** A late enable from the module a reload replaced must not arm the new, unsubscribed runtime. */ + @Test + public void aStaleModulesEnableDoesNotArmTheNewModule() { + final RecordingSink old = new RecordingSink(); + bridge.attach(old); + final RecordingSink next = new RecordingSink(); + bridge.attach(next); + bridge.arm(old); + + final MutableNetwork ev = new MutableNetwork("https://secret.example/token"); + final RecordingCallback dropped = new RecordingCallback(); + bridge.ask(ev, dropped); + assertEquals(0, next.ids.size()); + assertEquals(1, dropped.runs); + + bridge.arm(next); + final RecordingCallback asked = new RecordingCallback(); + bridge.ask(ev, asked); + assertEquals(1, next.ids.size()); + assertEquals(0, asked.runs); + } } diff --git a/packages/react-native/ios/BugseeModule.mm b/packages/react-native/ios/BugseeModule.mm index 79ff72d5..5a7c5014 100644 --- a/packages/react-native/ios/BugseeModule.mm +++ b/packages/react-native/ios/BugseeModule.mm @@ -31,6 +31,7 @@ #import #import #import +#import #import #import #import @@ -62,6 +63,7 @@ #import "BGSRNValues.h" #import "BGSRNJSON.h" #import "BGSRNNetworkFilter.h" +#import "BGSRNFilterArming.h" #import "BGSRNNetworkEvents.h" #import "BGSRNAttributes.h" #import "BGSRNDataRequestBridge.h" @@ -464,6 +466,26 @@ @implementation BGSRNNetworkFilterPending static __weak BugseeModule *BGSRNNetworkFilterModule = nil; static BOOL BGSRNNetworkFilterInstalled = NO; +// Which module's JS installed the filter. A reload leaves the filter block in +// place and attaches a new module that has not; see BGSRNFilterArming. +/// Orders a module's claim of the three filter pointers against a setter's +/// ownership check and arm, so a superseded module cannot pass the check, +/// pause, and arm or disable after the replacement attached. Lock order is +/// this lock, then a filter's own lock. The setters hold it across the SDK +/// uninstall on the disable paths (those calls only take locks); the filter +/// blocks the SDK calls never take it. +static id BGSRNFilterOwnerLock(void) { + static id lock; + static dispatch_once_t once; + dispatch_once(&once, ^{ lock = [NSObject new]; }); + return lock; +} +static BGSRNFilterArming *BGSRNNetworkFilterArming(void) { + static BGSRNFilterArming *arming; + static dispatch_once_t once; + dispatch_once(&once, ^{ arming = [BGSRNFilterArming new]; }); + return arming; +} static NSMutableDictionary *BGSRNNetworkFilterPendingTable; static int64_t BGSRNNetworkFilterNextId = 0; @@ -516,6 +538,12 @@ @implementation BGSRNLogFilterPending static __weak BugseeModule *BGSRNLogFilterModule = nil; static BOOL BGSRNLogFilterInstalled = NO; +static BGSRNFilterArming *BGSRNLogFilterArming(void) { + static BGSRNFilterArming *arming; + static dispatch_once_t once; + dispatch_once(&once, ^{ arming = [BGSRNFilterArming new]; }); + return arming; +} /** The JS round trip. Off is the native pass-through, which stays installed. */ static BOOL BGSRNLogFilterUserEnabled = NO; static NSMutableDictionary *BGSRNLogFilterPendingTable; @@ -612,7 +640,10 @@ static void BGSRNInstallLogEventFilter(void) { return; } BugseeModule *module = BGSRNLogFilterModule; - if (module == nil) { + // A reload kept the user's filter on but the new runtime has not called + // setLogFilter yet: drop, never record unfiltered and never emit to a + // module whose JS is not listening. + if (module == nil || ![BGSRNLogFilterArming() isArmedFor:module]) { decision(nil); return; } @@ -657,6 +688,12 @@ @implementation BGSRNBreadcrumbFilterPending static __weak BugseeModule *BGSRNBreadcrumbFilterModule = nil; static BOOL BGSRNBreadcrumbFilterInstalled = NO; +static BGSRNFilterArming *BGSRNBreadcrumbFilterArming(void) { + static BGSRNFilterArming *arming; + static dispatch_once_t once; + dispatch_once(&once, ^{ arming = [BGSRNFilterArming new]; }); + return arming; +} /// Bumped on every enable and every disable. A disable queued on main nils /// the filter only when this is still the generation it captured, so a later /// enable is not wiped by a nil that was already queued. @@ -1054,14 +1091,16 @@ - (void)attachToBridges { // After the emitter exists. A network filter installed earlier reads this // pointer when an event arrives; it is nil until then, and a nil module // drops the event. - BGSRNNetworkFilterModule = self; - // After the emitter exists. The native log filter is already installed. - // While a user callback is on, a nil module drops the line. With no user - // callback the same filter returns the line. - BGSRNLogFilterModule = self; - // Same lifetime as the log filter: nil until the emitter exists, and a nil - // module drops the crumb. - BGSRNBreadcrumbFilterModule = self; + @synchronized (BGSRNFilterOwnerLock()) { + BGSRNNetworkFilterModule = self; + // After the emitter exists. The native log filter is already installed. + // While a user callback is on, a nil module drops the line. With no user + // callback the same filter returns the line. + BGSRNLogFilterModule = self; + // Same lifetime as the log filter: nil until the emitter exists, and a nil + // module drops the crumb. + BGSRNBreadcrumbFilterModule = self; + } } /// Identity-checked inside the bus: a reload can construct and attach the NEW @@ -1087,20 +1126,26 @@ - (void)invalidate { // Drop this module's unanswered network events. A reload may already have // attached the new module; only clear the pointer when it is still us. BGSRNDropNetworkFiltersOwnedBy(self); - if (BGSRNNetworkFilterModule == self) { - BGSRNNetworkFilterModule = nil; + @synchronized (BGSRNFilterOwnerLock()) { + if (BGSRNNetworkFilterModule == self) { + BGSRNNetworkFilterModule = nil; + } } // Drop this module's unanswered log lines. A reload may already have // attached the new module; only clear the pointer when it is still us. BGSRNDropLogFiltersOwnedBy(self); - if (BGSRNLogFilterModule == self) { - BGSRNLogFilterModule = nil; + @synchronized (BGSRNFilterOwnerLock()) { + if (BGSRNLogFilterModule == self) { + BGSRNLogFilterModule = nil; + } } // Drop this module's unanswered crumbs. A reload may already have attached // the new module; only clear the pointer when it is still us. BGSRNDropBreadcrumbFiltersOwnedBy(self); - if (BGSRNBreadcrumbFilterModule == self) { - BGSRNBreadcrumbFilterModule = nil; + @synchronized (BGSRNFilterOwnerLock()) { + if (BGSRNBreadcrumbFilterModule == self) { + BGSRNBreadcrumbFilterModule = nil; + } } // The next runtime cannot know these handles, so nothing would ever finish // them and the SDK would keep the transactions open. Cancel what is still @@ -1651,6 +1696,18 @@ - (NSNumber *)noteConsoleEcho:(NSString *)message { /// install landed, with its url, headers and body unfiltered. The SDK setter /// only takes a lock, so it is safe on the JS thread. - (NSNumber *)setNetworkFilterEnabled:(BOOL)enabled { + // A reload can attach the new module before this one is invalidated. A + // superseded module must neither arm nor disable the process-wide filter. + BOOL owner = NO; + @synchronized (BGSRNFilterOwnerLock()) { + owner = BGSRNNetworkFilterModule == self; + if (owner && enabled) { + [BGSRNNetworkFilterArming() armFor:self]; + } + } + if (!owner) { + return @YES; + } if (enabled) { @synchronized (BGSRNNetworkFilterLock()) { if (BGSRNNetworkFilterInstalled) { @@ -1667,7 +1724,10 @@ - (NSNumber *)setNetworkFilterEnabled:(BOOL)enabled { return; } BugseeModule *module = BGSRNNetworkFilterModule; - if (module == nil) { + // After a reload the new module's JS has not installed its filter: + // drop now rather than wait out the deadline for a runtime that is + // not listening. + if (module == nil || ![BGSRNNetworkFilterArming() isArmedFor:module]) { decision(nil); return; } @@ -1698,10 +1758,18 @@ - (NSNumber *)setNetworkFilterEnabled:(BOOL)enabled { }]; return @YES; } - @synchronized (BGSRNNetworkFilterLock()) { - BGSRNNetworkFilterInstalled = NO; + // Ownership is checked again under the owner lock and held through the + // uninstall: a stale module that passed the check above must not remove a + // filter the replacement has since relied on. The SDK setter only takes + // locks. + @synchronized (BGSRNFilterOwnerLock()) { + if (BGSRNNetworkFilterModule == self) { + @synchronized (BGSRNNetworkFilterLock()) { + BGSRNNetworkFilterInstalled = NO; + } + [Bugsee setNetworkEventFilter:nil]; + } } - [Bugsee setNetworkEventFilter:nil]; return @YES; } @@ -1768,8 +1836,22 @@ - (void)addNetworkEvent:(NSString *)eventJson { /// console line written on the next line could be captured before a queued /// flag flip landed. The flag write and the SDK setter only take locks. - (NSNumber *)setLogFilterEnabled:(BOOL)enabled { - @synchronized (BGSRNLogFilterLock()) { - BGSRNLogFilterUserEnabled = enabled; + // A reload can attach the new module before this one is invalidated. A + // superseded module must neither arm nor disable the process-wide filter. + BOOL owner = NO; + @synchronized (BGSRNFilterOwnerLock()) { + owner = BGSRNLogFilterModule == self; + if (owner) { + if (enabled) { + [BGSRNLogFilterArming() armFor:self]; + } + @synchronized (BGSRNLogFilterLock()) { + BGSRNLogFilterUserEnabled = enabled; + } + } + } + if (!owner) { + return @YES; } BGSRNInstallLogEventFilter(); return @YES; @@ -1828,6 +1910,18 @@ - (void)replyLogFilter:(NSString *)requestId line:(NSString * _Nullable)line { /// callback, before main runs, must not have the queued nil remove the newer /// filter. - (NSNumber *)setBreadcrumbFilterEnabled:(BOOL)enabled { + // A reload can attach the new module before this one is invalidated. A + // superseded module must neither arm nor disable the process-wide filter. + BOOL owner = NO; + @synchronized (BGSRNFilterOwnerLock()) { + owner = BGSRNBreadcrumbFilterModule == self; + if (owner && enabled) { + [BGSRNBreadcrumbFilterArming() armFor:self]; + } + } + if (!owner) { + return @YES; + } if (enabled) { @synchronized (BGSRNBreadcrumbFilterLock()) { BGSRNBreadcrumbFilterGeneration += 1; @@ -1845,7 +1939,8 @@ - (NSNumber *)setBreadcrumbFilterEnabled:(BOOL)enabled { return; } BugseeModule *module = BGSRNBreadcrumbFilterModule; - if (module == nil) { + // Same reload case as the network filter. + if (module == nil || ![BGSRNBreadcrumbFilterArming() isArmedFor:module]) { decision(nil); return; } @@ -1895,12 +1990,20 @@ - (NSNumber *)setBreadcrumbFilterEnabled:(BOOL)enabled { generation = BGSRNBreadcrumbFilterGeneration; } BGSRNRunOnMain(^{ - @synchronized (BGSRNBreadcrumbFilterLock()) { - if (BGSRNBreadcrumbFilterGeneration != generation) { + // The generation does not catch a stale disable that captured it after + // the replacement's enable, so ownership is checked here too, under the + // owner lock held through the uninstall. + @synchronized (BGSRNFilterOwnerLock()) { + if (BGSRNBreadcrumbFilterModule != self) { return; } - BGSRNBreadcrumbFilterInstalled = NO; - [Bugsee setBreadcrumbFilter:nil]; + @synchronized (BGSRNBreadcrumbFilterLock()) { + if (BGSRNBreadcrumbFilterGeneration != generation) { + return; + } + BGSRNBreadcrumbFilterInstalled = NO; + [Bugsee setBreadcrumbFilter:nil]; + } } }); return @YES; diff --git a/packages/react-native/ios/Support/Sources/BugseeRNSupport/BGSRNFilterArming.m b/packages/react-native/ios/Support/Sources/BugseeRNSupport/BGSRNFilterArming.m new file mode 100644 index 00000000..334f047f --- /dev/null +++ b/packages/react-native/ios/Support/Sources/BugseeRNSupport/BGSRNFilterArming.m @@ -0,0 +1,22 @@ +#import "BGSRNFilterArming.h" + +@implementation BGSRNFilterArming { + __weak id _owner; +} + +- (void)armFor:(id)owner { + @synchronized (self) { + _owner = owner; + } +} + +- (BOOL)isArmedFor:(id)owner { + if (owner == nil) { + return NO; + } + @synchronized (self) { + return _owner == owner; + } +} + +@end diff --git a/packages/react-native/ios/Support/Sources/BugseeRNSupport/include/BGSRNFilterArming.h b/packages/react-native/ios/Support/Sources/BugseeRNSupport/include/BGSRNFilterArming.h new file mode 100644 index 00000000..1e760f42 --- /dev/null +++ b/packages/react-native/ios/Support/Sources/BugseeRNSupport/include/BGSRNFilterArming.h @@ -0,0 +1,26 @@ +#import + +NS_ASSUME_NONNULL_BEGIN + +/// Which module's JS has installed a filter. +/// +/// The native filter blocks and their flags are process statics, so they +/// outlive a JavaScript reload. The new module attaches as the sink, but its +/// JS has not subscribed, and a request emitted to it would never be +/// answered: on iOS it stays pending until process death. A filter block +/// therefore asks `isArmedFor:` with the current module and drops at once +/// when this returns NO. +/// +/// The owner is held weakly: an invalidated module that is released reads as +/// not armed. Thread-safe: the SDK calls filters from any thread. +@interface BGSRNFilterArming : NSObject + +/// `owner`'s JS has installed the filter. Replaces any earlier owner. +- (void)armFor:(id)owner; + +/// YES only when `owner` is the module that last called `armFor:`. +- (BOOL)isArmedFor:(nullable id)owner; + +@end + +NS_ASSUME_NONNULL_END diff --git a/packages/react-native/ios/Support/Tests/BugseeRNSupportTests/BGSRNFilterArmingTests.m b/packages/react-native/ios/Support/Tests/BugseeRNSupportTests/BGSRNFilterArmingTests.m new file mode 100644 index 00000000..87c8a0b4 --- /dev/null +++ b/packages/react-native/ios/Support/Tests/BugseeRNSupportTests/BGSRNFilterArmingTests.m @@ -0,0 +1,48 @@ +@import XCTest; +@import BugseeRNSupport; + +/// Pins `BGSRNFilterArming`: after a JS reload the previous module's JS +/// armed a filter, the new module's has not, and the filter must drop. +@interface BGSRNFilterArmingTests : XCTestCase +@end + +@implementation BGSRNFilterArmingTests + +- (void)testNothingIsArmedUntilJsArms { + BGSRNFilterArming *arming = [BGSRNFilterArming new]; + NSObject *module = [NSObject new]; + XCTAssertFalse([arming isArmedFor:module]); + XCTAssertFalse([arming isArmedFor:nil]); +} + +- (void)testArmedForTheModuleThatArmed { + BGSRNFilterArming *arming = [BGSRNFilterArming new]; + NSObject *module = [NSObject new]; + [arming armFor:module]; + XCTAssertTrue([arming isArmedFor:module]); +} + +- (void)testANewModuleIsNotArmedByTheOldOne { + BGSRNFilterArming *arming = [BGSRNFilterArming new]; + NSObject *oldModule = [NSObject new]; + NSObject *newModule = [NSObject new]; + [arming armFor:oldModule]; + XCTAssertFalse([arming isArmedFor:newModule]); + [arming armFor:newModule]; + XCTAssertTrue([arming isArmedFor:newModule]); + XCTAssertFalse([arming isArmedFor:oldModule]); +} + +- (void)testAReleasedOwnerIsNotArmed { + BGSRNFilterArming *arming = [BGSRNFilterArming new]; + NSObject *replacement = [NSObject new]; + @autoreleasepool { + NSObject *gone = [NSObject new]; + [arming armFor:gone]; + XCTAssertTrue([arming isArmedFor:gone]); + } + XCTAssertFalse([arming isArmedFor:replacement]); + XCTAssertFalse([arming isArmedFor:nil]); +} + +@end diff --git a/packages/react-native/src/breadcrumbs/filter.ts b/packages/react-native/src/breadcrumbs/filter.ts index fb9f2bc3..43794b9e 100644 --- a/packages/react-native/src/breadcrumbs/filter.ts +++ b/packages/react-native/src/breadcrumbs/filter.ts @@ -64,6 +64,16 @@ function claimBreadcrumbFilter(id: string | undefined): BreadcrumbFilter | undef * crumb filtered on the installing turn still has a listener. The subscription * stays: clearing the callback stops native from asking, and a later callback * must still be able to answer. + * + * The native filter lives for the whole process, but the JavaScript side that + * answers it does not. After a full reload, `DevSettings.reload()`, or an OTA + * reload (`Updates.reloadAsync()`, CodePush `restartApp()`), the new runtime + * has not installed a filter yet, so native fails closed: crumbs are dropped, not recorded. + * That lasts until `setBreadcrumbFilter` is called again, which can be the rest of the + * process. Call it unconditionally at startup, on every launch of the JS + * runtime, not lazily or conditionally (after login or consent), and keep it + * in every OTA bundle. To opt out, call `setBreadcrumbFilter(null)`, which restores + * pass-through. */ export function setBreadcrumbFilter(callback?: BreadcrumbFilter | null): void { if (callback != null && typeof callback !== 'function') { diff --git a/packages/react-native/src/logs/filter.ts b/packages/react-native/src/logs/filter.ts index f60e796e..04606b8b 100644 --- a/packages/react-native/src/logs/filter.ts +++ b/packages/react-native/src/logs/filter.ts @@ -24,10 +24,20 @@ let subscribed = false; * so lines are recorded without one. * * The JS listener is subscribed before the native filter is installed, so a - * line filtered on the installing turn still has a listener. On iOS a request - * emitted in that gap stays in the pending table until process death. The + * line filtered on the installing turn still has a listener. A request + * emitted in that gap is dropped when its deadline passes. The * subscription stays: clearing the callback stops native from asking, and a * later callback must still be able to answer. + * + * The native filter lives for the whole process, but the JavaScript side that + * answers it does not. After a full reload, `DevSettings.reload()`, or an OTA + * reload (`Updates.reloadAsync()`, CodePush `restartApp()`), the new runtime + * has not installed a filter yet, so native fails closed: lines are dropped, not recorded unfiltered. + * That lasts until `setLogFilter` is called again, which can be the rest of the + * process. Call it unconditionally at startup, on every launch of the JS + * runtime, not lazily or conditionally (after login or consent), and keep it + * in every OTA bundle. To opt out, call `setLogFilter(null)`, which restores + * pass-through. */ export function setLogFilter(callback?: LogFilter | null): void { if (callback != null && typeof callback !== 'function') { diff --git a/packages/react-native/src/network/filter.ts b/packages/react-native/src/network/filter.ts index 4205a33c..6d80089e 100644 --- a/packages/react-native/src/network/filter.ts +++ b/packages/react-native/src/network/filter.ts @@ -63,6 +63,16 @@ let subscribed = false; * The native request is subscribed to once and stays subscribed: clearing the * callback stops native from asking, and a later callback must still be able * to answer. + * + * The native filter lives for the whole process, but the JavaScript side that + * answers it does not. After a full reload, `DevSettings.reload()`, or an OTA + * reload (`Updates.reloadAsync()`, CodePush `restartApp()`), the new runtime + * has not installed a filter yet, so native fails closed: events are dropped, not recorded. + * That lasts until `setNetworkFilter` is called again, which can be the rest of the + * process. Call it unconditionally at startup, on every launch of the JS + * runtime, not lazily or conditionally (after login or consent), and keep it + * in every OTA bundle. To opt out, call `setNetworkFilter(null)`, which restores + * pass-through. */ export function setNetworkFilter(callback?: NetworkFilter | null): void { if (callback != null && typeof callback !== 'function') {