Skip to content

Commit b8732f8

Browse files
bram-maegermanalexandrevryghem
bram-maegerman
and
alexandrevryghem
authored
[Port dspace-8_x] Escape html tags in innerHTML #4737 (#4882)
* Refactored code to pass down whether they are injected in the code through an innerHTML attribute or not to properly escape them * 134319: Renamed injectedAsHTML to escapeHTML * 134380: added escapeMetadataHTML as an input on ThemedItemDetailPreviewFieldComponent * 134380: fixed abstract and date field not being escaped * 134380: reverted unrelated change --------- Co-authored-by: Alexandre Vryghem <[email protected]>
1 parent 9f1df0c commit b8732f8

File tree

23 files changed

+232
-180
lines changed

23 files changed

+232
-180
lines changed

‎src/app/collection-page/collection-item-mapper/collection-item-mapper.component.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ export class CollectionItemMapperComponent implements OnInit {
161161

162162
this.collectionName$ = this.collectionRD$.pipe(
163163
map((rd: RemoteData<Collection>) => {
164-
return this.dsoNameService.getName(rd.payload);
164+
return this.dsoNameService.getName(rd.payload, true);
165165
}),
166166
);
167167
this.searchOptions$ = this.searchConfigService.paginatedSearchOptions;

‎src/app/core/breadcrumbs/dso-name.service.spec.ts‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe(`DSONameService`, () => {
7878

7979
const result = service.getName(mockPerson);
8080

81-
expect((service as any).factories.Person).toHaveBeenCalledWith(mockPerson);
81+
expect((service as any).factories.Person).toHaveBeenCalledWith(mockPerson, undefined);
8282
expect(result).toBe('Bingo!');
8383
});
8484

@@ -87,7 +87,7 @@ describe(`DSONameService`, () => {
8787

8888
const result = service.getName(mockOrgUnit);
8989

90-
expect((service as any).factories.OrgUnit).toHaveBeenCalledWith(mockOrgUnit);
90+
expect((service as any).factories.OrgUnit).toHaveBeenCalledWith(mockOrgUnit, undefined);
9191
expect(result).toBe('Bingo!');
9292
});
9393

@@ -96,7 +96,7 @@ describe(`DSONameService`, () => {
9696

9797
const result = service.getName(mockEPerson);
9898

99-
expect((service as any).factories.EPerson).toHaveBeenCalledWith(mockEPerson);
99+
expect((service as any).factories.EPerson).toHaveBeenCalledWith(mockEPerson, undefined);
100100
expect(result).toBe('Bingo!');
101101
});
102102

@@ -105,7 +105,7 @@ describe(`DSONameService`, () => {
105105

106106
const result = service.getName(mockDSO);
107107

108-
expect((service as any).factories.Default).toHaveBeenCalledWith(mockDSO);
108+
expect((service as any).factories.Default).toHaveBeenCalledWith(mockDSO, undefined);
109109
expect(result).toBe('Bingo!');
110110
});
111111
});
@@ -119,9 +119,9 @@ describe(`DSONameService`, () => {
119119
it(`should return 'person.familyName, person.givenName'`, () => {
120120
const result = (service as any).factories.Person(mockPerson);
121121
expect(result).toBe(mockPersonName);
122-
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.familyName');
123-
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.givenName');
124-
expect(mockPerson.firstMetadataValue).not.toHaveBeenCalledWith('dc.title');
122+
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.familyName', undefined, undefined);
123+
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.givenName', undefined, undefined);
124+
expect(mockPerson.firstMetadataValue).not.toHaveBeenCalledWith('dc.title', undefined, undefined);
125125
});
126126
});
127127

@@ -133,9 +133,9 @@ describe(`DSONameService`, () => {
133133
it(`should return dc.title`, () => {
134134
const result = (service as any).factories.Person(mockPerson);
135135
expect(result).toBe(mockPersonName);
136-
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.familyName');
137-
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.givenName');
138-
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('dc.title');
136+
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.familyName', undefined, undefined);
137+
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('person.givenName', undefined, undefined);
138+
expect(mockPerson.firstMetadataValue).toHaveBeenCalledWith('dc.title', undefined, undefined);
139139
});
140140
});
141141
});
@@ -149,8 +149,8 @@ describe(`DSONameService`, () => {
149149
it(`should return 'eperson.firstname' and 'eperson.lastname'`, () => {
150150
const result = (service as any).factories.EPerson(mockEPerson);
151151
expect(result).toBe(mockEPersonName);
152-
expect(mockEPerson.firstMetadataValue).toHaveBeenCalledWith('eperson.firstname');
153-
expect(mockEPerson.firstMetadataValue).toHaveBeenCalledWith('eperson.lastname');
152+
expect(mockEPerson.firstMetadataValue).toHaveBeenCalledWith('eperson.firstname', undefined, undefined);
153+
expect(mockEPerson.firstMetadataValue).toHaveBeenCalledWith('eperson.lastname', undefined, undefined);
154154
});
155155
});
156156

