diff options
author | Thavarasa Prasanth <45243326+pthavarasa@users.noreply.github.com> | 2021-03-31 08:32:05 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-31 08:32:05 +0200 |
commit | 4097c6d66cb2919c28b5bce44b259e630923fbe0 (patch) | |
tree | d57897b0b8762202a0be90388e9a79153690b9d9 | |
parent | 47099aba46be7757d833422f1706b99a3c0e28ea (diff) | |
download | PeerTube-4097c6d66cb2919c28b5bce44b259e630923fbe0.tar.gz PeerTube-4097c6d66cb2919c28b5bce44b259e630923fbe0.tar.zst PeerTube-4097c6d66cb2919c28b5bce44b259e630923fbe0.zip |
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
-rw-r--r-- | client/src/app/+admin/moderation/video-block-list/video-block-list.component.ts | 3 | ||||
-rw-r--r-- | client/src/app/+videos/+video-watch/video-watch.component.ts | 1 | ||||
-rw-r--r-- | client/src/app/shared/shared-abuse-list/abuse-list-table.component.ts | 3 | ||||
-rw-r--r-- | client/src/app/shared/shared-moderation/report-modals/video-report.component.ts | 3 | ||||
-rw-r--r-- | client/src/app/shared/shared-share-modal/video-share.component.ts | 4 | ||||
-rw-r--r-- | client/src/assets/player/peertube-player-manager.ts | 9 | ||||
-rw-r--r-- | client/src/assets/player/utils.ts | 5 | ||||
-rw-r--r-- | client/src/standalone/videos/embed.ts | 3 | ||||
-rw-r--r-- | server/controllers/services.ts | 4 | ||||
-rw-r--r-- | server/helpers/core-utils.ts | 19 | ||||
-rw-r--r-- | server/lib/client-html.ts | 3 | ||||
-rw-r--r-- | server/tests/api/server/services.ts | 8 | ||||
-rw-r--r-- | shared/core-utils/renderer/html.ts | 18 |
13 files changed, 48 insertions, 35 deletions
diff --git a/client/src/app/+admin/moderation/video-block-list/video-block-list.component.ts b/client/src/app/+admin/moderation/video-block-list/video-block-list.component.ts index 82c371f4d..d6aca10e7 100644 --- a/client/src/app/+admin/moderation/video-block-list/video-block-list.component.ts +++ b/client/src/app/+admin/moderation/video-block-list/video-block-list.component.ts | |||
@@ -164,7 +164,8 @@ export class VideoBlockListComponent extends RestTable implements OnInit, AfterV | |||
164 | baseUrl: `${environment.originServerUrl}/videos/embed/${entry.video.uuid}`, | 164 | baseUrl: `${environment.originServerUrl}/videos/embed/${entry.video.uuid}`, |
165 | title: false, | 165 | title: false, |
166 | warningTitle: false | 166 | warningTitle: false |
167 | }) | 167 | }), |
168 | entry.video.name | ||
168 | ) | 169 | ) |
169 | } | 170 | } |
170 | 171 | ||
diff --git a/client/src/app/+videos/+video-watch/video-watch.component.ts b/client/src/app/+videos/+video-watch/video-watch.component.ts index 7a98cab3b..ce115dfab 100644 --- a/client/src/app/+videos/+video-watch/video-watch.component.ts +++ b/client/src/app/+videos/+video-watch/video-watch.component.ts | |||
@@ -815,6 +815,7 @@ export class VideoWatchComponent implements OnInit, OnDestroy { | |||
815 | ? this.videoService.getVideoViewUrl(video.uuid) | 815 | ? this.videoService.getVideoViewUrl(video.uuid) |
816 | : null, | 816 | : null, |
817 | embedUrl: video.embedUrl, | 817 | embedUrl: video.embedUrl, |
818 | embedTitle: video.name, | ||
818 | 819 | ||
819 | isLive: video.isLive, | 820 | isLive: video.isLive, |
820 | 821 | ||
diff --git a/client/src/app/shared/shared-abuse-list/abuse-list-table.component.ts b/client/src/app/shared/shared-abuse-list/abuse-list-table.component.ts index e34836a18..eeb9f128b 100644 --- a/client/src/app/shared/shared-abuse-list/abuse-list-table.component.ts +++ b/client/src/app/shared/shared-abuse-list/abuse-list-table.component.ts | |||
@@ -117,7 +117,8 @@ export class AbuseListTableComponent extends RestTable implements OnInit, AfterV | |||
117 | warningTitle: false, | 117 | warningTitle: false, |
118 | startTime: abuse.video.startAt, | 118 | startTime: abuse.video.startAt, |
119 | stopTime: abuse.video.endAt | 119 | stopTime: abuse.video.endAt |
120 | }) | 120 | }), |
121 | abuse.video.name | ||
121 | ) | 122 | ) |
122 | } | 123 | } |
123 | 124 | ||
diff --git a/client/src/app/shared/shared-moderation/report-modals/video-report.component.ts b/client/src/app/shared/shared-moderation/report-modals/video-report.component.ts index 5b06c0bc7..4ca6f52ad 100644 --- a/client/src/app/shared/shared-moderation/report-modals/video-report.component.ts +++ b/client/src/app/shared/shared-moderation/report-modals/video-report.component.ts | |||
@@ -61,7 +61,8 @@ export class VideoReportComponent extends FormReactive implements OnInit { | |||
61 | baseUrl: this.video.embedUrl, | 61 | baseUrl: this.video.embedUrl, |
62 | title: false, | 62 | title: false, |
63 | warningTitle: false | 63 | warningTitle: false |
64 | }) | 64 | }), |
65 | this.video.name | ||
65 | ) | 66 | ) |
66 | ) | 67 | ) |
67 | } | 68 | } |
diff --git a/client/src/app/shared/shared-share-modal/video-share.component.ts b/client/src/app/shared/shared-share-modal/video-share.component.ts index b06ff3751..e8760bfcc 100644 --- a/client/src/app/shared/shared-share-modal/video-share.component.ts +++ b/client/src/app/shared/shared-share-modal/video-share.component.ts | |||
@@ -86,14 +86,14 @@ export class VideoShareComponent { | |||
86 | const options = this.getVideoOptions(this.video.embedUrl) | 86 | const options = this.getVideoOptions(this.video.embedUrl) |
87 | 87 | ||
88 | const embedUrl = buildVideoLink(options) | 88 | const embedUrl = buildVideoLink(options) |
89 | return buildVideoOrPlaylistEmbed(embedUrl) | 89 | return buildVideoOrPlaylistEmbed(embedUrl, this.video.name) |
90 | } | 90 | } |
91 | 91 | ||
92 | getPlaylistIframeCode () { | 92 | getPlaylistIframeCode () { |
93 | const options = this.getPlaylistOptions(this.playlist.embedUrl) | 93 | const options = this.getPlaylistOptions(this.playlist.embedUrl) |
94 | 94 | ||
95 | const embedUrl = buildPlaylistLink(options) | 95 | const embedUrl = buildPlaylistLink(options) |
96 | return buildVideoOrPlaylistEmbed(embedUrl) | 96 | return buildVideoOrPlaylistEmbed(embedUrl, this.playlist.displayName) |
97 | } | 97 | } |
98 | 98 | ||
99 | getVideoUrl () { | 99 | getVideoUrl () { |
diff --git a/client/src/assets/player/peertube-player-manager.ts b/client/src/assets/player/peertube-player-manager.ts index e6d614c47..1d335805b 100644 --- a/client/src/assets/player/peertube-player-manager.ts +++ b/client/src/assets/player/peertube-player-manager.ts | |||
@@ -98,6 +98,7 @@ export interface CommonOptions extends CustomizationOptions { | |||
98 | 98 | ||
99 | videoViewUrl: string | 99 | videoViewUrl: string |
100 | embedUrl: string | 100 | embedUrl: string |
101 | embedTitle: string | ||
101 | 102 | ||
102 | isLive: boolean | 103 | isLive: boolean |
103 | 104 | ||
@@ -165,7 +166,7 @@ export class PeertubePlayerManager { | |||
165 | PeertubePlayerManager.alreadyPlayed = true | 166 | PeertubePlayerManager.alreadyPlayed = true |
166 | }) | 167 | }) |
167 | 168 | ||
168 | self.addContextMenu(mode, player, options.common.embedUrl) | 169 | self.addContextMenu(mode, player, options.common.embedUrl, options.common.embedTitle) |
169 | 170 | ||
170 | player.bezels() | 171 | player.bezels() |
171 | 172 | ||
@@ -203,7 +204,7 @@ export class PeertubePlayerManager { | |||
203 | videojs(newVideoElement, videojsOptions, function (this: videojs.Player) { | 204 | videojs(newVideoElement, videojsOptions, function (this: videojs.Player) { |
204 | const player = this | 205 | const player = this |
205 | 206 | ||
206 | self.addContextMenu(mode, player, options.common.embedUrl) | 207 | self.addContextMenu(mode, player, options.common.embedUrl, options.common.embedTitle) |
207 | 208 | ||
208 | PeertubePlayerManager.onPlayerChange(player) | 209 | PeertubePlayerManager.onPlayerChange(player) |
209 | }) | 210 | }) |
@@ -492,7 +493,7 @@ export class PeertubePlayerManager { | |||
492 | return children | 493 | return children |
493 | } | 494 | } |
494 | 495 | ||
495 | private static addContextMenu (mode: PlayerMode, player: videojs.Player, videoEmbedUrl: string) { | 496 | private static addContextMenu (mode: PlayerMode, player: videojs.Player, videoEmbedUrl: string, videoEmbedTitle: string) { |
496 | const content = [ | 497 | const content = [ |
497 | { | 498 | { |
498 | label: player.localize('Copy the video URL'), | 499 | label: player.localize('Copy the video URL'), |
@@ -509,7 +510,7 @@ export class PeertubePlayerManager { | |||
509 | { | 510 | { |
510 | label: player.localize('Copy embed code'), | 511 | label: player.localize('Copy embed code'), |
511 | listener: () => { | 512 | listener: () => { |
512 | copyToClipboard(buildVideoOrPlaylistEmbed(videoEmbedUrl)) | 513 | copyToClipboard(buildVideoOrPlaylistEmbed(videoEmbedUrl, videoEmbedTitle)) |
513 | } | 514 | } |
514 | } | 515 | } |
515 | ] | 516 | ] |
diff --git a/client/src/assets/player/utils.ts b/client/src/assets/player/utils.ts index 6767459ce..d7451fa1d 100644 --- a/client/src/assets/player/utils.ts +++ b/client/src/assets/player/utils.ts | |||
@@ -1,4 +1,5 @@ | |||
1 | import { VideoFile } from '@shared/models' | 1 | import { VideoFile } from '@shared/models' |
2 | import { escapeHTML } from '@shared/core-utils/renderer' | ||
2 | 3 | ||
3 | function toTitleCase (str: string) { | 4 | function toTitleCase (str: string) { |
4 | return str.charAt(0).toUpperCase() + str.slice(1) | 5 | return str.charAt(0).toUpperCase() + str.slice(1) |
@@ -170,9 +171,11 @@ function secondsToTime (seconds: number, full = false, symbol?: string) { | |||
170 | return time | 171 | return time |
171 | } | 172 | } |
172 | 173 | ||
173 | function buildVideoOrPlaylistEmbed (embedUrl: string) { | 174 | function buildVideoOrPlaylistEmbed (embedUrl: string, embedTitle: string) { |
175 | const title = escapeHTML(embedTitle) | ||
174 | return '<iframe width="560" height="315" ' + | 176 | return '<iframe width="560" height="315" ' + |
175 | 'sandbox="allow-same-origin allow-scripts allow-popups" ' + | 177 | 'sandbox="allow-same-origin allow-scripts allow-popups" ' + |
178 | 'title="' + title + '" ' + | ||
176 | 'src="' + embedUrl + '" ' + | 179 | 'src="' + embedUrl + '" ' + |
177 | 'frameborder="0" allowfullscreen>' + | 180 | 'frameborder="0" allowfullscreen>' + |
178 | '</iframe>' | 181 | '</iframe>' |
diff --git a/client/src/standalone/videos/embed.ts b/client/src/standalone/videos/embed.ts index c87270027..614a1cc0b 100644 --- a/client/src/standalone/videos/embed.ts +++ b/client/src/standalone/videos/embed.ts | |||
@@ -545,7 +545,8 @@ export class PeerTubeEmbed { | |||
545 | 545 | ||
546 | serverUrl: window.location.origin, | 546 | serverUrl: window.location.origin, |
547 | language: navigator.language, | 547 | language: navigator.language, |
548 | embedUrl: window.location.origin + videoInfo.embedPath | 548 | embedUrl: window.location.origin + videoInfo.embedPath, |
549 | embedTitle: videoInfo.name | ||
549 | }, | 550 | }, |
550 | 551 | ||
551 | webtorrent: { | 552 | webtorrent: { |
diff --git a/server/controllers/services.ts b/server/controllers/services.ts index d0217c30a..189e1651b 100644 --- a/server/controllers/services.ts +++ b/server/controllers/services.ts | |||
@@ -3,6 +3,7 @@ import { EMBED_SIZE, PREVIEWS_SIZE, WEBSERVER, THUMBNAILS_SIZE } from '../initia | |||
3 | import { asyncMiddleware, oembedValidator } from '../middlewares' | 3 | import { asyncMiddleware, oembedValidator } from '../middlewares' |
4 | import { accountNameWithHostGetValidator } from '../middlewares/validators' | 4 | import { accountNameWithHostGetValidator } from '../middlewares/validators' |
5 | import { MChannelSummary } from '@server/types/models' | 5 | import { MChannelSummary } from '@server/types/models' |
6 | import { escapeHTML } from '@shared/core-utils/renderer' | ||
6 | 7 | ||
7 | const servicesRouter = express.Router() | 8 | const servicesRouter = express.Router() |
8 | 9 | ||
@@ -79,6 +80,7 @@ function buildOEmbed (options: { | |||
79 | const embedUrl = webserverUrl + embedPath | 80 | const embedUrl = webserverUrl + embedPath |
80 | let embedWidth = EMBED_SIZE.width | 81 | let embedWidth = EMBED_SIZE.width |
81 | let embedHeight = EMBED_SIZE.height | 82 | let embedHeight = EMBED_SIZE.height |
83 | const embedTitle = escapeHTML(title) | ||
82 | 84 | ||
83 | let thumbnailUrl = previewPath | 85 | let thumbnailUrl = previewPath |
84 | ? webserverUrl + previewPath | 86 | ? webserverUrl + previewPath |
@@ -96,7 +98,7 @@ function buildOEmbed (options: { | |||
96 | } | 98 | } |
97 | 99 | ||
98 | const html = `<iframe width="${embedWidth}" height="${embedHeight}" sandbox="allow-same-origin allow-scripts" ` + | 100 | const html = `<iframe width="${embedWidth}" height="${embedHeight}" sandbox="allow-same-origin allow-scripts" ` + |
99 | `src="${embedUrl}" frameborder="0" allowfullscreen></iframe>` | 101 | `title="${embedTitle}" src="${embedUrl}" frameborder="0" allowfullscreen></iframe>` |
100 | 102 | ||
101 | const json: any = { | 103 | const json: any = { |
102 | type: 'video', | 104 | type: 'video', |
diff --git a/server/helpers/core-utils.ts b/server/helpers/core-utils.ts index 0bd84ffaa..b93868c12 100644 --- a/server/helpers/core-utils.ts +++ b/server/helpers/core-utils.ts | |||
@@ -154,24 +154,6 @@ function root () { | |||
154 | return rootPath | 154 | return rootPath |
155 | } | 155 | } |
156 | 156 | ||
157 | // Thanks: https://stackoverflow.com/a/12034334 | ||
158 | function escapeHTML (stringParam) { | ||
159 | if (!stringParam) return '' | ||
160 | |||
161 | const entityMap = { | ||
162 | '&': '&', | ||
163 | '<': '<', | ||
164 | '>': '>', | ||
165 | '"': '"', | ||
166 | '\'': ''', | ||
167 | '/': '/', | ||
168 | '`': '`', | ||
169 | '=': '=' | ||
170 | } | ||
171 | |||
172 | return String(stringParam).replace(/[&<>"'`=/]/g, s => entityMap[s]) | ||
173 | } | ||
174 | |||
175 | function pageToStartAndCount (page: number, itemsPerPage: number) { | 157 | function pageToStartAndCount (page: number, itemsPerPage: number) { |
176 | const start = (page - 1) * itemsPerPage | 158 | const start = (page - 1) * itemsPerPage |
177 | 159 | ||
@@ -278,7 +260,6 @@ export { | |||
278 | 260 | ||
279 | objectConverter, | 261 | objectConverter, |
280 | root, | 262 | root, |
281 | escapeHTML, | ||
282 | pageToStartAndCount, | 263 | pageToStartAndCount, |
283 | sanitizeUrl, | 264 | sanitizeUrl, |
284 | sanitizeHost, | 265 | sanitizeHost, |
diff --git a/server/lib/client-html.ts b/server/lib/client-html.ts index f19ec7df0..fcc11c7b2 100644 --- a/server/lib/client-html.ts +++ b/server/lib/client-html.ts | |||
@@ -5,7 +5,8 @@ import validator from 'validator' | |||
5 | import { buildFileLocale, getDefaultLocale, is18nLocale, POSSIBLE_LOCALES } from '../../shared/core-utils/i18n/i18n' | 5 | import { buildFileLocale, getDefaultLocale, is18nLocale, POSSIBLE_LOCALES } from '../../shared/core-utils/i18n/i18n' |
6 | import { HttpStatusCode } from '../../shared/core-utils/miscs/http-error-codes' | 6 | import { HttpStatusCode } from '../../shared/core-utils/miscs/http-error-codes' |
7 | import { VideoPlaylistPrivacy, VideoPrivacy } from '../../shared/models/videos' | 7 | import { VideoPlaylistPrivacy, VideoPrivacy } from '../../shared/models/videos' |
8 | import { escapeHTML, isTestInstance, sha256 } from '../helpers/core-utils' | 8 | import { isTestInstance, sha256 } from '../helpers/core-utils' |
9 | import { escapeHTML } from '@shared/core-utils/renderer' | ||
9 | import { logger } from '../helpers/logger' | 10 | import { logger } from '../helpers/logger' |
10 | import { CONFIG } from '../initializers/config' | 11 | import { CONFIG } from '../initializers/config' |
11 | import { | 12 | import { |
diff --git a/server/tests/api/server/services.ts b/server/tests/api/server/services.ts index df910c111..6202eb66c 100644 --- a/server/tests/api/server/services.ts +++ b/server/tests/api/server/services.ts | |||
@@ -20,6 +20,7 @@ const expect = chai.expect | |||
20 | describe('Test services', function () { | 20 | describe('Test services', function () { |
21 | let server: ServerInfo = null | 21 | let server: ServerInfo = null |
22 | let playlistUUID: string | 22 | let playlistUUID: string |
23 | let playlistDisplayName: string | ||
23 | let video: Video | 24 | let video: Video |
24 | 25 | ||
25 | before(async function () { | 26 | before(async function () { |
@@ -52,6 +53,7 @@ describe('Test services', function () { | |||
52 | }) | 53 | }) |
53 | 54 | ||
54 | playlistUUID = res.body.videoPlaylist.uuid | 55 | playlistUUID = res.body.videoPlaylist.uuid |
56 | playlistDisplayName = 'The Life and Times of Scrooge McDuck' | ||
55 | 57 | ||
56 | await addVideoInPlaylist({ | 58 | await addVideoInPlaylist({ |
57 | url: server.url, | 59 | url: server.url, |
@@ -69,7 +71,7 @@ describe('Test services', function () { | |||
69 | 71 | ||
70 | const res = await getOEmbed(server.url, oembedUrl) | 72 | const res = await getOEmbed(server.url, oembedUrl) |
71 | const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' + | 73 | const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' + |
72 | `src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` + | 74 | `title="${video.name}" src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` + |
73 | 'frameborder="0" allowfullscreen></iframe>' | 75 | 'frameborder="0" allowfullscreen></iframe>' |
74 | const expectedThumbnailUrl = 'http://localhost:' + server.port + video.previewPath | 76 | const expectedThumbnailUrl = 'http://localhost:' + server.port + video.previewPath |
75 | 77 | ||
@@ -88,7 +90,7 @@ describe('Test services', function () { | |||
88 | 90 | ||
89 | const res = await getOEmbed(server.url, oembedUrl) | 91 | const res = await getOEmbed(server.url, oembedUrl) |
90 | const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' + | 92 | const expectedHtml = '<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts" ' + |
91 | `src="http://localhost:${server.port}/video-playlists/embed/${playlistUUID}" ` + | 93 | `title="${playlistDisplayName}" src="http://localhost:${server.port}/video-playlists/embed/${playlistUUID}" ` + |
92 | 'frameborder="0" allowfullscreen></iframe>' | 94 | 'frameborder="0" allowfullscreen></iframe>' |
93 | 95 | ||
94 | expect(res.body.html).to.equal(expectedHtml) | 96 | expect(res.body.html).to.equal(expectedHtml) |
@@ -109,7 +111,7 @@ describe('Test services', function () { | |||
109 | 111 | ||
110 | const res = await getOEmbed(server.url, oembedUrl, format, maxHeight, maxWidth) | 112 | const res = await getOEmbed(server.url, oembedUrl, format, maxHeight, maxWidth) |
111 | const expectedHtml = '<iframe width="50" height="50" sandbox="allow-same-origin allow-scripts" ' + | 113 | const expectedHtml = '<iframe width="50" height="50" sandbox="allow-same-origin allow-scripts" ' + |
112 | `src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` + | 114 | `title="${video.name}" src="http://localhost:${server.port}/videos/embed/${video.uuid}" ` + |
113 | 'frameborder="0" allowfullscreen></iframe>' | 115 | 'frameborder="0" allowfullscreen></iframe>' |
114 | 116 | ||
115 | expect(res.body.html).to.equal(expectedHtml) | 117 | expect(res.body.html).to.equal(expectedHtml) |
diff --git a/shared/core-utils/renderer/html.ts b/shared/core-utils/renderer/html.ts index 1220848a0..de4ad47ac 100644 --- a/shared/core-utils/renderer/html.ts +++ b/shared/core-utils/renderer/html.ts | |||
@@ -19,3 +19,21 @@ export const SANITIZE_OPTIONS = { | |||
19 | } | 19 | } |
20 | } | 20 | } |
21 | } | 21 | } |
22 | |||
23 | // Thanks: https://stackoverflow.com/a/12034334 | ||
24 | export function escapeHTML (stringParam: string) { | ||
25 | if (!stringParam) return '' | ||
26 | |||
27 | const entityMap = { | ||
28 | '&': '&', | ||
29 | '<': '<', | ||
30 | '>': '>', | ||
31 | '"': '"', | ||
32 | '\'': ''', | ||
33 | '/': '/', | ||
34 | '`': '`', | ||
35 | '=': '=' | ||
36 | } | ||
37 | |||
38 | return String(stringParam).replace(/[&<>"'`=/]/g, s => entityMap[s]) | ||
39 | } | ||