Refactor video extensions logic in server

6 files changed, 87 insertions, 52 deletions

diff --git a/server/helpers/custom-validators/videos.ts b/server/helpers/custom-validators/videos.ts
index 60e8075f6..40fecc09b 100644
--- a/server/helpers/custom-validators/videos.ts
+++ b/server/helpers/custom-validators/videos.ts
@@ -81,11 +81,7 @@ function isVideoFileExtnameValid (value: string) {
 }
 
 function isVideoFile (files: { [ fieldname: string ]: Express.Multer.File[] } | Express.Multer.File[]) {
-  const videoFileTypesRegex = Object.keys(MIMETYPES.VIDEO.MIMETYPE_EXT)
-                                    .map(m => `(${m})`)
-                                    .join('|')
-
-  return isFileValid(files, videoFileTypesRegex, 'videofile', null)
+  return isFileValid(files, MIMETYPES.VIDEO.MIMETYPES_REGEX, 'videofile', null)
 }
 
 const videoImageTypes = CONSTRAINTS_FIELDS.VIDEOS.IMAGE.EXTNAME
diff --git a/server/helpers/express-utils.ts b/server/helpers/express-utils.ts
index f46812977..ba23557ba 100644
--- a/server/helpers/express-utils.ts
+++ b/server/helpers/express-utils.ts
@@ -6,6 +6,7 @@ import { deleteFileAsync, generateRandomString } from './utils'
 import { extname } from 'path'
 import { isArray } from './custom-validators/misc'
 import { CONFIG } from '../initializers/config'
