From f8401f55d57ef62f4368db27a20c3c2b9a3c272a Mon Sep 17 00:00:00 2001 From: dominikiwanekhyland <141320833+dominikiwanekhyland@users.noreply.github.com> Date: Thu, 18 Sep 2025 14:33:58 +0200 Subject: [PATCH] [ACS-10116] [ACA] Search page renders HTML from the Description,Title field instead of showing it as plain text (#4784) --- .../search-results-row.component.spec.ts | 124 ++++++++++++------ .../search-results-row.component.ts | 56 ++++++-- 2 files changed, 126 insertions(+), 54 deletions(-) diff --git a/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.spec.ts b/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.spec.ts index 7e5d0c180..9ef9a034f 100644 --- a/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.spec.ts +++ b/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.spec.ts @@ -24,11 +24,11 @@ import { NodeEntry, ResultSetRowEntry } from '@alfresco/js-api'; import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { By } from '@angular/platform-browser'; import { first } from 'rxjs/operators'; import { AppTestingModule } from '../../../testing/app-testing.module'; import { SearchResultsRowComponent } from './search-results-row.component'; import { Component, Input } from '@angular/core'; +import { UnitTestingUtils } from '@alfresco/adf-core'; @Component({ selector: 'aca-datatable-cell-badges', @@ -42,6 +42,7 @@ class MockDatatableCellBadgesComponent { describe('SearchResultsRowComponent', () => { let component: SearchResultsRowComponent; let fixture: ComponentFixture; + let utils: UnitTestingUtils; const nodeEntry: NodeEntry = { entry: { @@ -59,26 +60,17 @@ describe('SearchResultsRowComponent', () => { modifiedAt: new Date(), isFile: true, name: 'Random name', - properties: { 'cm:title': 'Random title', 'cm:description': 'some random description' }, + properties: { + 'cm:title': 'Random title', + 'cm:description': 'some random description' + }, search: { score: 10, highlight: [ - { - field: 'cm:content', - snippets: [`Interesting random content`] - }, - { - field: 'cm:name', - snippets: [`Random`] - }, - { - field: 'cm:title', - snippets: [`Random title`] - }, - { - field: 'cm:description', - snippets: [`some random description`] - } + { field: 'cm:content', snippets: [`Interesting random content`] }, + { field: 'cm:name', snippets: [`Random`] }, + { field: 'cm:title', snippets: [`Random title`] }, + { field: 'cm:description', snippets: [`some random description`] } ] } } @@ -91,50 +83,96 @@ describe('SearchResultsRowComponent', () => { fixture = TestBed.createComponent(SearchResultsRowComponent); component = fixture.componentInstance; + utils = new UnitTestingUtils(fixture.debugElement); }); + const getNameEl = (): HTMLSpanElement => utils.getByCSS('.aca-link.aca-crop-text').nativeElement; + const getTitleEl = (): HTMLSpanElement => utils.getByDataAutomationId('search-results-entry-title').nativeElement; + const getDescriptionEl = (): HTMLDivElement => utils.getByDataAutomationId('search-results-entry-description').nativeElement; + const getContentEl = (): HTMLDivElement => utils.getByCSS('.aca-result-content.aca-crop-text').nativeElement; + it('should show the current node', () => { component.context = { row: { node: nodeEntry } }; fixture.detectChanges(); - const element = fixture.nativeElement.querySelector('div'); - expect(element).not.toBeNull(); + expect(utils.getByCSS('div')).not.toBeNull(); }); it('should correctly parse highlights', (done) => { component.context = { row: { node: resultEntry } }; - component.content$ - .asObservable() - .pipe(first()) - .subscribe(() => { - fixture.detectChanges(); + component.content$.pipe(first()).subscribe(() => { + fixture.detectChanges(); - const nameElement: HTMLSpanElement = fixture.debugElement.query(By.css('.aca-link.aca-crop-text')).nativeElement; - expect(nameElement.innerHTML).toBe('Random'); - expect(nameElement.title).toBe('Random'); + expect(getNameEl().innerHTML).toBe('Random'); + expect(getNameEl().title).toBe('Random'); - const titleElement: HTMLSpanElement = fixture.debugElement.query(By.css('[data-automation-id="search-results-entry-title"]')).nativeElement; - expect(titleElement.innerHTML).toBe(' ( Random title )'); - expect(titleElement.title).toBe('Random title'); + expect(getTitleEl().innerHTML).toBe(' ( Random title )'); + expect(getTitleEl().title).toBe('Random title'); - const descriptionElement: HTMLDivElement = fixture.debugElement.query( - By.css('[data-automation-id="search-results-entry-description"]') - ).nativeElement; - expect(descriptionElement.innerHTML).toBe('some random description'); - expect(descriptionElement.title).toBe('some random description'); + expect(getDescriptionEl().innerHTML).toBe('some random description'); + expect(getDescriptionEl().title).toBe('some random description'); - const contentElement: HTMLDivElement = fixture.debugElement.query(By.css('.aca-result-content.aca-crop-text')).nativeElement; - expect(contentElement.innerHTML).toBe('...Interesting random content...'); - expect(contentElement.title).toBe('...Interesting random content...'); - done(); - }); + expect(getContentEl().innerHTML).toBe('...Interesting random content...'); + expect(getContentEl().title).toBe('...Interesting random content...'); + done(); + }); fixture.detectChanges(); }); it('should pass node to badge component', () => { component.context = { row: { node: nodeEntry } }; - const badgeElement = fixture.debugElement.query(By.css('aca-datatable-cell-badges')); + fixture.detectChanges(); + + const badgeElement = utils.getByCSS('aca-datatable-cell-badges').componentInstance; expect(badgeElement).not.toBe(null); - expect(badgeElement.componentInstance.node).toBe(component.context.node); + expect(badgeElement.node).toBe(component.context.row.node); + }); + + it('should escape plain < and > in values', (done) => { + const customEntry: ResultSetRowEntry = { + entry: { ...nodeEntry.entry, name: '2 < 5 > 3', search: { score: 5 } } + } as ResultSetRowEntry; + + component.context = { row: { node: customEntry } }; + component.name$.pipe(first()).subscribe(() => { + fixture.detectChanges(); + + expect(getNameEl().innerHTML).toBe('2 < 5 > 3'); + expect(getNameEl().textContent).toBe('2 < 5 > 3'); + done(); + }); + fixture.detectChanges(); + }); + + it('should not render script tags as HTML', (done) => { + const customEntry: ResultSetRowEntry = { + entry: { ...nodeEntry.entry, name: '', search: { score: 5 } } + } as ResultSetRowEntry; + + component.context = { row: { node: customEntry } }; + component.name$.pipe(first()).subscribe(() => { + fixture.detectChanges(); + + expect(getNameEl().innerHTML).toContain('<script>alert("xss")</script>'); + expect(getNameEl().textContent).toBe(''); + done(); + }); + fixture.detectChanges(); + }); + + it('should allow highlight spans but escape other tags', (done) => { + const customEntry: ResultSetRowEntry = { + entry: { ...nodeEntry.entry, name: 'BoldHighlight', search: { score: 5 } } + } as ResultSetRowEntry; + + component.context = { row: { node: customEntry } }; + component.name$.pipe(first()).subscribe(() => { + fixture.detectChanges(); + + expect(getNameEl().innerHTML).toBe('<b>BoldHighlight</b>'); + expect(getNameEl().textContent).toBe('BoldHighlight'); + done(); + }); + fixture.detectChanges(); }); }); diff --git a/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.ts b/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.ts index 11c196bfa..25857e794 100644 --- a/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.ts +++ b/projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.ts @@ -48,8 +48,20 @@ import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; export class SearchResultsRowComponent implements OnInit { private settings = inject(AppSettingsService); - private readonly highlightPrefix = ""; - private readonly highlightPostfix = ''; + private readonly highlightPrefix = ``; + private readonly highlightPostfix = ``; + + private readonly highlightOpenEscapedRegex = /<span class=(['"])aca-highlight\1>/g; + private readonly highlightCloseEscapedRegex = /<\/span>/g; + + private readonly highlightOpenRawRegex = //g; + private readonly highlightCloseRawRegex = /<\/span>/g; + + private readonly escapeMap: Record = { + '&': '&', + '<': '<', + '>': '>' + }; private node: NodeEntry; @@ -122,17 +134,26 @@ export class SearchResultsRowComponent implements OnInit { break; } }); - this.name$.next(name); - this.description$.next(description); - this.content$.next(content); + + const safeName = this.sanitizeAndHighlight(name); + const safeDescription = this.sanitizeAndHighlight(description); + const safeContent = this.sanitizeAndHighlight(content); + + this.name$.next(safeName); + this.description$.next(safeDescription); + this.content$.next(safeContent); this.nameStripped = this.stripHighlighting(name); + this.titleStripped = this.stripHighlighting(title); this.descriptionStripped = this.stripHighlighting(description); this.contentStripped = this.stripHighlighting(content); - if (title !== name) { - this.title$.next(title ? ` ( ${title} )` : ''); + if (title && title !== name) { + const safeTitle = this.sanitizeAndHighlight(` ( ${title} )`); + this.title$.next(safeTitle); this.titleStripped = this.stripHighlighting(title); + } else { + this.title$.next(''); } } @@ -149,9 +170,22 @@ export class SearchResultsRowComponent implements OnInit { this.store.dispatch(new NavigateToFolder(this.node)); } - private stripHighlighting(highlightedContent: string): string { - return highlightedContent - ? highlightedContent.replace(new RegExp(this.highlightPrefix, 'g'), '').replace(new RegExp(this.highlightPostfix, 'g'), '') - : ''; + private stripHighlighting(input: string): string { + if (!input) { + return ''; + } + return input.replace(this.highlightOpenRawRegex, '').replace(this.highlightCloseRawRegex, ''); + } + + private sanitizeAndHighlight(value: string | null | undefined): string { + if (!value) { + return ''; + } + + let escaped = value.replace(/[&<>]/g, (char) => this.escapeMap[char] ?? char); + + escaped = escaped.replace(this.highlightOpenEscapedRegex, this.highlightPrefix).replace(this.highlightCloseEscapedRegex, this.highlightPostfix); + + return escaped; } }