Filters: fail closed after a JS reload until the new runtime installs its filter

packages/react-native/android/src/main/java/com/bugsee/reactnative/BreadcrumbFilterBridge.java

@@ -119,6 +119,16 @@ static BreadcrumbFilterBridge shared() {
     private final ConcurrentHashMap<String, Pending> 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<Sink> 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<Pending> 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<Breadcrumb> callback
     ) {
         final Sink current = sink.get();
-        if (current == null) {
+        if (current == null || armedFor.get() != current) {
             drop(callback);
             return;
         }

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;
     }
 

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<Sink> 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<Pending> 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;
         }

packages/react-native/android/src/main/java/com/bugsee/reactnative/NetworkFilterBridge.java

@@ -111,6 +111,18 @@ static NetworkFilterBridge shared() {
     private final ConcurrentHashMap<String, Pending> 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<Sink> 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<Pending> 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<NetworkEvent> 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);