1
0
Fork 0

fix missing title attribute on <iframe> tag suggested for embedding (#3901)

* title attribute is missing on <iframe> tag suggested for embedding #3861

* fix #3901

* fix: escapeHTML #3901

* fix: playlist title instead of video title #3901

* fix #3901

* assign title directly #3901
This commit is contained in:
Thavarasa Prasanth 2021-03-31 08:32:05 +02:00 committed by GitHub
parent 47099aba46
commit 4097c6d66c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 48 additions and 35 deletions

View file

@ -164,7 +164,8 @@ export class VideoBlockListComponent extends RestTable implements OnInit, AfterV
baseUrl: `${environment.originServerUrl}/videos/embed/${entry.video.uuid}`, baseUrl: `${environment.originServerUrl}/videos/embed/${entry.video.uuid}`,
title: false, title: false,
warningTitle: false warningTitle: false
}) }),
entry.video.name
) )
} }

View file

@ -815,6 +815,7 @@ export class VideoWatchComponent implements OnInit, OnDestroy {
? this.videoService.getVideoViewUrl(video.uuid) ? this.videoService.getVideoViewUrl(video.uuid)
: null, : null,
embedUrl: video.embedUrl, embedUrl: video.embedUrl,
embedTitle: video.name,
isLive: video.isLive, isLive: video.isLive,

View file

@ -117,7 +117,8 @@ export class AbuseListTableComponent extends RestTable implements OnInit, AfterV
warningTitle: false, warningTitle: false,
startTime: abuse.video.startAt, startTime: abuse.video.startAt,
stopTime: abuse.video.endAt stopTime: abuse.video.endAt
}) }),
abuse.video.name
) )
} }

View file

@ -61,7 +61,8 @@ export class VideoReportComponent extends FormReactive implements OnInit {
baseUrl: this.video.embedUrl, baseUrl: this.video.embedUrl,
title: false, title: false,
warningTitle: false warningTitle: false
}) }),
this.video.name
) )
) )
} }

View file

