[Port dspace-9_x] Escape html tags in innerHTML #4737 #4883
Conversation
…rough an innerHTML attribute or not to properly escape them
…rt-4737-to-dspace-9_x
alexandrevryghem
added
bug
high priority
component: Item
alexandrevryghem
moved this to 🙋 Needs Reviewers Assigned
in DSpace Maintenance (9.x, 8.x, 7.6.x)
tdonohue
left a comment
tdonohue
left a comment
There was a problem hiding this comment.
@bram-maegerman : Thanks for this backport as well. I gave this a review today, but unfortunately I'm finding a lot of differences between this and the original #4737. The dspace-9_x and main branches are nearly identical at this time, so I'd expect fewer differences. Please take a look at the inline notes below.
I'd be glad to rereview / retest once this is updated.
| for (const inputKey of inputKeys) { | ||
| if (inputKey.includes('*')) { | ||
| const inputKeyRegex = new RegExp('^' + inputKey.replace(/\\/g, '\\\\').replace(/\./g, '\\.').replace(/\*/g, '.*') + '$'); | ||
| const inputKeyRegex = new RegExp('^' + inputKey.replace(/\./g, '\\.').replace(/\*/g, '.*') + '$'); |
There was a problem hiding this comment.
Please revert this change as it's not in the original PR. I believe it's accidentally undoing a prior bug fix.
| <span | ||
| class="item-list-authors"> | ||
| @if (item.allMetadataValues(['dc.contributor.author', 'dc.creator', 'dc.contributor.*']).length === 0) { | ||
| @if (item.allMetadataValues(['dc.contributor.author', 'dc.creator', 'dc.contributor.*'], undefined, true).length === 0) { |
There was a problem hiding this comment.
This line doesn't need to be changed as it's just checking for the existence of a value.
| </ds-truncatable-part> | ||
| } | ||
| @if (dso.hasMetadata('dc.description.abstract')) { | ||
| @if (firstMetadataValue('dc.description.abstract'); as abstract) { |
There was a problem hiding this comment.
The changes in this file are not in the original PR #4737. Are we sure they are necessary here? I'm not sure I understand why they are necessary and I believe the changes can be removed.
| <ds-truncatable-part [id]="item.id" [minLines]="1"> | ||
| (@if (item.hasMetadata('dc.publisher')) { | ||
| <span class="item-list-publisher" | ||
| [innerHTML]="item.firstMetadataValue('dc.publisher') + ', '"></span> |
There was a problem hiding this comment.
This file doesn't include all the changes in #4737. There's a missing change to line 15, where it should now use item.hasMetadata() instead of item.firstMetadataValue. There's also a missing change to this dc.publisher line because the publisher field is no longer escaped. This should use: item.firstMetadataValue('dc.publisher', undefined, true)
| <ds-truncatable-part [id]="item.id" [minLines]="1" class="item-list-abstract"> | ||
| <span [ngClass]="{'text-muted': !item.firstMetadataValue('dc.description.abstract')}" | ||
| <span [ngClass]="{'text-muted': !item.hasMetadata('dc.description.abstract')}" | ||
| [innerHTML]="(item.firstMetadataValue('dc.description.abstract')) || ('mydspace.results.no-abstract' | translate)"></span> |
There was a problem hiding this comment.
This line is not correctly updated based on #4737 either. It should use: item.firstMetadataValue('dc.description.abstract', undefined, true)
| </ds-truncatable-part> | ||
| </span> | ||
| @if (dso.firstMetadataValue('dc.description.abstract')) { | ||
| @if (firstMetadataValue('dc.description.abstract'); as abstract) { |
There was a problem hiding this comment.
There are changes to this file that are also missing from #4737. please compare this file with the version on main, as I think they should be identical.
github-project-automation
bot
moved this from 🙋 Needs Reviewers Assigned
to 👀 Under Review
in DSpace Maintenance (9.x, 8.x, 7.6.x)
3 participants
Port of #4737 by @bram-maegerman to
dspace-9_x