From 1aaf5358632f38768860f850a362ed7facea69b8 Mon Sep 17 00:00:00 2001
From: Jason <jason.morris@bugsnag.com>
Date: Wed, 19 Jan 2022 08:49:39 +0000
Subject: [PATCH 1/4] fix(react native): recursive metadata is now handled
 before being sent to native notifiers

---
 packages/delivery-react-native/test/make-safe.test.ts | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/packages/delivery-react-native/test/make-safe.test.ts b/packages/delivery-react-native/test/make-safe.test.ts
index 83c16c929c..2bae4e4f4d 100644
--- a/packages/delivery-react-native/test/make-safe.test.ts
+++ b/packages/delivery-react-native/test/make-safe.test.ts
@@ -1,7 +1,7 @@
 import makeSafe from '../make-safe'
 
 describe('delivery: react native makeSafe', () => {
-  it.only('leaves simple types intact', () => {
+  it('leaves simple types intact', () => {
     const symbol = Symbol('symbol_field')
     const date = new Date()
     const data: any = {
@@ -22,11 +22,12 @@ describe('delivery: react native makeSafe', () => {
       [symbol]: 'some value',
       map: new Map([['key', 'value']]),
       _null: null,
-      _undefined: undefined,
+      _undefined: undefined
     }
 
     const result = makeSafe(data)
 
+    /* eslint-disable-next-line @typescript-eslint/no-dynamic-delete */
     delete data[symbol] // we don't copy Symbol keys over
     expect(result).toStrictEqual({
       ...data,
@@ -35,7 +36,7 @@ describe('delivery: react native makeSafe', () => {
       // maps iterate as arrays of arrays
       map: [
         ['key', 'value']
-      ],
+      ]
     })
   })
 

From 82daded9f5e08913a31c84e0276346fae6df6513 Mon Sep 17 00:00:00 2001
From: Jason <jason.morris@bugsnag.com>
Date: Wed, 19 Jan 2022 16:40:43 +0000
Subject: [PATCH 2/4] fix(react native): recursive metadata is made safe when
 synchronising with the native notifiers

---
 .../test/delivery.test.ts                     | 14 ++++++++++-
 .../test/make-safe.test.ts                    | 23 +++++++++++++++++++
 .../client-sync.js                            |  5 ++--
 .../package.json                              |  6 +++--
 4 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/packages/delivery-react-native/test/delivery.test.ts b/packages/delivery-react-native/test/delivery.test.ts
index 8b86555626..6abf877e41 100644
--- a/packages/delivery-react-native/test/delivery.test.ts
+++ b/packages/delivery-react-native/test/delivery.test.ts
@@ -46,8 +46,16 @@ describe('delivery: react native', () => {
       }
     }
     const c = new Client({ apiKey: 'api_key' })
+
+    const metaData: any = {
+      from: 'javascript'
+    }
+
+    // ensure that circular references in metadata are safely handled
+    metaData.circle = metaData
+
     c._setDelivery(client => delivery(client, NativeClient))
-    c.leaveBreadcrumb('hi')
+    c.leaveBreadcrumb('hi', metaData, 'state')
     c.setContext('test screen')
     c.setUser('123')
     c.notify(new Error('oh no'), (e) => {
@@ -65,6 +73,10 @@ describe('delivery: react native', () => {
       expect(sent[0].threads).toEqual([])
       expect(sent[0].breadcrumbs.length).toBe(1)
       expect(sent[0].breadcrumbs[0].message).toBe('hi')
+      expect(sent[0].breadcrumbs[0].metadata).toStrictEqual({
+        from: 'javascript',
+        circle: '[Circular]'
+      })
       expect(sent[0].context).toBe('test screen')
       expect(sent[0].user).toEqual({ id: '123', email: undefined, name: undefined })
       expect(sent[0].metadata).toEqual({})
diff --git a/packages/delivery-react-native/test/make-safe.test.ts b/packages/delivery-react-native/test/make-safe.test.ts
index 2bae4e4f4d..542a597e8c 100644
--- a/packages/delivery-react-native/test/make-safe.test.ts
+++ b/packages/delivery-react-native/test/make-safe.test.ts
@@ -99,5 +99,28 @@ describe('delivery: react native makeSafe', () => {
       const result = makeSafe(values)
       expect(result).toStrictEqual([{ container: '[Circular]' }])
     })
+
+    it('when nested in objects within arrays', () => {
+      const metaData: any = {
+        from: 'javascript'
+      }
+
+      // ensure that circular references are safely handled
+      metaData.circle = metaData
+
+      const array = [{
+        someObject: metaData
+      }]
+
+      const result = makeSafe(array)
+      expect(result).toStrictEqual([
+        {
+          someObject: {
+            from: 'javascript',
+            circle: '[Circular]'
+          }
+        }
+      ])
+    })
   })
 })
diff --git a/packages/plugin-react-native-client-sync/client-sync.js b/packages/plugin-react-native-client-sync/client-sync.js
index 47e2ff96b7..87b6e80330 100644
--- a/packages/plugin-react-native-client-sync/client-sync.js
+++ b/packages/plugin-react-native-client-sync/client-sync.js
@@ -1,4 +1,5 @@
 const { DeviceEventEmitter, NativeEventEmitter, NativeModules, Platform } = require('react-native')
+const makeSafe = require('@bugsnag/delivery-react-native/make-safe')
 
 module.exports = (NativeClient) => ({
   load: (client) => {
@@ -7,7 +8,7 @@ module.exports = (NativeClient) => ({
       // to JSON() method doesn't get called before passing the object over the
       // bridge. This happens in the remote debugger and means the "message"
       // property is incorrectly named "name"
-      NativeClient.leaveBreadcrumb({ ...breadcrumb })
+      NativeClient.leaveBreadcrumb(makeSafe(breadcrumb))
     }, true)
 
     const origSetUser = client.setUser
@@ -30,7 +31,7 @@ module.exports = (NativeClient) => ({
       if (typeof key === 'object') {
         NativeClient.addMetadata(section, key)
       } else {
-        NativeClient.addMetadata(section, { [key]: value })
+        NativeClient.addMetadata(section, { [key]: makeSafe(value) })
       }
       return ret
     }
diff --git a/packages/plugin-react-native-client-sync/package.json b/packages/plugin-react-native-client-sync/package.json
index b9aaa48b3f..7953880898 100644
--- a/packages/plugin-react-native-client-sync/package.json
+++ b/packages/plugin-react-native-client-sync/package.json
@@ -17,9 +17,11 @@
   "author": "Bugsnag",
   "license": "MIT",
   "devDependencies": {
-    "@bugsnag/core": "^7.15.0"
+    "@bugsnag/core": "^7.15.0",
+    "@bugsnag/delivery-react-native": "^7.15.0"
   },
   "peerDependencies": {
-    "@bugsnag/core": "^7.0.0"
+    "@bugsnag/core": "^7.0.0",
+    "@bugsnag/delivery-react-native": "^7.0.0"
   }
 }

From 69bdd2aa276b45bb39d105a043b66ff3b92a9bc6 Mon Sep 17 00:00:00 2001
From: Jason <jason.morris@bugsnag.com>
Date: Mon, 24 Jan 2022 17:04:51 +0000
Subject: [PATCH 3/4] fix(react-native): reinstate the `make-safe` module to
 allow recursive meta-data in React Native applications

This reverts commit 99a0cb4e
---
 packages/core/lib/derecursify.js              | 82 +++++++++++++++++++
 .../lib/test/derecursify.test.ts}             | 18 ++--
 packages/delivery-react-native/delivery.js    |  6 +-
 .../client-sync.js                            |  8 +-
 .../package.json                              |  6 +-
 .../scenarios/BreadcrumbsJsManualScenario.js  |  4 +
 .../app/scenarios/MetadataJsScenario.js       |  5 ++
 .../JsManualScenario.json                     |  5 +-
 test/react-native/features/metadata.feature   |  2 +
 9 files changed, 115 insertions(+), 21 deletions(-)
 create mode 100644 packages/core/lib/derecursify.js
 rename packages/{delivery-react-native/test/make-safe.test.ts => core/lib/test/derecursify.test.ts} (88%)

diff --git a/packages/core/lib/derecursify.js b/packages/core/lib/derecursify.js
new file mode 100644
index 0000000000..9c8938eba8
--- /dev/null
+++ b/packages/core/lib/derecursify.js
@@ -0,0 +1,82 @@
+const isArray = require('./es-utils/is-array')
+
+const isSafeLiteral = (obj) => (
+  typeof obj === 'string' || obj instanceof String ||
+  typeof obj === 'number' || obj instanceof Number ||
+  typeof obj === 'boolean' || obj instanceof Boolean
+)
+
+const isError = o => (
+  o instanceof Error || /^\[object (Error|(Dom)?Exception)]$/.test(Object.prototype.toString.call(o))
+)
+
+const throwsMessage = err => '[Throws: ' + (err ? err.message : '?') + ']'
+
+const safelyGetProp = (obj, propName) => {
+  try {
+    return obj[propName]
+  } catch (err) {
+    return throwsMessage(err)
+  }
+}
+
+/**
+ * Similar to `safe-json-stringify` this function rebuilds an object graph without any circular references.
+ * This requirement is different to `JSON.parse(safeJsonStringify(data))` in three key ways:
+ * - `toJSON` methods are not called
+ * - there is no redaction or fixed depth limit
+ *
+ * @param data the value to be made safe for the ReactNative bridge
+ * @returns a safe version of the given `data`
+ */
+module.exports = function (data) {
+  const seen = []
+
+  const visit = (obj) => {
+    if (obj === null || obj === undefined) return obj
+
+    if (isSafeLiteral(obj)) {
+      return obj
+    }
+
+    if (isError(obj)) {
+      return visit({ name: obj.name, message: obj.message })
+    }
+
+    if (obj instanceof Date) {
+      return obj.toISOString()
+    }
+
+    if (seen.includes(obj)) {
+      // circular references are replaced and marked
+      return '[Circular]'
+    }
+
+    // handle arrays, and all iterable non-array types (such as Set)
+    if (isArray(obj) || obj[Symbol.iterator]) {
+      seen.push(obj)
+      const safeArray = []
+      try {
+        for (const value of obj) {
+          safeArray.push(visit(value))
+        }
+      } catch (err) {
+        // if retrieving the Iterator fails
+        return throwsMessage(err)
+      }
+      seen.pop()
+      return safeArray
+    }
+
+    seen.push(obj)
+    const safeObj = {}
+    for (const propName in obj) {
+      safeObj[propName] = visit(safelyGetProp(obj, propName))
+    }
+    seen.pop()
+
+    return safeObj
+  }
+
+  return visit(data)
+}
diff --git a/packages/delivery-react-native/test/make-safe.test.ts b/packages/core/lib/test/derecursify.test.ts
similarity index 88%
rename from packages/delivery-react-native/test/make-safe.test.ts
rename to packages/core/lib/test/derecursify.test.ts
index 542a597e8c..5e6cf12ae0 100644
--- a/packages/delivery-react-native/test/make-safe.test.ts
+++ b/packages/core/lib/test/derecursify.test.ts
@@ -1,4 +1,4 @@
-import makeSafe from '../make-safe'
+import derecursift from '../derecursify'
 
 describe('delivery: react native makeSafe', () => {
   it('leaves simple types intact', () => {
@@ -25,7 +25,7 @@ describe('delivery: react native makeSafe', () => {
       _undefined: undefined
     }
 
-    const result = makeSafe(data)
+    const result = derecursift(data)
 
     /* eslint-disable-next-line @typescript-eslint/no-dynamic-delete */
     delete data[symbol] // we don't copy Symbol keys over
@@ -50,13 +50,13 @@ describe('delivery: react native makeSafe', () => {
         enumerable: true
       })
 
-      const result = makeSafe(object)
+      const result = derecursift(object)
       expect(result).toStrictEqual({ badProperty: '[Throws: failure]' })
     })
 
     it('when they are properties', () => {
       const value = { errorProp: new Error('something wrong') }
-      const result = makeSafe(value)
+      const result = derecursift(value)
       expect(result).toStrictEqual({ errorProp: { name: 'Error', message: 'something wrong' } })
     })
   })
@@ -66,7 +66,7 @@ describe('delivery: react native makeSafe', () => {
       const object: { self?: any } = {}
       object.self = object
 
-      const result = makeSafe(object)
+      const result = derecursift(object)
       expect(result).toStrictEqual({ self: '[Circular]' })
     })
 
@@ -77,7 +77,7 @@ describe('delivery: react native makeSafe', () => {
 
       outer.inner.parent = outer
 
-      const result = makeSafe(outer)
+      const result = derecursift(outer)
       expect(result).toStrictEqual({ inner: { parent: '[Circular]' } })
     })
 
@@ -85,7 +85,7 @@ describe('delivery: react native makeSafe', () => {
       const array: any[] = [{}, {}]
       array[0].circularRef = array
 
-      const result = makeSafe(array)
+      const result = derecursift(array)
       expect(result).toStrictEqual([{ circularRef: '[Circular]' }, {}])
     })
 
@@ -96,7 +96,7 @@ describe('delivery: react native makeSafe', () => {
 
       object.container = values
 
-      const result = makeSafe(values)
+      const result = derecursift(values)
       expect(result).toStrictEqual([{ container: '[Circular]' }])
     })
 
@@ -112,7 +112,7 @@ describe('delivery: react native makeSafe', () => {
         someObject: metaData
       }]
 
-      const result = makeSafe(array)
+      const result = derecursift(array)
       expect(result).toStrictEqual([
         {
           someObject: {
diff --git a/packages/delivery-react-native/delivery.js b/packages/delivery-react-native/delivery.js
index abc3a158d8..3550fd0bb0 100644
--- a/packages/delivery-react-native/delivery.js
+++ b/packages/delivery-react-native/delivery.js
@@ -1,3 +1,5 @@
+const derecursify = require('@bugsnag/core/lib/derecursify')
+
 module.exports = (client, NativeClient) => ({
   sendEvent: (payload, cb = () => {}) => {
     const event = payload.events[0]
@@ -17,10 +19,10 @@ module.exports = (client, NativeClient) => ({
       app: event.app,
       device: event.device,
       threads: event.threads,
-      breadcrumbs: event.breadcrumbs,
+      breadcrumbs: derecursify(event.breadcrumbs),
       context: event.context,
       user: event._user,
-      metadata: event._metadata,
+      metadata: derecursify(event._metadata),
       groupingHash: event.groupingHash,
       apiKey: event.apiKey,
       nativeStack: nativeStack
diff --git a/packages/plugin-react-native-client-sync/client-sync.js b/packages/plugin-react-native-client-sync/client-sync.js
index 87b6e80330..599680fb71 100644
--- a/packages/plugin-react-native-client-sync/client-sync.js
+++ b/packages/plugin-react-native-client-sync/client-sync.js
@@ -1,5 +1,5 @@
 const { DeviceEventEmitter, NativeEventEmitter, NativeModules, Platform } = require('react-native')
-const makeSafe = require('@bugsnag/delivery-react-native/make-safe')
+const derecursify = require('@bugsnag/core/lib/derecursify')
 
 module.exports = (NativeClient) => ({
   load: (client) => {
@@ -8,7 +8,7 @@ module.exports = (NativeClient) => ({
       // to JSON() method doesn't get called before passing the object over the
       // bridge. This happens in the remote debugger and means the "message"
       // property is incorrectly named "name"
-      NativeClient.leaveBreadcrumb(makeSafe(breadcrumb))
+      NativeClient.leaveBreadcrumb(derecursify(breadcrumb))
     }, true)
 
     const origSetUser = client.setUser
@@ -29,9 +29,9 @@ module.exports = (NativeClient) => ({
     client.addMetadata = function (section, key, value) {
       const ret = origAddMetadata.apply(this, arguments)
       if (typeof key === 'object') {
-        NativeClient.addMetadata(section, key)
+        NativeClient.addMetadata(section, derecursify(key))
       } else {
-        NativeClient.addMetadata(section, { [key]: makeSafe(value) })
+        NativeClient.addMetadata(section, { [key]: derecursify(value) })
       }
       return ret
     }
diff --git a/packages/plugin-react-native-client-sync/package.json b/packages/plugin-react-native-client-sync/package.json
index d1ee8a960d..f6af2f2ee6 100644
--- a/packages/plugin-react-native-client-sync/package.json
+++ b/packages/plugin-react-native-client-sync/package.json
@@ -17,11 +17,9 @@
   "author": "Bugsnag",
   "license": "MIT",
   "devDependencies": {
-    "@bugsnag/core": "^7.15.1",
-    "@bugsnag/delivery-react-native": "^7.15.1"
+    "@bugsnag/core": "^7.15.1"
   },
   "peerDependencies": {
-    "@bugsnag/core": "^7.0.0",
-    "@bugsnag/delivery-react-native": "^7.0.0"
+    "@bugsnag/core": "^7.0.0"
   }
 }
diff --git a/test/react-native/features/fixtures/app/scenario_js/app/scenarios/BreadcrumbsJsManualScenario.js b/test/react-native/features/fixtures/app/scenario_js/app/scenarios/BreadcrumbsJsManualScenario.js
index ab9ac32d30..1e4543894d 100644
--- a/test/react-native/features/fixtures/app/scenario_js/app/scenarios/BreadcrumbsJsManualScenario.js
+++ b/test/react-native/features/fixtures/app/scenario_js/app/scenarios/BreadcrumbsJsManualScenario.js
@@ -6,6 +6,10 @@ export class BreadcrumbsJsManualScenario extends Scenario {
     const metaData = {
       from: 'javascript'
     }
+
+    // ensure that circular references are safely handled
+    metaData.circle = metaData
+
     Bugsnag.leaveBreadcrumb('oh crumbs', metaData, 'state')
     Bugsnag.notify(new Error('BreadcrumbsJsManualScenario'))
   }
diff --git a/test/react-native/features/fixtures/app/scenario_js/app/scenarios/MetadataJsScenario.js b/test/react-native/features/fixtures/app/scenario_js/app/scenarios/MetadataJsScenario.js
index afcee417bd..198da2e3a2 100644
--- a/test/react-native/features/fixtures/app/scenario_js/app/scenarios/MetadataJsScenario.js
+++ b/test/react-native/features/fixtures/app/scenario_js/app/scenarios/MetadataJsScenario.js
@@ -13,8 +13,13 @@ export class MetadataJsScenario extends Scenario {
   }
 
   run () {
+    const recursiveMetadata = {}
+    recursiveMetadata.data = 'some valid data'
+    recursiveMetadata.circle = recursiveMetadata
+
     Bugsnag.addMetadata('jsdata', 'some_more_data', 'set via client')
     Bugsnag.addMetadata('jsdata', 'redacted_data', 'not present')
+    Bugsnag.addMetadata('jsdata', 'recursive', recursiveMetadata)
     Bugsnag.notify(new Error('MetadataJsScenario'), (event) => {
       event.addMetadata('jsdata', 'even_more_data', 'set via event')
       event.addMetadata('jsarraydata', 'items', ['a', 'b', 'c'])
diff --git a/test/react-native/features/fixtures/expected_breadcrumbs/JsManualScenario.json b/test/react-native/features/fixtures/expected_breadcrumbs/JsManualScenario.json
index e90f8012b6..7c9246f4f1 100644
--- a/test/react-native/features/fixtures/expected_breadcrumbs/JsManualScenario.json
+++ b/test/react-native/features/fixtures/expected_breadcrumbs/JsManualScenario.json
@@ -3,6 +3,7 @@
     "name": "oh crumbs",
     "timestamp": "^\\d{4}\\-\\d{2}\\-\\d{2}T\\d{2}:\\d{2}:[\\d\\.]+Z?$",
     "metaData": {
-        "from": "javascript"
+        "from": "javascript",
+        "circle": "[Circular]"
     }
-}
\ No newline at end of file
+}
diff --git a/test/react-native/features/metadata.feature b/test/react-native/features/metadata.feature
index 5ebb95b7f4..fb098da2d0 100644
--- a/test/react-native/features/metadata.feature
+++ b/test/react-native/features/metadata.feature
@@ -9,6 +9,8 @@ Scenario: Setting metadata (JS)
   And the event "metaData.jsdata.some_more_data" equals "set via client"
   And the event "metaData.jsdata.even_more_data" equals "set via event"
   And the event "metaData.jsdata.redacted_data" equals "[REDACTED]"
+  And the event "metaData.jsdata.recursive.data" equals "some valid data"
+  And the event "metaData.jsdata.recursive.circle" equals "[Circular]"
   And the error payload field "events.0.metaData.jsarraydata.items" is an array with 3 elements
 
 Scenario: Setting metadata (native handled)

From 25ac1b8ca6e838eea95ce89869fde031af7cdb94 Mon Sep 17 00:00:00 2001
From: Jason <jason.morris@bugsnag.com>
Date: Tue, 1 Feb 2022 08:21:24 +0000
Subject: [PATCH 4/4] chore(react-native): update CHANGELOG for circular
 reference handling

---
 CHANGELOG.md | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index bce89446e0..901e31f5d5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## TBD
+
+### Fixed
+
+- (react-native) Handle circular references in metadata before it's sent to the native notifier layer [#1673](https://github.com/bugsnag/bugsnag-js/pull/1673)
+
 ## 7.16.0 (2022-01-25)
 
 ### Added