@ -86,14 +86,14 @@ export class VideoShareComponent {
const options = this.getVideoOptions(this.video.embedUrl) const options = this.getVideoOptions(this.video.embedUrl)
const embedUrl = buildVideoLink(options) const embedUrl = buildVideoLink(options)
return buildVideoOrPlaylistEmbed(embedUrl) return buildVideoOrPlaylistEmbed(embedUrl, this.video.name)
} }
getPlaylistIframeCode () { getPlaylistIframeCode () {
const options = this.getPlaylistOptions(this.playlist.embedUrl) const options = this.getPlaylistOptions(this.playlist.embedUrl)
const embedUrl = buildPlaylistLink(options) const embedUrl = buildPlaylistLink(options)
return buildVideoOrPlaylistEmbed(embedUrl) return buildVideoOrPlaylistEmbed(embedUrl, this.playlist.displayName)
} }
getVideoUrl () { getVideoUrl () {

View file

@ -98,6 +98,7 @@ export interface CommonOptions extends CustomizationOptions {
videoViewUrl: string videoViewUrl: string
embedUrl: string embedUrl: string
embedTitle: string
isLive: boolean isLive: boolean
@ -165,7 +166,7 @@ export class PeertubePlayerManager {
PeertubePlayerManager.alreadyPlayed = true PeertubePlayerManager.alreadyPlayed = true
}) })
self.addContextMenu(mode, player, options.common.embedUrl) self.addContextMenu(mode, player, options.common.embedUrl, options.common.embedTitle)
player.bezels() player.bezels()
@ -203,7 +204,7 @@ export class PeertubePlayerManager {
videojs(newVideoElement, videojsOptions, function (this: videojs.Player) { videojs(newVideoElement, videojsOptions, function (this: videojs.Player) {
const player = this const player = this
self.addContextMenu(mode, player, options.common.embedUrl) self.addContextMenu(mode, player, options.common.embedUrl, options.common.embedTitle)
PeertubePlayerManager.onPlayerChange(player) PeertubePlayerManager.onPlayerChange(player)
}) })
@ -492,7 +493,7 @@ export class PeertubePlayerManager {
return children return children
} }
private static addContextMenu (mode: PlayerMode, player: videojs.Player, videoEmbedUrl: string) { private static addContextMenu (mode: PlayerMode, player: videojs.Player, videoEmbedUrl: string, videoEmbedTitle: string) {
const content = [ const content = [
{ {
label: player.localize('Copy the video URL'), label: player.localize('Copy the video URL'),
@ -509,7 +510,7 @@ export class PeertubePlayerManager {
{ {
label: player.localize('Copy embed code'), label: player.localize('Copy embed code'),
listener: () => { listener: () => {
copyToClipboard(buildVideoOrPlaylistEmbed(videoEmbedUrl)) copyToClipboard(buildVideoOrPlaylistEmbed(videoEmbedUrl, videoEmbedTitle))
} }
} }
] ]

View file

@ -1,4 +1,5 @@
import { VideoFile } from '@shared/models' import { VideoFile } from '@shared/models'
import { escapeHTML } from '@shared/core-utils/renderer'
function toTitleCase (str: string) { function toTitleCase (str: string) {
return str.charAt(0).toUpperCase() + str.slice(1) return str.charAt(0).toUpperCase() + str.slice(1)
@ -170,9 +171,11 @@ function secondsToTime (seconds: number, full = false, symbol?: string) {
return time return time
} }
function buildVideoOrPlaylistEmbed (embedUrl: string) { function buildVideoOrPlaylistEmbed (embedUrl: string, embedTitle: string) {
const title = escapeHTML(embedTitle)
return '<iframe width="560" height="315" ' + return '<iframe width="560" height="315" ' +
'sandbox="allow-same-origin allow-scripts allow-popups" ' + 'sandbox="allow-same-origin allow-scripts allow-popups" ' +
'title="' + title + '" ' +
'src="' + embedUrl + '" ' + 'src="' + embedUrl + '" ' +
'frameborder="0" allowfullscreen>' + 'frameborder="0" allowfullscreen>' +
'</iframe>' '</iframe>'

View file

@ -545,7 +545,8 @@ export class PeerTubeEmbed {
serverUrl: window.location.origin, serverUrl: window.location.origin,
language: navigator.language, language: navigator.language,
embedUrl: window.location.origin + videoInfo.embedPath embedUrl: window.location.origin + videoInfo.embedPath,
embedTitle: videoInfo.name
}, },
webtorrent: { webtorrent: {

View file

@ -3,6 +3,7 @@ import { EMBED_SIZE, PREVIEWS_SIZE, WEBSERVER, THUMBNAILS_SIZE } from '../initia
import { asyncMiddleware, oembedValidator } from '../middlewares' import { asyncMiddleware, oembedValidator } from '../middlewares'
import { accountNameWithHostGetValidator } from '../middlewares/validators' import { accountNameWithHostGetValidator } from '../middlewares/validators'
import { MChannelSummary } from '@server/types/models' import { MChannelSummary } from '@server/types/models'
import { escapeHTML } from '@shared/core-utils/renderer'
const servicesRouter = express.Router() const servicesRouter = express.Router()
@ -79,6 +80,7 @@ function buildOEmbed (options: {
const embedUrl = webserverUrl + embedPath const embedUrl = webserverUrl + embedPath
let embedWidth = EMBED_SIZE.width let embedWidth = EMBED_SIZE.width
let embedHeight = EMBED_SIZE.height let embedHeight = EMBED_SIZE.height
const embedTitle = escapeHTML(title)
let thumbnailUrl = previewPath let thumbnailUrl = previewPath
? webserverUrl + previewPath ? webserverUrl + previewPath
@ -96,7 +98,7 @@ function buildOEmbed (options: {
} }
const html = `<iframe width="${embedWidth}" height="${embedHeight}" sandbox="allow-same-origin allow-scripts" ` + const html = `<iframe width="${embedWidth}" height="${embedHeight}" sandbox="allow-same-origin allow-scripts" ` +
`src="${embedUrl}" frameborder="0" allowfullscreen></iframe>` `title="${embedTitle}" src="${embedUrl}" frameborder="0" allowfullscreen></iframe>`
const json: any = { const json: any = {
type: 'video', type: 'video',

View file

@ -154,24 +154,6 @@ function root () {
return rootPath return rootPath
} }
// Thanks: https://stackoverflow.com/a/12034334
function escapeHTML (stringParam) {
if (!stringParam) return ''
const entityMap = {
'&': '&amp;',
'<': '&lt;',
'>': '&gt;',
'"': '&quot;',
'\'': '&#39;',
'/': '&#x2F;',
'`': '&#x60;',
'=': '&#x3D;'
}
return String(stringParam).replace(/[&<>"'`=/]/g, s => entityMap[s])
}
function pageToStartAndCount (page: number, itemsPerPage: number) { function pageToStartAndCount (page: number, itemsPerPage: number) {
const start = (page - 1) * itemsPerPage const start = (page - 1) * itemsPerPage
@ -278,7 +260,6 @@ export {
objectConverter, objectConverter,
root, root,
escapeHTML,
pageToStartAndCount, pageToStartAndCount,
sanitizeUrl, sanitizeUrl,
sanitizeHost, sanitizeHost,

View file

@ -5,7 +5,8 @@ import validator from 'validator'
import { buildFileLocale, getDefaultLocale, is18nLocale, POSSIBLE_LOCALES } from '../../shared/core-utils/i18n/i18n' import { buildFileLocale, getDefaultLocale, is18nLocale, POSSIBLE_LOCALES } from '../../shared/core-utils/i18n/i18n'
import { HttpStatusCode } from '../../shared/core-utils/miscs/http-error-codes' import { HttpStatusCode } from '../../shared/core-utils/miscs/http-error-codes'
import { VideoPlaylistPrivacy, VideoPrivacy } from '../../shared/models/videos' import { VideoPlaylistPrivacy, VideoPrivacy } from '../../shared/models/videos'
import { escapeHTML, isTestInstance, sha256 } from '../helpers/core-utils' import { isTestInstance, sha256 } from '../helpers/core-utils'
import { escapeHTML } from '@shared/core-utils/renderer'
import { logger } from '../helpers/logger' import { logger } from '../helpers/logger'
import { CONFIG } from '../initializers/config' import { CONFIG } from '../initializers/config'
import { import {

View file

@ -20,6 +20,7 @@ const expect = chai.expect
describe('Test services', function () { describe('Test services', function () {
let server: ServerInfo = null let server: ServerInfo = null
let playlistUUID: string let playlistUUID: string
let playlistDisplayName: string
let video: Video let video: Video
before(async function () { before(async function () {
@ -52,6 +53,7 @@ describe('Test services', function () {
}) })
playlistUUID = res.body.videoPlaylist.uuid playlistUUID = res.body.videoPlaylist.uuid
playlistDisplayName = 'The Life and Times of Scrooge McDuck'
await addVideoInPlaylist({ await addVideoInPlaylist({
url: server.url, url: server.url,
@ -69,7 +71,7 @@ describe('Test services', function () {
const res = await getOEmbed(server.url, oembedUrl) const res = await getOEmbed(server.url, oembedUrl)
const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' + const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' +
`src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` + `title="${video.name}" src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` +
'frameborder="0" allowfullscreen></iframe>' 'frameborder="0" allowfullscreen></iframe>'
const expectedThumbnailUrl = 'http://localhost:' + server.port + video.previewPath const expectedThumbnailUrl = 'http://localhost:' + server.port + video.previewPath
@ -88,7 +90,7 @@ describe('Test services', function () {
const res = await getOEmbed(server.url, oembedUrl) const res = await getOEmbed(server.url, oembedUrl)
const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' + const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' +
`src="http://localhost:${server.port}/video-playlists/embed/${playlistUUID}" ` + `title="${playlistDisplayName}" src="http://localhost:${server.port}/video-playlists/embed/${playlistUUID}" ` +
'frameborder="0" allowfullscreen></iframe>' 'frameborder="0" allowfullscreen></iframe>'
expect(res.body.html).to.equal(expectedHtml) expect(res.body.html).to.equal(expectedHtml)
@ -109,7 +111,7 @@ describe('Test services', function () {
const res = await getOEmbed(server.url, oembedUrl, format, maxHeight, maxWidth) const res = await getOEmbed(server.url, oembedUrl, format, maxHeight, maxWidth)
const expectedHtml = '<iframe width="50" height="50" sandbox="allow-same-origin allow-scripts" ' + const expectedHtml = '<iframe width="50" height="50" sandbox="allow-same-origin allow-scripts" ' +
`src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` + `title="${video.name}" src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` +
'frameborder="0" allowfullscreen></iframe>' 'frameborder="0" allowfullscreen></iframe>'
expect(res.body.html).to.equal(expectedHtml) expect(res.body.html).to.equal(expectedHtml)

View file

@ -19,3 +19,21 @@ export const SANITIZE_OPTIONS = {
} }
} }
} }
// Thanks: https://stackoverflow.com/a/12034334
export function escapeHTML (stringParam: string) {
if (!stringParam) return ''
const entityMap = {
'&': '&amp;',
'<': '&lt;',
'>': '&gt;',
'"': '&quot;',
'\'': '&#39;',
'/': '&#x2F;',
'`': '&#x60;',
'=': '&#x3D;'
}
return String(stringParam).replace(/[&<>"'`=/]/g, s => entityMap[s])
}