fix: Ignore reserved attribute names on UserActions tied to the window (

#1261)
newrelic · Dec 12, 2024 · 8410dbe · 8410dbe
1 parent cdd45fb
commit 8410dbe
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 5 deletions.
diff --git a/src/features/generic_events/aggregate/index.js b/src/features/generic_events/aggregate/index.js
@@ -74,11 +74,20 @@ export class Aggregate extends AggregateBase {
                 rageClick: aggregatedUserAction.rageClick,
                 target: aggregatedUserAction.selectorPath,
                 ...(isIFrameWindow(window) && { iframe: true }),
-                ...(target?.id && { targetId: target.id }),
-                ...(target?.tagName && { targetTag: target.tagName }),
-                ...(target?.type && { targetType: target.type }),
-                ...(target?.className && { targetClass: target.className })
+                ...(canTrustTargetAttribute('id') && { targetId: target.id }),
+                ...(canTrustTargetAttribute('tagName') && { targetTag: target.tagName }),
+                ...(canTrustTargetAttribute('type') && { targetType: target.type }),
+                ...(canTrustTargetAttribute('className') && { targetClass: target.className })
               })
+
+              /**
+               * Only trust attributes that exist on HTML element targets, which excludes the window and the document targets
+               * @param {string} attribute The attribute to check for on the target element
+               * @returns {boolean} Whether the target element has the attribute and can be trusted
+               */
+              function canTrustTargetAttribute (attribute) {
+                return !!(aggregatedUserAction.selectorPath !== 'window' && aggregatedUserAction.selectorPath !== 'document' && target instanceof HTMLElement && target?.[attribute])
+              }
             }
           } catch (e) {
             // do nothing for now

diff --git a/tests/assets/user-actions-modified-window.html b/tests/assets/user-actions-modified-window.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<!--
+  Copyright 2020 New Relic Corporation.
+  PDX-License-Identifier: Apache-2.0
+-->
+<html>
+  <head>
+    <title>User Action Test</title>
+    {init} {config} {loader}
+  </head>
+  <body style="height: 200vh; overflow: scroll;">
+		<button id="pay-btn" class="btn-cart-add flex-grow container" type="submit">Create click user action</button>
+		<input type="text" id="textbox"/>
+    <script>
+      // having a type attribute on the window should be ignored by the collected UserActions
+      window.type = 'test'
+    </script>
+  </body>
+</html>
diff --git a/tests/specs/ins/harvesting.e2e.js b/tests/specs/ins/harvesting.e2e.js
@@ -87,6 +87,31 @@ describe('ins harvesting', () => {
     expect(clickUAs[1].actionMs).toEqual(expect.stringMatching(/^\[\d+(,\d+){4}\]$/))
   })
 
+  // firefox generates a focus event when the page loads, which makes testing this easier
+  it.withBrowsersMatching(onlyFirefox)('should ignore window attributes on UserAction blur and focus', async () => {
+    const testUrl = await browser.testHandle.assetURL('user-actions-modified-window.html', { init: { user_actions: { enabled: true } } })
+    await browser.url(testUrl).then(() => browser.waitForAgentLoad())
+
+    const [insHarvests] = await Promise.all([
+      insightsCapture.waitForResult({ timeout: 7500 }),
+      $('#pay-btn').click().then(async () => {
+        // rage click
+        await browser.execute(function () {
+          for (let i = 0; i < 5; i++) {
+            document.querySelector('#textbox').click()
+          }
+        })
+        // stop aggregating textbox clicks
+        await $('body').click()
+      })
+    ])
+
+    const userActionsHarvest = insHarvests.flatMap(harvest => harvest.request.body.ins) // firefox sends a window focus event on load, so we may end up with 2 harvests
+    const focusBlurUAs = userActionsHarvest.filter(ua => ua.action === 'focus' || ua.action === 'blur')
+    expect(focusBlurUAs.length).toBeGreaterThan(0)
+    expect(focusBlurUAs[0].targetType).toBeUndefined()
+  })
+
   it('should detect iframes on UserActions if agent is running inside iframe', async () => {
     const testUrl = await browser.testHandle.assetURL('iframe/same-origin.html', getInsInit({ user_actions: { enabled: true } }))
     await browser.url(testUrl).then(() => browser.pause(2000))
@@ -324,7 +349,6 @@ describe('ins harvesting', () => {
     }
 
     checkAllTested () {
-      console.log(this.typesToTest)
       return Object.entries(this.typesToTest).forEach(([type, { tested }]) => {
         expect(tested).toEqual(this.expectedTypes.includes(type))
       })