@@ -162,8 +162,8 @@ describe(`DSONameService`, () => {
162162
it(`should return 'eperson.firstname'`, () => {
163163
const result = (service as any).factories.EPerson(mockEPersonFirst);
164164
expect(result).toBe(mockEPersonNameFirst);
165-
expect(mockEPersonFirst.firstMetadataValue).toHaveBeenCalledWith('eperson.firstname');
166-
expect(mockEPersonFirst.firstMetadataValue).toHaveBeenCalledWith('eperson.lastname');
165+
expect(mockEPersonFirst.firstMetadataValue).toHaveBeenCalledWith('eperson.firstname', undefined, undefined);
166+
expect(mockEPersonFirst.firstMetadataValue).toHaveBeenCalledWith('eperson.lastname', undefined, undefined);
167167
});
168168
});
169169
});
@@ -177,7 +177,7 @@ describe(`DSONameService`, () => {
177177
it(`should return 'organization.legalName'`, () => {
178178
const result = (service as any).factories.OrgUnit(mockOrgUnit);
179179
expect(result).toBe(mockOrgUnitName);
180-
expect(mockOrgUnit.firstMetadataValue).toHaveBeenCalledWith('organization.legalName');
180+
expect(mockOrgUnit.firstMetadataValue).toHaveBeenCalledWith('organization.legalName', undefined, undefined);
181181
});
182182
});
183183

@@ -189,7 +189,7 @@ describe(`DSONameService`, () => {
189189
it(`should return 'dc.title'`, () => {
190190
const result = (service as any).factories.Default(mockDSO);
191191
expect(result).toBe(mockDSOName);
192-
expect(mockDSO.firstMetadataValue).toHaveBeenCalledWith('dc.title');
192+
expect(mockDSO.firstMetadataValue).toHaveBeenCalledWith('dc.title', undefined, undefined);
193193
});
194194
});
195195
});