+import { getExtFromMimetype } from './video'
 
 function buildNSFWFilter (res?: express.Response, paramNSFW?: string) {
   if (paramNSFW === 'true') return true
@@ -65,7 +66,7 @@ function badRequest (req: express.Request, res: express.Response) {
 
 function createReqFiles (
   fieldNames: string[],
-  mimeTypes: { [id: string]: string },
+  mimeTypes: { [id: string]: string | string[] },
   destinations: { [fieldName: string]: string }
 ) {
   const storage = multer.diskStorage({
@@ -76,13 +77,13 @@ function createReqFiles (
     filename: async (req, file, cb) => {
       let extension: string
       const fileExtension = extname(file.originalname)
-      const extensionFromMimetype = mimeTypes[file.mimetype]
+      const extensionFromMimetype = getExtFromMimetype(mimeTypes, file.mimetype)
 
       // Take the file extension if we don't understand the mime type
-      // We have the OGG/OGV exception too because firefox sends a bad mime type when sending an OGG file
-      if (fileExtension === '.ogg' || fileExtension === '.ogv' || !extensionFromMimetype) {
+      if (!extensionFromMimetype) {
         extension = fileExtension
       } else {
+        // Take the first available extension for this mimetype
         extension = extensionFromMimetype
       }
 
diff --git a/server/helpers/video.ts b/server/helpers/video.ts
index 89c85accb..488b4da17 100644
--- a/server/helpers/video.ts
+++ b/server/helpers/video.ts
@@ -1,5 +1,8 @@
-import { VideoModel } from '../models/video/video'
 import * as Bluebird from 'bluebird'
+import { Response } from 'express'
+import { CONFIG } from '@server/initializers/config'
+import { DEFAULT_AUDIO_RESOLUTION } from '@server/initializers/constants'
+import { JobQueue } from '@server/lib/job-queue'
 import {
   isStreamingPlaylist,
   MStreamingPlaylistVideo,
@@ -12,11 +15,8 @@ import {
   MVideoThumbnail,
   MVideoWithRights
 } from '@server/types/models'
-import { Response } from 'express'
-import { DEFAULT_AUDIO_RESOLUTION } from '@server/initializers/constants'
-import { JobQueue } from '@server/lib/job-queue'
 import { VideoPrivacy, VideoTranscodingPayload } from '@shared/models'
-import { CONFIG } from "@server/initializers/config"
+import { VideoModel } from '../models/video/video'
 
 type VideoFetchType = 'all' | 'only-video' | 'only-video-with-rights' | 'id' | 'none' | 'only-immutable-attributes'
 
@@ -110,6 +110,14 @@ function getPrivaciesForFederation () {
     : [ { privacy: VideoPrivacy.PUBLIC } ]
 }
 
+function getExtFromMimetype (mimeTypes: { [id: string]: string | string[] }, mimeType: string) {
+  const value = mimeTypes[mimeType]
+
+  if (Array.isArray(value)) return value[0]
+
+  return value
+}
+
 export {
   VideoFetchType,
   VideoFetchByUrlType,
@@ -118,6 +126,7 @@ export {
   fetchVideoByUrl,
   addOptimizeOrMergeAudioJob,
   extractVideo,
+  getExtFromMimetype,
   isPrivacyForFederation,
   getPrivaciesForFederation
 }
diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts
index 573d86b60..ebbdba262 100644
--- a/server/initializers/constants.ts
+++ b/server/initializers/constants.ts
@@ -422,7 +422,8 @@ const MIMETYPES = {
     EXT_MIMETYPE: null as { [ id: string ]: string }
   },
   VIDEO: {
-    MIMETYPE_EXT: null as { [ id: string ]: string },
+    MIMETYPE_EXT: null as { [ id: string ]: string | string[] },
+    MIMETYPES_REGEX: null as string,
     EXT_MIMETYPE: null as { [ id: string ]: string }
   },
   IMAGE: {
@@ -825,15 +826,19 @@ function buildVideoMimetypeExt () {
   const data = {
     // streamable formats that warrant cross-browser compatibility
     'video/webm': '.webm',
-    'video/ogg': '.ogv',
+    // We'll add .ogg if additional extensions are enabled
+    // We could add .ogg here but since it could be an audio file,
+    // it would be confusing for users because PeerTube will refuse their file (based on the mimetype)
+    'video/ogg': [ '.ogv' ],
     'video/mp4': '.mp4'
   }
 
   if (CONFIG.TRANSCODING.ENABLED) {
     if (CONFIG.TRANSCODING.ALLOW_ADDITIONAL_EXTENSIONS) {
+      data['video/ogg'].push('.ogg')
+
       Object.assign(data, {
         'video/x-matroska': '.mkv',
-        'video/ogg': '.ogg',
 
         // Developed by Apple
         'video/quicktime': '.mov', // often used as output format by editing software
@@ -892,14 +897,36 @@ function updateWebserverUrls () {
 
 function updateWebserverConfig () {
   MIMETYPES.VIDEO.MIMETYPE_EXT = buildVideoMimetypeExt()
-  MIMETYPES.VIDEO.EXT_MIMETYPE = invert(MIMETYPES.VIDEO.MIMETYPE_EXT)
+  MIMETYPES.VIDEO.MIMETYPES_REGEX = buildMimetypesRegex(MIMETYPES.VIDEO.MIMETYPE_EXT)
+
   ACTIVITY_PUB.URL_MIME_TYPES.VIDEO = Object.keys(MIMETYPES.VIDEO.MIMETYPE_EXT)
 
-  CONSTRAINTS_FIELDS.VIDEOS.EXTNAME = buildVideosExtname()
+  MIMETYPES.VIDEO.EXT_MIMETYPE = buildVideoExtMimetype(MIMETYPES.VIDEO.MIMETYPE_EXT)
+
+  CONSTRAINTS_FIELDS.VIDEOS.EXTNAME = Object.keys(MIMETYPES.VIDEO.EXT_MIMETYPE)
+}
+
+function buildVideoExtMimetype (obj: { [ id: string ]: string | string[] }) {
+  const result: { [id: string]: string } = {}
+
+  for (const mimetype of Object.keys(obj)) {
+    const value = obj[mimetype]
+    if (!value) continue
+
+    const extensions = Array.isArray(value) ? value : [ value ]
+
+    for (const extension of extensions) {
+      result[extension] = mimetype
+    }
+  }
+
+  return result
 }
 
-function buildVideosExtname () {
-  return Object.keys(MIMETYPES.VIDEO.EXT_MIMETYPE).filter(e => e !== 'null')
+function buildMimetypesRegex (obj: { [id: string]: string | string[] }) {
+  return Object.keys(obj)
+    .map(m => `(${m})`)
+    .join('|')
 }
 
 function loadLanguages () {
diff --git a/server/lib/activitypub/videos.ts b/server/lib/activitypub/videos.ts
index 6c5f7f306..cbbf23be1 100644
--- a/server/lib/activitypub/videos.ts
+++ b/server/lib/activitypub/videos.ts
@@ -1,12 +1,15 @@
 import * as Bluebird from 'bluebird'
-import * as sequelize from 'sequelize'
+import { maxBy, minBy } from 'lodash'
 import * as magnetUtil from 'magnet-uri'
+import { join } from 'path'
 import * as request from 'request'
+import * as sequelize from 'sequelize'
 import {
   ActivityHashTagObject,
   ActivityMagnetUrlObject,
   ActivityPlaylistSegmentHashesObject,
-  ActivityPlaylistUrlObject, ActivitypubHttpFetcherPayload,
+  ActivityPlaylistUrlObject,
+  ActivitypubHttpFetcherPayload,
   ActivityTagObject,
   ActivityUrlObject,
   ActivityVideoUrlObject,
@@ -14,11 +17,16 @@ import {
 } from '../../../shared/index'
 import { VideoTorrentObject } from '../../../shared/models/activitypub/objects'
 import { VideoPrivacy } from '../../../shared/models/videos'
+import { ThumbnailType } from '../../../shared/models/videos/thumbnail.type'
+import { VideoStreamingPlaylistType } from '../../../shared/models/videos/video-streaming-playlist.type'
+import { buildRemoteVideoBaseUrl, checkUrlsSameHost, getAPId } from '../../helpers/activitypub'
 import { isAPVideoFileMetadataObject, sanitizeAndCheckVideoTorrentObject } from '../../helpers/custom-validators/activitypub/videos'
+import { isArray } from '../../helpers/custom-validators/misc'
 import { isVideoFileInfoHashValid } from '../../helpers/custom-validators/videos'
 import { deleteNonExistingModels, resetSequelizeInstance, retryTransactionWrapper } from '../../helpers/database-utils'
 import { logger } from '../../helpers/logger'
 import { doRequest } from '../../helpers/requests'
+import { fetchVideoByUrl, getExtFromMimetype, VideoFetchByUrlType } from '../../helpers/video'
 import {
   ACTIVITY_PUB,
   MIMETYPES,
@@ -28,33 +36,15 @@ import {
   STATIC_PATHS,
   THUMBNAILS_SIZE
 } from '../../initializers/constants'
+import { sequelizeTypescript } from '../../initializers/database'
+import { AccountVideoRateModel } from '../../models/account/account-video-rate'
 import { TagModel } from '../../models/video/tag'
 import { VideoModel } from '../../models/video/video'
-import { VideoFileModel } from '../../models/video/video-file'
-import { getOrCreateActorAndServerAndModel } from './actor'
-import { addVideoComments } from './video-comments'
-import { crawlCollectionPage } from './crawl'
-import { sendCreateVideo, sendUpdateVideo } from './send'
-import { isArray } from '../../helpers/custom-validators/misc'
 import { VideoCaptionModel } from '../../models/video/video-caption'
-import { JobQueue } from '../job-queue'
-import { createRates } from './video-rates'
-import { addVideoShares, shareVideoByServerAndChannel } from './share'
-import { fetchVideoByUrl, VideoFetchByUrlType } from '../../helpers/video'
-import { buildRemoteVideoBaseUrl, checkUrlsSameHost, getAPId } from '../../helpers/activitypub'
-import { Notifier } from '../notifier'
-import { VideoStreamingPlaylistModel } from '../../models/video/video-streaming-playlist'
-import { VideoStreamingPlaylistType } from '../../../shared/models/videos/video-streaming-playlist.type'
-import { AccountVideoRateModel } from '../../models/account/account-video-rate'
-import { VideoShareModel } from '../../models/video/video-share'
 import { VideoCommentModel } from '../../models/video/video-comment'
-import { sequelizeTypescript } from '../../initializers/database'
-import { createPlaceholderThumbnail, createVideoMiniatureFromUrl } from '../thumbnail'
-import { ThumbnailType } from '../../../shared/models/videos/thumbnail.type'
-import { join } from 'path'
-import { FilteredModelAttributes } from '../../types/sequelize'
-import { autoBlacklistVideoIfNeeded } from '../video-blacklist'
-import { ActorFollowScoreCache } from '../files-cache'
+import { VideoFileModel } from '../../models/video/video-file'
+import { VideoShareModel } from '../../models/video/video-share'
+import { VideoStreamingPlaylistModel } from '../../models/video/video-streaming-playlist'
 import {
   MAccountIdActor,
   MChannelAccountLight,
@@ -73,7 +63,18 @@ import {
   MVideoThumbnail
 } from '../../types/models'
 import { MThumbnail } from '../../types/models/video/thumbnail'
-import { maxBy, minBy } from 'lodash'
+import { FilteredModelAttributes } from '../../types/sequelize'
+import { ActorFollowScoreCache } from '../files-cache'
+import { JobQueue } from '../job-queue'
+import { Notifier } from '../notifier'
+import { createPlaceholderThumbnail, createVideoMiniatureFromUrl } from '../thumbnail'
+import { autoBlacklistVideoIfNeeded } from '../video-blacklist'
+import { getOrCreateActorAndServerAndModel } from './actor'
+import { crawlCollectionPage } from './crawl'
+import { sendCreateVideo, sendUpdateVideo } from './send'
+import { addVideoShares, shareVideoByServerAndChannel } from './share'
+import { addVideoComments } from './video-comments'
+import { createRates } from './video-rates'
 
 async function federateVideoIfNeeded (videoArg: MVideoAPWithoutCaption, isNewVideo: boolean, transaction?: sequelize.Transaction) {
   const video = videoArg as MVideoAP
@@ -516,10 +517,9 @@ export {
 // ---------------------------------------------------------------------------
 
 function isAPVideoUrlObject (url: any): url is ActivityVideoUrlObject {
-  const mimeTypes = Object.keys(MIMETYPES.VIDEO.MIMETYPE_EXT)
-
   const urlMediaType = url.mediaType
-  return mimeTypes.includes(urlMediaType) && urlMediaType.startsWith('video/')
+
+  return MIMETYPES.VIDEO.MIMETYPE_EXT[urlMediaType] && urlMediaType.startsWith('video/')
 }
 
 function isAPStreamingPlaylistUrlObject (url: ActivityUrlObject): url is ActivityPlaylistUrlObject {
@@ -716,7 +716,7 @@ function videoFileActivityUrlToDBAttributes (
 
     const mediaType = fileUrl.mediaType
     const attribute = {
-      extname: MIMETYPES.VIDEO.MIMETYPE_EXT[mediaType],
+      extname: getExtFromMimetype(MIMETYPES.VIDEO.MIMETYPE_EXT, mediaType),
       infoHash: parsed.infoHash,
       resolution: fileUrl.height,
       size: fileUrl.size,
diff --git a/server/tests/api/server/config.ts b/server/tests/api/server/config.ts
index f5183042c..60efd332c 100644
--- a/server/tests/api/server/config.ts
+++ b/server/tests/api/server/config.ts
@@ -363,10 +363,12 @@ describe('Test config', function () {
   })
 
   it('Should have the correct updated video allowed extensions', async function () {
+    this.timeout(10000)
+
     const res = await getConfig(server.url)
     const data: ServerConfig = res.body
 
-    expect(data.video.file.extensions).to.have.length.above(3)
+    expect(data.video.file.extensions).to.have.length.above(4)
     expect(data.video.file.extensions).to.contain('.mp4')
     expect(data.video.file.extensions).to.contain('.webm')
     expect(data.video.file.extensions).to.contain('.ogv')