Skip to content

Commit f8401f5

Browse files
[ACS-10116] [ACA] Search page renders HTML from the Description,Title field instead of showing it as plain text (#4784)
1 parent 3c26475 commit f8401f5

File tree

2 files changed

+130
-58
lines changed

2 files changed

+130
-58
lines changed

projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.spec.ts

Lines changed: 85 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@
2424

2525
import { NodeEntry, ResultSetRowEntry } from '@alfresco/js-api';
2626
import { ComponentFixture, TestBed } from '@angular/core/testing';
27-
import { By } from '@angular/platform-browser';
2827
import { first } from 'rxjs/operators';
2928
import { AppTestingModule } from '../../../testing/app-testing.module';
3029
import { SearchResultsRowComponent } from './search-results-row.component';
3130
import { Component, Input } from '@angular/core';
31+
import { UnitTestingUtils } from '@alfresco/adf-core';
3232

3333
@Component({
3434
selector: 'aca-datatable-cell-badges',
@@ -42,6 +42,7 @@ class MockDatatableCellBadgesComponent {
4242
describe('SearchResultsRowComponent', () => {
4343
let component: SearchResultsRowComponent;
4444
let fixture: ComponentFixture<SearchResultsRowComponent>;
45+
let utils: UnitTestingUtils;
4546

4647
const nodeEntry: NodeEntry = {
4748
entry: {
@@ -59,26 +60,17 @@ describe('SearchResultsRowComponent', () => {
5960
modifiedAt: new Date(),
6061
isFile: true,
6162
name: 'Random name',
62-
properties: { 'cm:title': 'Random title', 'cm:description': 'some random description' },
63+
properties: {
64+
'cm:title': 'Random title',
65+
'cm:description': 'some random description'
66+
},
6367
search: {
6468
score: 10,
6569
highlight: [
66-
{
67-
field: 'cm:content',
68-
snippets: [`Interesting <span class='aca-highlight'>random</span> content`]
69-
},
70-
{
71-
field: 'cm:name',
72-
snippets: [`<span class='aca-highlight'>Random</span>`]
73-
},
74-
{
75-
field: 'cm:title',
76-
snippets: [`<span class='aca-highlight'>Random</span> title`]
77-
},
78-
{
79-
field: 'cm:description',
80-
snippets: [`some <span class='aca-highlight'>random</span> description`]
81-
}
70+
{ field: 'cm:content', snippets: [`Interesting <span class='aca-highlight'>random</span> content`] },
71+
{ field: 'cm:name', snippets: [`<span class='aca-highlight'>Random</span>`] },
72+
{ field: 'cm:title', snippets: [`<span class='aca-highlight'>Random</span> title`] },
73+
{ field: 'cm:description', snippets: [`some <span class='aca-highlight'>random</span> description`] }
8274
]
8375
}
8476
}
@@ -91,50 +83,96 @@ describe('SearchResultsRowComponent', () => {
9183

9284
fixture = TestBed.createComponent(SearchResultsRowComponent);
9385
component = fixture.componentInstance;
86+
utils = new UnitTestingUtils(fixture.debugElement);
9487
});
9588

89+
const getNameEl = (): HTMLSpanElement => utils.getByCSS('.aca-link.aca-crop-text').nativeElement;
90+
const getTitleEl = (): HTMLSpanElement => utils.getByDataAutomationId('search-results-entry-title').nativeElement;
91+
const getDescriptionEl = (): HTMLDivElement => utils.getByDataAutomationId('search-results-entry-description').nativeElement;
92+
const getContentEl = (): HTMLDivElement => utils.getByCSS('.aca-result-content.aca-crop-text').nativeElement;
93+
9694
it('should show the current node', () => {
9795
component.context = { row: { node: nodeEntry } };
9896
fixture.detectChanges();
9997

100-
const element = fixture.nativeElement.querySelector('div');
101-
expect(element).not.toBeNull();
98+
expect(utils.getByCSS('div')).not.toBeNull();
10299
});
103100

104101
it('should correctly parse highlights', (done) => {
105102
component.context = { row: { node: resultEntry } };
106-
component.content$
107-
.asObservable()
108-
.pipe(first())
109-
.subscribe(() => {
110-
fixture.detectChanges();
111-
112-
const nameElement: HTMLSpanElement = fixture.debugElement.query(By.css('.aca-link.aca-crop-text')).nativeElement;
113-
expect(nameElement.innerHTML).toBe('<span class="aca-highlight">Random</span>');
114-
expect(nameElement.title).toBe('Random');
115-
116-
const titleElement: HTMLSpanElement = fixture.debugElement.query(By.css('[data-automation-id="search-results-entry-title"]')).nativeElement;
117-
expect(titleElement.innerHTML).toBe(' ( <span class="aca-highlight">Random</span> title )');
118-
expect(titleElement.title).toBe('Random title');
119-
120-
const descriptionElement: HTMLDivElement = fixture.debugElement.query(
121-
By.css('[data-automation-id="search-results-entry-description"]')
122-
).nativeElement;
123-
expect(descriptionElement.innerHTML).toBe('some <span class="aca-highlight">random</span> description');
124-
expect(descriptionElement.title).toBe('some random description');
125-
126-
const contentElement: HTMLDivElement = fixture.debugElement.query(By.css('.aca-result-content.aca-crop-text')).nativeElement;
127-
expect(contentElement.innerHTML).toBe('...Interesting <span class="aca-highlight">random</span> content...');
128-
expect(contentElement.title).toBe('...Interesting random content...');
129-
done();
130-
});
103+
component.content$.pipe(first()).subscribe(() => {
104+
fixture.detectChanges();
105+
106+
expect(getNameEl().innerHTML).toBe('<span class="aca-highlight">Random</span>');
107+
expect(getNameEl().title).toBe('Random');
108+
109+
expect(getTitleEl().innerHTML).toBe(' ( <span class="aca-highlight">Random</span> title )');
110+
expect(getTitleEl().title).toBe('Random title');
111+
112+
expect(getDescriptionEl().innerHTML).toBe('some <span class="aca-highlight">random</span> description');
113+
expect(getDescriptionEl().title).toBe('some random description');
114+
115+
expect(getContentEl().innerHTML).toBe('...Interesting <span class="aca-highlight">random</span> content...');
116+
expect(getContentEl().title).toBe('...Interesting random content...');
117+
done();
118+
});
131119
fixture.detectChanges();
132120
});
133121

134122
it('should pass node to badge component', () => {
135123
component.context = { row: { node: nodeEntry } };
136-
const badgeElement = fixture.debugElement.query(By.css('aca-datatable-cell-badges'));
124+
fixture.detectChanges();
125+
126+
const badgeElement = utils.getByCSS('aca-datatable-cell-badges').componentInstance;
137127
expect(badgeElement).not.toBe(null);
138-
expect(badgeElement.componentInstance.node).toBe(component.context.node);
128+
expect(badgeElement.node).toBe(component.context.row.node);
129+
});
130+
131+
it('should escape plain < and > in values', (done) => {
132+
const customEntry: ResultSetRowEntry = {
133+
entry: { ...nodeEntry.entry, name: '2 < 5 > 3', search: { score: 5 } }
134+
} as ResultSetRowEntry;
135+
136+
component.context = { row: { node: customEntry } };
137+
component.name$.pipe(first()).subscribe(() => {
138+
fixture.detectChanges();
139+
140+
expect(getNameEl().innerHTML).toBe('2 &lt; 5 &gt; 3');
141+
expect(getNameEl().textContent).toBe('2 < 5 > 3');
142+
done();
143+
});
144+
fixture.detectChanges();
145+
});
146+
147+
it('should not render script tags as HTML', (done) => {
148+
const customEntry: ResultSetRowEntry = {
149+
entry: { ...nodeEntry.entry, name: '<script>alert("xss")</script>', search: { score: 5 } }
150+
} as ResultSetRowEntry;
151+
152+
component.context = { row: { node: customEntry } };
153+
component.name$.pipe(first()).subscribe(() => {
154+
fixture.detectChanges();
155+
156+
expect(getNameEl().innerHTML).toContain('&lt;script&gt;alert("xss")&lt;/script&gt;');
157+
expect(getNameEl().textContent).toBe('<script>alert("xss")</script>');
158+
done();
159+
});
160+
fixture.detectChanges();
161+
});
162+
163+
it('should allow highlight spans but escape other tags', (done) => {
164+
const customEntry: ResultSetRowEntry = {
165+
entry: { ...nodeEntry.entry, name: '<b><span class="aca-highlight">BoldHighlight</span></b>', search: { score: 5 } }
166+
} as ResultSetRowEntry;
167+
168+
component.context = { row: { node: customEntry } };
169+
component.name$.pipe(first()).subscribe(() => {
170+
fixture.detectChanges();
171+
172+
expect(getNameEl().innerHTML).toBe('&lt;b&gt;<span class="aca-highlight">BoldHighlight</span>&lt;/b&gt;');
173+
expect(getNameEl().textContent).toBe('<b>BoldHighlight</b>');
174+
done();
175+
});
176+
fixture.detectChanges();
139177
});
140178
});

projects/aca-content/src/lib/components/search/search-results-row/search-results-row.component.ts

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,20 @@ import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
4848
export class SearchResultsRowComponent implements OnInit {
4949
private settings = inject(AppSettingsService);
5050

51-
private readonly highlightPrefix = "<span class='aca-highlight'>";
52-
private readonly highlightPostfix = '</span>';
51+
private readonly highlightPrefix = `<span class="aca-highlight">`;
52+
private readonly highlightPostfix = `</span>`;
53+
54+
private readonly highlightOpenEscapedRegex = /&lt;span class=(['"])aca-highlight\1&gt;/g;
55+
private readonly highlightCloseEscapedRegex = /&lt;\/span&gt;/g;
56+
57+
private readonly highlightOpenRawRegex = /<span class=(['"])aca-highlight\1>/g;
58+
private readonly highlightCloseRawRegex = /<\/span>/g;
59+
60+
private readonly escapeMap: Record<string, string> = {
61+
'&': '&amp;',
62+
'<': '&lt;',
63+
'>': '&gt;'
64+
};
5365

5466
private node: NodeEntry;
5567

@@ -122,17 +134,26 @@ export class SearchResultsRowComponent implements OnInit {
122134
break;
123135
}
124136
});
125-
this.name$.next(name);
126-
this.description$.next(description);
127-
this.content$.next(content);
137+
138+
const safeName = this.sanitizeAndHighlight(name);
139+
const safeDescription = this.sanitizeAndHighlight(description);
140+
const safeContent = this.sanitizeAndHighlight(content);
141+
142+
this.name$.next(safeName);
143+
this.description$.next(safeDescription);
144+
this.content$.next(safeContent);
128145

129146
this.nameStripped = this.stripHighlighting(name);
147+
this.titleStripped = this.stripHighlighting(title);
130148
this.descriptionStripped = this.stripHighlighting(description);
131149
this.contentStripped = this.stripHighlighting(content);
132150

133-
if (title !== name) {
134-
this.title$.next(title ? ` ( ${title} )` : '');
151+
if (title && title !== name) {
152+
const safeTitle = this.sanitizeAndHighlight(` ( ${title} )`);
153+
this.title$.next(safeTitle);
135154
this.titleStripped = this.stripHighlighting(title);
155+
} else {
156+
this.title$.next('');
136157
}
137158
}
138159

@@ -149,9 +170,22 @@ export class SearchResultsRowComponent implements OnInit {
149170
this.store.dispatch(new NavigateToFolder(this.node));
150171
}
151172

152-
private stripHighlighting(highlightedContent: string): string {
153-
return highlightedContent
154-
? highlightedContent.replace(new RegExp(this.highlightPrefix, 'g'), '').replace(new RegExp(this.highlightPostfix, 'g'), '')
155-
: '';
173+
private stripHighlighting(input: string): string {
174+
if (!input) {
175+
return '';
176+
}
177+
return input.replace(this.highlightOpenRawRegex, '').replace(this.highlightCloseRawRegex, '');
178+
}
179+
180+
private sanitizeAndHighlight(value: string | null | undefined): string {
181+
if (!value) {
182+
return '';
183+
}
184+
185+
let escaped = value.replace(/[&<>]/g, (char) => this.escapeMap[char] ?? char);
186+
187+
escaped = escaped.replace(this.highlightOpenEscapedRegex, this.highlightPrefix).replace(this.highlightCloseEscapedRegex, this.highlightPostfix);
188+
189+
return escaped;
156190
}
157191
}

0 commit comments

Comments
 (0)