From 9a3c133cc2382f9e7e2037f8ecef570e480d6e24 Mon Sep 17 00:00:00 2001 From: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com> Date: Fri, 22 Dec 2023 15:03:40 +0100 Subject: [PATCH] fix(editor): Avoid sanitizing output to search node data (#8126) In search feature, output sanitization was added to support ` A bug is not considered fixed, unless a test is added to prevent it from happening again. > A feature is not complete without tests. --- cypress/e2e/5-ndv.cy.ts | 210 +++++++++++++----- .../fixtures/Test_workflow_xml_output.json | 53 +++++ .../editor-ui/src/components/RunDataJson.vue | 22 +- .../src/components/RunDataSchemaItem.vue | 21 +- .../editor-ui/src/components/RunDataTable.vue | 22 +- .../src/components/TextWithHighlights.vue | 52 +++++ .../__tests__/TextWithHIghlights.test.ts | 103 +++++++++ .../src/utils/__tests__/htmlUtils.test.ts | 41 ---- packages/editor-ui/src/utils/htmlUtils.ts | 6 - 9 files changed, 397 insertions(+), 133 deletions(-) create mode 100644 cypress/fixtures/Test_workflow_xml_output.json create mode 100644 packages/editor-ui/src/components/TextWithHighlights.vue create mode 100644 packages/editor-ui/src/components/__tests__/TextWithHIghlights.test.ts delete mode 100644 packages/editor-ui/src/utils/__tests__/htmlUtils.test.ts diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index 8089a1240733a..12410ae42225b 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -370,108 +370,129 @@ describe('NDV', () => { }); describe('floating nodes', () => { - function getFloatingNodeByPosition(position: 'inputMain' | 'outputMain' | 'outputSub'| 'inputSub') { + function getFloatingNodeByPosition( + position: 'inputMain' | 'outputMain' | 'outputSub' | 'inputSub', + ) { return cy.get(`[data-node-placement=${position}]`); } beforeEach(() => { cy.createFixtureWorkflow('Floating_Nodes.json', `Floating Nodes`); - workflowPage.getters.canvasNodes().first().dblclick() - getFloatingNodeByPosition("inputMain").should('not.exist'); - getFloatingNodeByPosition("outputMain").should('exist'); + workflowPage.getters.canvasNodes().first().dblclick(); + getFloatingNodeByPosition('inputMain').should('not.exist'); + getFloatingNodeByPosition('outputMain').should('exist'); }); it('should traverse floating nodes with mouse', () => { // Traverse 4 connected node forwards - Array.from(Array(4).keys()).forEach(i => { - getFloatingNodeByPosition("outputMain").click({ force: true}); + Array.from(Array(4).keys()).forEach((i) => { + getFloatingNodeByPosition('outputMain').click({ force: true }); ndv.getters.nodeNameContainer().should('contain', `Node ${i + 1}`); - getFloatingNodeByPosition("inputMain").should('exist'); - getFloatingNodeByPosition("outputMain").should('exist'); + getFloatingNodeByPosition('inputMain').should('exist'); + getFloatingNodeByPosition('outputMain').should('exist'); ndv.actions.close(); workflowPage.getters.selectedNodes().should('have.length', 1); - workflowPage.getters.selectedNodes().first().should('contain', `Node ${i + 1}`); + workflowPage.getters + .selectedNodes() + .first() + .should('contain', `Node ${i + 1}`); workflowPage.getters.selectedNodes().first().dblclick(); - }) + }); - getFloatingNodeByPosition("outputMain").click({ force: true}); + getFloatingNodeByPosition('outputMain').click({ force: true }); ndv.getters.nodeNameContainer().should('contain', 'Chain'); - getFloatingNodeByPosition("inputSub").should('exist'); - getFloatingNodeByPosition("inputSub").click({ force: true}); + getFloatingNodeByPosition('inputSub').should('exist'); + getFloatingNodeByPosition('inputSub').click({ force: true }); ndv.getters.nodeNameContainer().should('contain', 'Model'); - getFloatingNodeByPosition("inputSub").should('not.exist'); - getFloatingNodeByPosition("inputMain").should('not.exist'); - getFloatingNodeByPosition("outputMain").should('not.exist'); - getFloatingNodeByPosition("outputSub").should('exist'); + getFloatingNodeByPosition('inputSub').should('not.exist'); + getFloatingNodeByPosition('inputMain').should('not.exist'); + getFloatingNodeByPosition('outputMain').should('not.exist'); + getFloatingNodeByPosition('outputSub').should('exist'); ndv.actions.close(); workflowPage.getters.selectedNodes().should('have.length', 1); workflowPage.getters.selectedNodes().first().should('contain', 'Model'); workflowPage.getters.selectedNodes().first().dblclick(); - getFloatingNodeByPosition("outputSub").click({ force: true}); + getFloatingNodeByPosition('outputSub').click({ force: true }); ndv.getters.nodeNameContainer().should('contain', 'Chain'); // Traverse 4 connected node backwards - Array.from(Array(4).keys()).forEach(i => { - getFloatingNodeByPosition("inputMain").click({ force: true}); - ndv.getters.nodeNameContainer().should('contain', `Node ${4 - (i)}`); - getFloatingNodeByPosition("outputMain").should('exist'); - getFloatingNodeByPosition("inputMain").should('exist'); - }) - getFloatingNodeByPosition("inputMain").click({ force: true}); - workflowPage.getters.selectedNodes().first().should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); - getFloatingNodeByPosition("inputMain").should('not.exist'); - getFloatingNodeByPosition("inputSub").should('not.exist'); - getFloatingNodeByPosition("outputSub").should('not.exist'); + Array.from(Array(4).keys()).forEach((i) => { + getFloatingNodeByPosition('inputMain').click({ force: true }); + ndv.getters.nodeNameContainer().should('contain', `Node ${4 - i}`); + getFloatingNodeByPosition('outputMain').should('exist'); + getFloatingNodeByPosition('inputMain').should('exist'); + }); + getFloatingNodeByPosition('inputMain').click({ force: true }); + workflowPage.getters + .selectedNodes() + .first() + .should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); + getFloatingNodeByPosition('inputMain').should('not.exist'); + getFloatingNodeByPosition('inputSub').should('not.exist'); + getFloatingNodeByPosition('outputSub').should('not.exist'); ndv.actions.close(); workflowPage.getters.selectedNodes().should('have.length', 1); - workflowPage.getters.selectedNodes().first().should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); + workflowPage.getters + .selectedNodes() + .first() + .should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); }); it('should traverse floating nodes with mouse', () => { // Traverse 4 connected node forwards - Array.from(Array(4).keys()).forEach(i => { - cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowRight']) + Array.from(Array(4).keys()).forEach((i) => { + cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowRight']); ndv.getters.nodeNameContainer().should('contain', `Node ${i + 1}`); - getFloatingNodeByPosition("inputMain").should('exist'); - getFloatingNodeByPosition("outputMain").should('exist'); + getFloatingNodeByPosition('inputMain').should('exist'); + getFloatingNodeByPosition('outputMain').should('exist'); ndv.actions.close(); workflowPage.getters.selectedNodes().should('have.length', 1); - workflowPage.getters.selectedNodes().first().should('contain', `Node ${i + 1}`); + workflowPage.getters + .selectedNodes() + .first() + .should('contain', `Node ${i + 1}`); workflowPage.getters.selectedNodes().first().dblclick(); - }) + }); - cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowRight']) + cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowRight']); ndv.getters.nodeNameContainer().should('contain', 'Chain'); - getFloatingNodeByPosition("inputSub").should('exist'); - cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowDown']) + getFloatingNodeByPosition('inputSub').should('exist'); + cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowDown']); ndv.getters.nodeNameContainer().should('contain', 'Model'); - getFloatingNodeByPosition("inputSub").should('not.exist'); - getFloatingNodeByPosition("inputMain").should('not.exist'); - getFloatingNodeByPosition("outputMain").should('not.exist'); - getFloatingNodeByPosition("outputSub").should('exist'); + getFloatingNodeByPosition('inputSub').should('not.exist'); + getFloatingNodeByPosition('inputMain').should('not.exist'); + getFloatingNodeByPosition('outputMain').should('not.exist'); + getFloatingNodeByPosition('outputSub').should('exist'); ndv.actions.close(); workflowPage.getters.selectedNodes().should('have.length', 1); workflowPage.getters.selectedNodes().first().should('contain', 'Model'); workflowPage.getters.selectedNodes().first().dblclick(); - cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowUp']) + cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowUp']); ndv.getters.nodeNameContainer().should('contain', 'Chain'); // Traverse 4 connected node backwards - Array.from(Array(4).keys()).forEach(i => { - cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowLeft']) - ndv.getters.nodeNameContainer().should('contain', `Node ${4 - (i)}`); - getFloatingNodeByPosition("outputMain").should('exist'); - getFloatingNodeByPosition("inputMain").should('exist'); - }) - cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowLeft']) - workflowPage.getters.selectedNodes().first().should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); - getFloatingNodeByPosition("inputMain").should('not.exist'); - getFloatingNodeByPosition("inputSub").should('not.exist'); - getFloatingNodeByPosition("outputSub").should('not.exist'); + Array.from(Array(4).keys()).forEach((i) => { + cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowLeft']); + ndv.getters.nodeNameContainer().should('contain', `Node ${4 - i}`); + getFloatingNodeByPosition('outputMain').should('exist'); + getFloatingNodeByPosition('inputMain').should('exist'); + }); + cy.realPress(['ShiftLeft', 'Meta', 'AltLeft', 'ArrowLeft']); + workflowPage.getters + .selectedNodes() + .first() + .should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); + getFloatingNodeByPosition('inputMain').should('not.exist'); + getFloatingNodeByPosition('inputSub').should('not.exist'); + getFloatingNodeByPosition('outputSub').should('not.exist'); ndv.actions.close(); workflowPage.getters.selectedNodes().should('have.length', 1); - workflowPage.getters.selectedNodes().first().should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); + workflowPage.getters + .selectedNodes() + .first() + .should('contain', MANUAL_TRIGGER_NODE_DISPLAY_NAME); }); - }) + }); + it('should show node name and version in settings', () => { cy.createFixtureWorkflow('Test_workflow_ndv_version.json', `NDV test version ${uuid()}`); @@ -490,17 +511,88 @@ describe('NDV', () => { ndv.getters.nodeVersion().should('have.text', 'Function node version 1 (Deprecated)'); ndv.actions.close(); }); + + it('Should render xml and html tags as strings and can search', () => { + cy.createFixtureWorkflow('Test_workflow_xml_output.json', `test`); + + workflowPage.actions.executeWorkflow(); + + workflowPage.actions.openNode('Edit Fields'); + + ndv.getters.outputDisplayMode().find('[class*=active]').should('contain', 'Table'); + + ndv.getters + .outputTableRow(1) + .should('include.text', ' '); + + cy.document().trigger('keyup', { key: '/' }); + ndv.getters.searchInput().filter(':focus').type(' Introduction to XML John Doe 2020 1234567890 Data Science Basics Jane Smith 2019 0987654321 Programming in Python Bob Johnson 2021 5432109876 "}]', + ); + ndv.getters.outputDataContainer().find('mark').should('have.text', ' span') + .should('include.text', ''); + }); + + it('should properly show node execution indicator', () => { + workflowPage.actions.addInitialNodeToCanvas('Code'); + workflowPage.actions.openNode('Code'); + // Should not show run info before execution + ndv.getters.nodeRunSuccessIndicator().should('not.exist'); + ndv.getters.nodeRunErrorIndicator().should('not.exist'); + ndv.getters.nodeExecuteButton().click(); + ndv.getters.nodeRunSuccessIndicator().should('exist'); + }); + + it('should properly show node execution indicator for multiple nodes', () => { + workflowPage.actions.addInitialNodeToCanvas('Code'); + workflowPage.actions.openNode('Code'); + ndv.actions.typeIntoParameterInput('jsCode', 'testets'); + ndv.getters.backToCanvas().click(); + workflowPage.actions.executeWorkflow(); + // Manual tigger node should show success indicator + workflowPage.actions.openNode('When clicking "Execute Workflow"'); + ndv.getters.nodeRunSuccessIndicator().should('exist'); + // Code node should show error + ndv.getters.backToCanvas().click(); + workflowPage.actions.openNode('Code'); + ndv.getters.nodeRunErrorIndicator().should('exist'); + }); + it('Should handle mismatched option attributes', () => { - workflowPage.actions.addInitialNodeToCanvas('LDAP', { keepNdvOpen: true, action: 'Create a new entry' }); + workflowPage.actions.addInitialNodeToCanvas('LDAP', { + keepNdvOpen: true, + action: 'Create a new entry', + }); // Add some attributes in Create operation cy.getByTestId('parameter-item').contains('Add Attributes').click(); ndv.actions.changeNodeOperation('Update'); // Attributes should be empty after operation change cy.getByTestId('parameter-item').contains('Currently no items exist').should('exist'); }); + it('Should keep RLC values after operation change', () => { const TEST_DOC_ID = '1111'; - workflowPage.actions.addInitialNodeToCanvas('Google Sheets', { keepNdvOpen: true, action: 'Append row in sheet' }); + workflowPage.actions.addInitialNodeToCanvas('Google Sheets', { + keepNdvOpen: true, + action: 'Append row in sheet', + }); ndv.actions.setRLCValue('documentId', TEST_DOC_ID); ndv.actions.changeNodeOperation('Update Row'); ndv.getters.resourceLocatorInput('documentId').find('input').should('have.value', TEST_DOC_ID); diff --git a/cypress/fixtures/Test_workflow_xml_output.json b/cypress/fixtures/Test_workflow_xml_output.json new file mode 100644 index 0000000000000..b8422c101e7c7 --- /dev/null +++ b/cypress/fixtures/Test_workflow_xml_output.json @@ -0,0 +1,53 @@ +{ + "meta": { + "instanceId": "2d1cf27f75b18bb9e146336f791c37884f4fc7ddb97c2def27c0444d106778bf" + }, + "nodes": [ + { + "parameters": {}, + "id": "8108d313-8b03-4aa4-963d-cd1c0fe8f85c", + "name": "When clicking \"Execute Workflow\"", + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [ + 420, + 220 + ] + }, + { + "parameters": { + "fields": { + "values": [ + { + "name": "body", + "stringValue": " Introduction to XML John Doe 2020 1234567890 Data Science Basics Jane Smith 2019 0987654321 Programming in Python Bob Johnson 2021 5432109876 " + } + ] + }, + "options": {} + }, + "id": "45888152-7c5f-4d88-9039-660c594da084", + "name": "Edit Fields", + "type": "n8n-nodes-base.set", + "typeVersion": 3.2, + "position": [ + 640, + 220 + ] + } + ], + "connections": { + "When clicking \"Execute Workflow\"": { + "main": [ + [ + { + "node": "Edit Fields", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": {} + } \ No newline at end of file diff --git a/packages/editor-ui/src/components/RunDataJson.vue b/packages/editor-ui/src/components/RunDataJson.vue index b46e95fae61a1..42d05866d3d36 100644 --- a/packages/editor-ui/src/components/RunDataJson.vue +++ b/packages/editor-ui/src/components/RunDataJson.vue @@ -33,7 +33,9 @@ @update:selectedValue="selectedJsonPath = $event" > @@ -76,7 +82,6 @@ import type { IDataObject, INodeExecutionData } from 'n8n-workflow'; import Draggable from '@/components/Draggable.vue'; import { executionDataToJson } from '@/utils/nodeTypesUtils'; import { isString } from '@/utils/typeGuards'; -import { highlightText, sanitizeHtml } from '@/utils/htmlUtils'; import { shorten } from '@/utils/typesUtils'; import type { INodeUi } from '@/Interface'; import { mapStores } from 'pinia'; @@ -86,6 +91,7 @@ import { getMappedExpression } from '@/utils/mappingUtils'; import { useWorkflowsStore } from '@/stores/workflows.store'; import { nonExistingJsonPath } from '@/constants'; import { useExternalHooks } from '@/composables/useExternalHooks'; +import TextWithHighlights from './TextWithHighlights.vue'; const RunDataJsonActions = defineAsyncComponent( async () => import('@/components/RunDataJsonActions.vue'), @@ -98,6 +104,7 @@ export default defineComponent({ Draggable, RunDataJsonActions, MappingPill, + TextWithHighlights, }, props: { editMode: { @@ -202,9 +209,6 @@ export default defineComponent({ getListItemName(path: string): string { return path.replace(/^(\["?\d"?]\.?)/g, ''); }, - highlightSearchTerm(value: string): string { - return sanitizeHtml(highlightText(this.getContent(value), this.search)); - }, }, }); diff --git a/packages/editor-ui/src/components/RunDataSchemaItem.vue b/packages/editor-ui/src/components/RunDataSchemaItem.vue index a4a496bd07299..fe47770c897db 100644 --- a/packages/editor-ui/src/components/RunDataSchemaItem.vue +++ b/packages/editor-ui/src/components/RunDataSchemaItem.vue @@ -1,10 +1,10 @@ + + diff --git a/packages/editor-ui/src/components/__tests__/TextWithHIghlights.test.ts b/packages/editor-ui/src/components/__tests__/TextWithHIghlights.test.ts new file mode 100644 index 0000000000000..555ae1dae879b --- /dev/null +++ b/packages/editor-ui/src/components/__tests__/TextWithHIghlights.test.ts @@ -0,0 +1,103 @@ +import { shallowMount } from '@vue/test-utils'; +import TextWithHighlights from '@/components/TextWithHighlights.vue'; + +describe('TextWithHighlights', () => { + it('highlights the search text in the content', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + content: 'Test content', + search: 'Test', + }, + }); + + expect(wrapper.html()).toContain('Test'); + expect(wrapper.html()).toContain(' content'); + }); + + it('renders correctly when search is not set', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + content: 'Test content', + }, + }); + + expect(wrapper.html()).toEqual('Test content'); + expect(wrapper.html()).not.toContain(''); + }); + + it('renders correctly numbers when search is not set', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + content: 1, + }, + }); + + expect(wrapper.html()).toEqual('1'); + expect(wrapper.html()).not.toContain(''); + }); + + it('renders correctly objects when search is not set', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + content: { hello: 'world' }, + }, + }); + + expect(wrapper.html()).toEqual('{\n "hello": "world"\n}'); + expect(wrapper.html()).not.toContain(''); + }); + + it('renders correctly objects ignoring search', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + content: { hello: 'world' }, + search: 'yo', + }, + }); + + expect(wrapper.html()).toEqual('{\n "hello": "world"\n}'); + expect(wrapper.html()).not.toContain(''); + }); + + it('highlights the search text in middle of the content', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + content: 'Test content hello world', + search: 'con', + }, + }); + + expect(wrapper.html()).toEqual( + 'Test content hello world', + ); + }); + + it('handles special regex characters in search correctly', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + content: 'Test content (hello) world', + search: '(hello)', + }, + }); + + expect(wrapper.html()).toEqual( + 'Test content (hello) world', + ); + }); + + it('searches for special regex characters correctly', () => { + const wrapper = shallowMount(TextWithHighlights, { + props: { + // eslint-disable-next-line n8n-local-rules/no-interpolation-in-regular-string + content: 'Test content ()^${}[] world', + // eslint-disable-next-line n8n-local-rules/no-interpolation-in-regular-string + search: '()^${}[]', + }, + }); + + expect(wrapper.html()).toEqual( + // eslint-disable-next-line n8n-local-rules/no-interpolation-in-regular-string + 'Test content ()^${}[] world', + ); + }); +}); diff --git a/packages/editor-ui/src/utils/__tests__/htmlUtils.test.ts b/packages/editor-ui/src/utils/__tests__/htmlUtils.test.ts deleted file mode 100644 index 096b421b95d56..0000000000000 --- a/packages/editor-ui/src/utils/__tests__/htmlUtils.test.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { highlightText } from '@/utils/htmlUtils'; - -describe('highlightText', () => { - it('should return original text if search parameter is an empty string', () => { - const text = 'some text'; - const result = highlightText(text); - expect(result).toBe(text); - }); - - it('should return original text if it is an empty string', () => { - const text = ''; - const result = highlightText(text, 'search'); - expect(result).toBe(text); - }); - - it('should escape special characters in the search string', () => { - const text = 'some text [example]'; - const result = highlightText(text, '[example]'); - expect(result).toBe('some text [example]'); - }); - - it('should escape other special characters in the search string', () => { - const text = 'phone number: +123-456-7890'; - const result = highlightText(text, '+123-456-7890'); - expect(result).toBe('phone number: +123-456-7890'); - }); - - it('should highlight occurrences of the search string in text', () => { - const text = 'example text example'; - const result = highlightText(text, 'example'); - expect(result).toBe( - 'example text example', - ); - }); - - it('should return original text if the search string is not found', () => { - const text = 'some text'; - const result = highlightText(text, 'notfound'); - expect(result).toBe(text); - }); -}); diff --git a/packages/editor-ui/src/utils/htmlUtils.ts b/packages/editor-ui/src/utils/htmlUtils.ts index e1c39bbbd88c5..48a6794e65c0f 100644 --- a/packages/editor-ui/src/utils/htmlUtils.ts +++ b/packages/editor-ui/src/utils/htmlUtils.ts @@ -63,9 +63,3 @@ export const getBannerRowHeight = async (): Promise => { }, 0); }); }; - -export const highlightText = (text: string, search = ''): string => { - const pattern = search.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, '\\$&'); - const regex = new RegExp(`(${pattern})`, 'gi'); - return search ? text?.replace(regex, '$1') : text; -};