‎src/app/core/breadcrumbs/dso-name.service.ts‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ export class DSONameService {
3131
* With only two exceptions those solutions seem overkill for now.
3232
*/
3333
private readonly factories = {
34-
EPerson: (dso: DSpaceObject): string => {
35-
const firstName = dso.firstMetadataValue('eperson.firstname');
36-
const lastName = dso.firstMetadataValue('eperson.lastname');
34+
EPerson: (dso: DSpaceObject, escapeHTML?: boolean): string => {
35+
const firstName = dso.firstMetadataValue('eperson.firstname', undefined, escapeHTML);
36+
const lastName = dso.firstMetadataValue('eperson.lastname', undefined, escapeHTML);
3737
if (isEmpty(firstName) && isEmpty(lastName)) {
3838
return this.translateService.instant('dso.name.unnamed');
3939
} else if (isEmpty(firstName) || isEmpty(lastName)) {
@@ -42,32 +42,33 @@ export class DSONameService {
4242
return `${firstName} ${lastName}`;
4343
}
4444
},
45-
Person: (dso: DSpaceObject): string => {
46-
const familyName = dso.firstMetadataValue('person.familyName');
47-
const givenName = dso.firstMetadataValue('person.givenName');
45+
Person: (dso: DSpaceObject, escapeHTML?: boolean): string => {
46+
const familyName = dso.firstMetadataValue('person.familyName', undefined, escapeHTML);
47+
const givenName = dso.firstMetadataValue('person.givenName', undefined, escapeHTML);
4848
if (isEmpty(familyName) && isEmpty(givenName)) {
49-
return dso.firstMetadataValue('dc.title') || this.translateService.instant('dso.name.unnamed');
49+
return dso.firstMetadataValue('dc.title', undefined, escapeHTML) || this.translateService.instant('dso.name.unnamed');
5050
} else if (isEmpty(familyName) || isEmpty(givenName)) {
5151
return familyName || givenName;
5252
} else {
5353
return `${familyName}, ${givenName}`;
5454
}
5555
},
56-
OrgUnit: (dso: DSpaceObject): string => {
57-
return dso.firstMetadataValue('organization.legalName') || this.translateService.instant('dso.name.untitled');
56+
OrgUnit: (dso: DSpaceObject, escapeHTML?: boolean): string => {
57+
return dso.firstMetadataValue('organization.legalName', undefined, escapeHTML);
5858
},
59-
Default: (dso: DSpaceObject): string => {
59+
Default: (dso: DSpaceObject, escapeHTML?: boolean): string => {
6060
// If object doesn't have dc.title metadata use name property
61-
return dso.firstMetadataValue('dc.title') || dso.name || this.translateService.instant('dso.name.untitled');
61+
return dso.firstMetadataValue('dc.title', undefined, escapeHTML) || dso.name || this.translateService.instant('dso.name.untitled');
6262
},
6363
};
6464

6565
/**
6666
* Get the name for the given {@link DSpaceObject}
6767
*
6868
* @param dso The {@link DSpaceObject} you want a name for
69+
* @param escapeHTML Whether the HTML is used inside a `[innerHTML]` attribute
6970
*/
70-
getName(dso: DSpaceObject | undefined): string {
71+
getName(dso: DSpaceObject | undefined, escapeHTML?: boolean): string {
7172
if (dso) {
7273
const types = dso.getRenderTypes();
7374
const match = types
@@ -76,10 +77,10 @@ export class DSONameService {
7677

7778
let name;
7879
if (hasValue(match)) {
79-
name = this.factories[match](dso);
80+
name = this.factories[match](dso, escapeHTML);
8081
}
8182
if (isEmpty(name)) {
82-
name = this.factories.Default(dso);
83+
name = this.factories.Default(dso, escapeHTML);
8384
}
8485
return name;
8586
} else {
@@ -92,27 +93,28 @@ export class DSONameService {
9293
*
9394
* @param object
9495
* @param dso
96+
* @param escapeHTML Whether the HTML is used inside a `[innerHTML]` attribute
9597
*
9698
* @returns {string} html embedded hit highlight.
9799
*/
98-
getHitHighlights(object: any, dso: DSpaceObject): string {
100+
getHitHighlights(object: any, dso: DSpaceObject, escapeHTML?: boolean): string {
99101
const types = dso.getRenderTypes();
100102
const entityType = types
101103
.filter((type) => typeof type === 'string')
102104
.find((type: string) => (['Person', 'OrgUnit']).includes(type)) as string;
103105
if (entityType === 'Person') {
104-
const familyName = this.firstMetadataValue(object, dso, 'person.familyName');
105-
const givenName = this.firstMetadataValue(object, dso, 'person.givenName');
106+
const familyName = this.firstMetadataValue(object, dso, 'person.familyName', escapeHTML);
107+
const givenName = this.firstMetadataValue(object, dso, 'person.givenName', escapeHTML);
106108
if (isEmpty(familyName) && isEmpty(givenName)) {
107-
return this.firstMetadataValue(object, dso, 'dc.title') || dso.name;
109+
return this.firstMetadataValue(object, dso, 'dc.title', escapeHTML) || dso.name;
108110
} else if (isEmpty(familyName) || isEmpty(givenName)) {
109111
return familyName || givenName;
110112
}
111113
return `${familyName}, ${givenName}`;
112114
} else if (entityType === 'OrgUnit') {
113-
return this.firstMetadataValue(object, dso, 'organization.legalName') || this.translateService.instant('dso.name.untitled');
115+
return this.firstMetadataValue(object, dso, 'organization.legalName', escapeHTML);
114116
}
115-
return this.firstMetadataValue(object, dso, 'dc.title') || dso.name || this.translateService.instant('dso.name.untitled');
117+
return this.firstMetadataValue(object, dso, 'dc.title', escapeHTML) || dso.name || this.translateService.instant('dso.name.untitled');
116118
}
117119

118120
/**
@@ -121,11 +123,12 @@ export class DSONameService {
121123
* @param object
122124
* @param dso
123125
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
126+
* @param escapeHTML Whether the HTML is used inside a `[innerHTML]` attribute
124127
*
125128
* @returns {string} the first matching string value, or `undefined`.
126129
*/
127-
firstMetadataValue(object: any, dso: DSpaceObject, keyOrKeys: string | string[]): string {
128-
return Metadata.firstValue([object.hitHighlights, dso.metadata], keyOrKeys);
130+
firstMetadataValue(object: any, dso: DSpaceObject, keyOrKeys: string | string[], escapeHTML?: boolean): string {
131+
return Metadata.firstValue(dso.metadata, keyOrKeys, object.hitHighlights, undefined, escapeHTML);
129132
}
130133

131134
}

‎src/app/core/shared/dspace-object.model.ts‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -118,60 +118,64 @@ export class DSpaceObject extends ListableObject implements CacheableObject {
118118
* Gets all matching metadata in this DSpaceObject.
119119
*
120120
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
121-
* @param {MetadataValueFilter} filter The value filter to use. If unspecified, no filtering will be done.
121+
* @param {MetadataValueFilter} valueFilter The value filter to use. If unspecified, no filtering will be done.
122+
* @param escapeHTML Whether the HTML is used inside a `[innerHTML]` attribute
122123
* @returns {MetadataValue[]} the matching values or an empty array.
123124
*/
124-
allMetadata(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter): MetadataValue[] {
125-
return Metadata.all(this.metadata, keyOrKeys, valueFilter);
125+
allMetadata(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter, escapeHTML?: boolean): MetadataValue[] {
126+
return Metadata.all(this.metadata, keyOrKeys, undefined, valueFilter, escapeHTML);
126127
}
127128

128129
/**
129130
* Like [[allMetadata]], but only returns string values.
130131
*
131132
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
132-
* @param {MetadataValueFilter} filter The value filter to use. If unspecified, no filtering will be done.
133+
* @param {MetadataValueFilter} valueFilter The value filter to use. If unspecified, no filtering will be done.
134+
* @param escapeHTML Whether the HTML is used inside a `[innerHTML]` attribute
133135
* @returns {string[]} the matching string values or an empty array.
134136
*/
135-
allMetadataValues(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter): string[] {
136-
return Metadata.allValues(this.metadata, keyOrKeys, valueFilter);
137+
allMetadataValues(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter, escapeHTML?: boolean): string[] {
138+
return Metadata.allValues(this.metadata, keyOrKeys, undefined, valueFilter, escapeHTML);
137139
}
138140

139141
/**
140142
* Gets the first matching MetadataValue object in this DSpaceObject, or `undefined`.
141143
*
142144
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
143-
* @param {MetadataValueFilter} filter The value filter to use. If unspecified, no filtering will be done.
145+
* @param {MetadataValueFilter} valueFilter The value filter to use. If unspecified, no filtering will be done.
146+
* @param escapeHTML Whether the HTML is used inside a `[innerHTML]` attribute
144147
* @returns {MetadataValue} the first matching value, or `undefined`.
145148
*/
146-
firstMetadata(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter): MetadataValue {
147-
return Metadata.first(this.metadata, keyOrKeys, valueFilter);
149+
firstMetadata(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter, escapeHTML?: boolean): MetadataValue {
150+
return Metadata.first(this.metadata, keyOrKeys, undefined, valueFilter, escapeHTML);
148151
}
149152

150153
/**
151154
* Like [[firstMetadata]], but only returns a string value, or `undefined`.
152155
*
153156
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
154157
* @param {MetadataValueFilter} valueFilter The value filter to use. If unspecified, no filtering will be done.
158+
* @param escapeHTML Whether the HTML is used inside a `[innerHTML]` attribute
155159
* @returns {string} the first matching string value, or `undefined`.
156160
*/
157-
firstMetadataValue(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter): string {
158-
return Metadata.firstValue(this.metadata, keyOrKeys, valueFilter);
161+
firstMetadataValue(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter, escapeHTML?: boolean): string {
162+
return Metadata.firstValue(this.metadata, keyOrKeys, undefined, valueFilter, escapeHTML);
159163
}
160164

161165
/**
162166
* Checks for a matching metadata value in this DSpaceObject.
163167
*
164168
* @param {string|string[]} keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
165-
* @param {MetadataValueFilter} filter The value filter to use. If unspecified, no filtering will be done.
169+
* @param {MetadataValueFilter} valueFilter The value filter to use. If unspecified, no filtering will be done.
166170
* @returns {boolean} whether a match is found.
167171
*/
168172
hasMetadata(keyOrKeys: string | string[], valueFilter?: MetadataValueFilter): boolean {
169-
return Metadata.has(this.metadata, keyOrKeys, valueFilter);
173+
return Metadata.has(this.metadata, keyOrKeys, undefined, valueFilter);
170174
}
171175

172176
/**
173177
* Find metadata on a specific field and order all of them using their "place" property.
174-
* @param key
178+
* @param keyOrKeys The metadata key(s) in scope. Wildcards are supported; see [[Metadata]].
175179
*/
176180
findMetadataSortedByPlace(keyOrKeys: string | string[]): MetadataValue[] {
177181
return this.allMetadata(keyOrKeys).sort((a: MetadataValue, b: MetadataValue) => {

0 commit comments

Comments
 (0)