fix(ui): use div for fragment, check lastfm url for artist page (#4980)

* fix(ui): use div for fragment, check lastfm url for artist page

* use span instead of div for better compat

* fix: implement isLastFmURL utility and add tests for URL validation

---------

Co-authored-by: Deluan <deluan@navidrome.org>

ui/src/artist/ArtistExternalLink.jsx

@@ -4,7 +4,7 @@ import { IconButton, Tooltip, Link } from '@material-ui/core'
 
 import { ImLastfm2 } from 'react-icons/im'
 import MusicBrainz from '../icons/MusicBrainz'
-import { intersperse } from '../utils'
+import { intersperse, isLastFmURL } from '../utils'
 import config from '../config'
 import { makeStyles } from '@material-ui/core/styles'
 
@@ -38,13 +38,13 @@ const ArtistExternalLinks = ({ artistInfo, record }) => {
   }
 
   if (config.lastFMEnabled) {
-    if (lastFMlink) {
+    if (lastFMlink && isLastFmURL(lastFMlink[2])) {
       addLink(
         lastFMlink[2],
         'message.openIn.lastfm',
         <ImLastfm2 className="lastfm-icon" />,
       )
-    } else if (artistInfo?.lastFmUrl) {
+    } else if (isLastFmURL(artistInfo?.lastFmUrl)) {
       addLink(
         artistInfo?.lastFmUrl,
         'message.openIn.lastfm',

ui/src/common/SafeHTML.jsx

@@ -1,5 +1,5 @@
 import DOMPurify from 'dompurify'
-import { Fragment, useMemo } from 'react'
+import { useMemo } from 'react'
 
 export const SafeHTML = ({ children }) => {
   const purified = useMemo(() => {
@@ -23,5 +23,5 @@ export const SafeHTML = ({ children }) => {
     return purify.sanitize(children, { ADD_ATTR: ['referrer-policy'] })
   }, [children])
 
-  return <Fragment dangerouslySetInnerHTML={{ __html: purified }} />
+  return <span dangerouslySetInnerHTML={{ __html: purified }} />
 }

ui/src/utils/urls.js

@@ -44,3 +44,16 @@ export const shareCoverUrl = (id, square) => {
 }
 
 export const docsUrl = (path) => `https://www.navidrome.org${path}`
+
+export const isLastFmURL = (url) => {
+  try {
+    const parsed = new URL(url)
+    return (
+      (parsed.protocol === 'http:' || parsed.protocol === 'https:') &&
+      (parsed.hostname === 'last.fm' || parsed.hostname.endsWith('.last.fm')) &&
+      parsed.pathname.startsWith('/music/')
+    )
+  } catch (e) {
+    return false
+  }
+}

ui/src/utils/urls.test.js

@@ -0,0 +1,25 @@
+import { isLastFmURL } from './urls'
+
+describe('isLastFmURL', () => {
+  it('returns true for valid Last.fm music URLs', () => {
+    expect(isLastFmURL('https://last.fm/music/The+Beatles')).toBe(true)
+    expect(isLastFmURL('http://last.fm/music/Radiohead')).toBe(true)
+    expect(isLastFmURL('https://www.last.fm/music/Daft+Punk')).toBe(true)
+  })
+
+  it('returns false for non-http(s) protocols (XSS prevention)', () => {
+    expect(isLastFmURL('javascript:alert(1)//last.fm/music/')).toBe(false)
+    expect(isLastFmURL('data:text/html,<script>//last.fm/music/')).toBe(false)
+  })
+
+  it('returns false for non-last.fm domains', () => {
+    expect(isLastFmURL('https://example.com/?q=last.fm/music/')).toBe(false)
+    expect(isLastFmURL('https://fake-last.fm/music/Artist')).toBe(false)
+  })
+
+  it('returns false for invalid paths or inputs', () => {
+    expect(isLastFmURL('https://last.fm/user/someone')).toBe(false)
+    expect(isLastFmURL(null)).toBe(false)
+    expect(isLastFmURL('not-a-url')).toBe(false)
+  })
+})