aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorRigel Kent <sendmemail@rigelk.eu>2021-10-25 17:42:20 +0200
committerGitHub <noreply@github.com>2021-10-25 17:42:20 +0200
commit276250f0a36e00373166d91d539e5220d6f158c7 (patch)
tree394e4fd65912edbbe9266ccfbacfc14f433371e7
parentb2ad0090c182c7f2a8cba1cced3987d408a4b159 (diff)
downloadPeerTube-276250f0a36e00373166d91d539e5220d6f158c7.tar.gz
PeerTube-276250f0a36e00373166d91d539e5220d6f158c7.tar.zst
PeerTube-276250f0a36e00373166d91d539e5220d6f158c7.zip
prevent multiple post-process triggering of upload-resumable (#4175)
* prevent multiple post-process triggering of upload-resumable * switch from 409 to 503 for upload being processed * Improve resumable upload check Co-authored-by: Chocobozzz <me@florianbigard.com>
-rw-r--r--client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts7
-rw-r--r--server/controllers/api/videos/upload.ts18
-rw-r--r--server/helpers/upload.ts9
-rw-r--r--server/initializers/constants.ts3
-rw-r--r--server/lib/job-queue/job-queue.ts2
-rw-r--r--server/lib/redis.ts27
-rw-r--r--server/middlewares/validators/videos/videos.ts28
-rw-r--r--server/tests/api/videos/resumable-upload.ts15
-rw-r--r--shared/models/server/job.model.ts4
-rw-r--r--support/doc/api/openapi.yaml7
10 files changed, 107 insertions, 13 deletions
diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts
index 6f72a07c4..91d89a535 100644
--- a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts
+++ b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts
@@ -82,9 +82,10 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy
82 uploaderClass: UploaderXFormData, 82 uploaderClass: UploaderXFormData,
83 chunkSize, 83 chunkSize,
84 retryConfig: { 84 retryConfig: {
85 maxAttempts: 6, 85 maxAttempts: 30, // maximum attempts for 503 codes, otherwise set to 6, see below
86 shouldRetry: (code: number) => { 86 maxDelay: 120_000, // 2 min
87 return code < 400 || code >= 501 87 shouldRetry: (code: number, attempts: number) => {
88 return code === HttpStatusCode.SERVICE_UNAVAILABLE_503 || ((code < 400 || code > 500) && attempts < 6)
88 } 89 }
89 } 90 }
90 } 91 }
diff --git a/server/controllers/api/videos/upload.ts b/server/controllers/api/videos/upload.ts
index 55cb9cf20..02aadd426 100644
--- a/server/controllers/api/videos/upload.ts
+++ b/server/controllers/api/videos/upload.ts
@@ -7,6 +7,7 @@ import { uuidToShort } from '@server/helpers/uuid'
7import { createTorrentAndSetInfoHash } from '@server/helpers/webtorrent' 7import { createTorrentAndSetInfoHash } from '@server/helpers/webtorrent'
8import { getLocalVideoActivityPubUrl } from '@server/lib/activitypub/url' 8import { getLocalVideoActivityPubUrl } from '@server/lib/activitypub/url'
9import { generateWebTorrentVideoFilename } from '@server/lib/paths' 9import { generateWebTorrentVideoFilename } from '@server/lib/paths'
10import { Redis } from '@server/lib/redis'
10import { 11import {
11 addMoveToObjectStorageJob, 12 addMoveToObjectStorageJob,
12 addOptimizeOrMergeAudioJob, 13 addOptimizeOrMergeAudioJob,
@@ -94,7 +95,7 @@ uploadRouter.delete('/upload-resumable',
94uploadRouter.put('/upload-resumable', 95uploadRouter.put('/upload-resumable',
95 openapiOperationDoc({ operationId: 'uploadResumable' }), 96 openapiOperationDoc({ operationId: 'uploadResumable' }),
96 authenticate, 97 authenticate,
97 uploadxMiddleware, // uploadx doesn't use call next() before the file upload completes 98 uploadxMiddleware, // uploadx doesn't next() before the file upload completes
98 asyncMiddleware(videosAddResumableValidator), 99 asyncMiddleware(videosAddResumableValidator),
99 asyncMiddleware(addVideoResumable) 100 asyncMiddleware(addVideoResumable)
100) 101)
@@ -122,15 +123,20 @@ export async function addVideoLegacy (req: express.Request, res: express.Respons
122 const videoInfo: VideoCreate = req.body 123 const videoInfo: VideoCreate = req.body
123 const files = req.files 124 const files = req.files
124 125
125 return addVideo({ res, videoPhysicalFile, videoInfo, files }) 126 const response = await addVideo({ res, videoPhysicalFile, videoInfo, files })
127
128 return res.json(response)
126} 129}
127 130
128export async function addVideoResumable (_req: express.Request, res: express.Response) { 131export async function addVideoResumable (req: express.Request, res: express.Response) {
129 const videoPhysicalFile = res.locals.videoFileResumable 132 const videoPhysicalFile = res.locals.videoFileResumable
130 const videoInfo = videoPhysicalFile.metadata 133 const videoInfo = videoPhysicalFile.metadata
131 const files = { previewfile: videoInfo.previewfile } 134 const files = { previewfile: videoInfo.previewfile }
132 135
133 return addVideo({ res, videoPhysicalFile, videoInfo, files }) 136 const response = await addVideo({ res, videoPhysicalFile, videoInfo, files })
137 await Redis.Instance.setUploadSession(req.query.upload_id, response)
138
139 return res.json(response)
134} 140}
135 141
136async function addVideo (options: { 142async function addVideo (options: {
@@ -225,13 +231,13 @@ async function addVideo (options: {
225 231
226 Hooks.runAction('action:api.video.uploaded', { video: videoCreated }) 232 Hooks.runAction('action:api.video.uploaded', { video: videoCreated })
227 233
228 return res.json({ 234 return {
229 video: { 235 video: {
230 id: videoCreated.id, 236 id: videoCreated.id,
231 shortUUID: uuidToShort(videoCreated.uuid), 237 shortUUID: uuidToShort(videoCreated.uuid),
232 uuid: videoCreated.uuid 238 uuid: videoCreated.uuid
233 } 239 }
234 }) 240 }
235} 241}
236 242
237async function buildNewFile (videoPhysicalFile: express.VideoUploadFile) { 243async function buildNewFile (videoPhysicalFile: express.VideoUploadFile) {
diff --git a/server/helpers/upload.ts b/server/helpers/upload.ts
index 3cb17edd0..c94c7ab82 100644
--- a/server/helpers/upload.ts
+++ b/server/helpers/upload.ts
@@ -1,4 +1,5 @@
1import { join } from 'path' 1import { join } from 'path'
2import { JobQueue } from '@server/lib/job-queue'
2import { RESUMABLE_UPLOAD_DIRECTORY } from '../initializers/constants' 3import { RESUMABLE_UPLOAD_DIRECTORY } from '../initializers/constants'
3 4
4function getResumableUploadPath (filename?: string) { 5function getResumableUploadPath (filename?: string) {
@@ -7,8 +8,14 @@ function getResumableUploadPath (filename?: string) {
7 return RESUMABLE_UPLOAD_DIRECTORY 8 return RESUMABLE_UPLOAD_DIRECTORY
8} 9}
9 10
11function scheduleDeleteResumableUploadMetaFile (filepath: string) {
12 const payload = { filepath }
13 JobQueue.Instance.createJob({ type: 'delete-resumable-upload-meta-file', payload }, { delay: 900 * 1000 }) // executed in 15 min
14}
15
10// --------------------------------------------------------------------------- 16// ---------------------------------------------------------------------------
11 17
12export { 18export {
13 getResumableUploadPath 19 getResumableUploadPath,
20 scheduleDeleteResumableUploadMetaFile
14} 21}
diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts
index 87a74a32c..f6c19dab4 100644
--- a/server/initializers/constants.ts
+++ b/server/initializers/constants.ts
@@ -665,6 +665,8 @@ const RESUMABLE_UPLOAD_DIRECTORY = join(CONFIG.STORAGE.TMP_DIR, 'resumable-uploa
665const HLS_STREAMING_PLAYLIST_DIRECTORY = join(CONFIG.STORAGE.STREAMING_PLAYLISTS_DIR, 'hls') 665const HLS_STREAMING_PLAYLIST_DIRECTORY = join(CONFIG.STORAGE.STREAMING_PLAYLISTS_DIR, 'hls')
666const HLS_REDUNDANCY_DIRECTORY = join(CONFIG.STORAGE.REDUNDANCY_DIR, 'hls') 666const HLS_REDUNDANCY_DIRECTORY = join(CONFIG.STORAGE.REDUNDANCY_DIR, 'hls')
667 667
668const RESUMABLE_UPLOAD_SESSION_LIFETIME = SCHEDULER_INTERVALS_MS.REMOVE_DANGLING_RESUMABLE_UPLOADS
669
668const VIDEO_LIVE = { 670const VIDEO_LIVE = {
669 EXTENSION: '.ts', 671 EXTENSION: '.ts',
670 CLEANUP_DELAY: 1000 * 60 * 5, // 5 minutes 672 CLEANUP_DELAY: 1000 * 60 * 5, // 5 minutes
@@ -838,6 +840,7 @@ export {
838 LAZY_STATIC_PATHS, 840 LAZY_STATIC_PATHS,
839 SEARCH_INDEX, 841 SEARCH_INDEX,
840 RESUMABLE_UPLOAD_DIRECTORY, 842 RESUMABLE_UPLOAD_DIRECTORY,
843 RESUMABLE_UPLOAD_SESSION_LIFETIME,
841 HLS_REDUNDANCY_DIRECTORY, 844 HLS_REDUNDANCY_DIRECTORY,
842 P2P_MEDIA_LOADER_PEER_VERSION, 845 P2P_MEDIA_LOADER_PEER_VERSION,
843 ACTOR_IMAGES_SIZE, 846 ACTOR_IMAGES_SIZE,
diff --git a/server/lib/job-queue/job-queue.ts b/server/lib/job-queue/job-queue.ts
index 4cda12b57..53d6b6a9c 100644
--- a/server/lib/job-queue/job-queue.ts
+++ b/server/lib/job-queue/job-queue.ts
@@ -8,6 +8,7 @@ import {
8 ActivitypubHttpFetcherPayload, 8 ActivitypubHttpFetcherPayload,
9 ActivitypubHttpUnicastPayload, 9 ActivitypubHttpUnicastPayload,
10 ActorKeysPayload, 10 ActorKeysPayload,
11 DeleteResumableUploadMetaFilePayload,
11 EmailPayload, 12 EmailPayload,
12 JobState, 13 JobState,
13 JobType, 14 JobType,
@@ -52,6 +53,7 @@ type CreateJobArgument =
52 { type: 'video-live-ending', payload: VideoLiveEndingPayload } | 53 { type: 'video-live-ending', payload: VideoLiveEndingPayload } |
53 { type: 'actor-keys', payload: ActorKeysPayload } | 54 { type: 'actor-keys', payload: ActorKeysPayload } |
54 { type: 'video-redundancy', payload: VideoRedundancyPayload } | 55 { type: 'video-redundancy', payload: VideoRedundancyPayload } |
56 { type: 'delete-resumable-upload-meta-file', payload: DeleteResumableUploadMetaFilePayload } |
55 { type: 'move-to-object-storage', payload: MoveObjectStoragePayload } 57 { type: 'move-to-object-storage', payload: MoveObjectStoragePayload }
56 58
57export type CreateJobOptions = { 59export type CreateJobOptions = {
diff --git a/server/lib/redis.ts b/server/lib/redis.ts
index d1d88d853..46617b07e 100644
--- a/server/lib/redis.ts
+++ b/server/lib/redis.ts
@@ -9,7 +9,8 @@ import {
9 USER_PASSWORD_CREATE_LIFETIME, 9 USER_PASSWORD_CREATE_LIFETIME,
10 VIEW_LIFETIME, 10 VIEW_LIFETIME,
11 WEBSERVER, 11 WEBSERVER,
12 TRACKER_RATE_LIMITS 12 TRACKER_RATE_LIMITS,
13 RESUMABLE_UPLOAD_SESSION_LIFETIME
13} from '../initializers/constants' 14} from '../initializers/constants'
14import { CONFIG } from '../initializers/config' 15import { CONFIG } from '../initializers/config'
15 16
@@ -202,6 +203,30 @@ class Redis {
202 ]) 203 ])
203 } 204 }
204 205
206 /* ************ Resumable uploads final responses ************ */
207
208 setUploadSession (uploadId: string, response?: { video: { id: number, shortUUID: string, uuid: string } }) {
209 return this.setValue(
210 'resumable-upload-' + uploadId,
211 response
212 ? JSON.stringify(response)
213 : '',
214 RESUMABLE_UPLOAD_SESSION_LIFETIME
215 )
216 }
217
218 doesUploadSessionExist (uploadId: string) {
219 return this.exists('resumable-upload-' + uploadId)
220 }
221
222 async getUploadSession (uploadId: string) {
223 const value = await this.getValue('resumable-upload-' + uploadId)
224
225 return value
226 ? JSON.parse(value)
227 : ''
228 }
229
205 /* ************ Keys generation ************ */ 230 /* ************ Keys generation ************ */
206 231
207 generateCachedRouteKey (req: express.Request) { 232 generateCachedRouteKey (req: express.Request) {
diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts
index 23ee9778a..e486887a7 100644
--- a/server/middlewares/validators/videos/videos.ts
+++ b/server/middlewares/validators/videos/videos.ts
@@ -1,6 +1,8 @@
1import express from 'express' 1import express from 'express'
2import { body, header, param, query, ValidationChain } from 'express-validator' 2import { body, header, param, query, ValidationChain } from 'express-validator'
3import { isTestInstance } from '@server/helpers/core-utils'
3import { getResumableUploadPath } from '@server/helpers/upload' 4import { getResumableUploadPath } from '@server/helpers/upload'
5import { Redis } from '@server/lib/redis'
4import { isAbleToUploadVideo } from '@server/lib/user' 6import { isAbleToUploadVideo } from '@server/lib/user'
5import { getServerActor } from '@server/models/application/application' 7import { getServerActor } from '@server/models/application/application'
6import { ExpressPromiseHandler } from '@server/types/express' 8import { ExpressPromiseHandler } from '@server/types/express'
@@ -105,12 +107,34 @@ const videosAddLegacyValidator = getCommonVideoEditAttributes().concat([
105const videosAddResumableValidator = [ 107const videosAddResumableValidator = [
106 async (req: express.Request, res: express.Response, next: express.NextFunction) => { 108 async (req: express.Request, res: express.Response, next: express.NextFunction) => {
107 const user = res.locals.oauth.token.User 109 const user = res.locals.oauth.token.User
108
109 const body: express.CustomUploadXFile<express.UploadXFileMetadata> = req.body 110 const body: express.CustomUploadXFile<express.UploadXFileMetadata> = req.body
110 const file = { ...body, duration: undefined, path: getResumableUploadPath(body.id), filename: body.metadata.filename } 111 const file = { ...body, duration: undefined, path: getResumableUploadPath(body.id), filename: body.metadata.filename }
111
112 const cleanup = () => deleteFileAndCatch(file.path) 112 const cleanup = () => deleteFileAndCatch(file.path)
113 113
114 const uploadId = req.query.upload_id
115 const sessionExists = await Redis.Instance.doesUploadSessionExist(uploadId)
116
117 if (sessionExists) {
118 const sessionResponse = await Redis.Instance.getUploadSession(uploadId)
119
120 if (!sessionResponse) {
121 res.setHeader('Retry-After', 300) // ask to retry after 5 min, knowing the upload_id is kept for up to 15 min after completion
122
123 return res.fail({
124 status: HttpStatusCode.SERVICE_UNAVAILABLE_503,
125 message: 'The upload is already being processed'
126 })
127 }
128
129 if (isTestInstance()) {
130 res.setHeader('x-resumable-upload-cached', 'true')
131 }
132
133 return res.json(sessionResponse)
134 }
135
136 await Redis.Instance.setUploadSession(uploadId)
137
114 if (!await doesVideoChannelOfAccountExist(file.metadata.channelId, user, res)) return cleanup() 138 if (!await doesVideoChannelOfAccountExist(file.metadata.channelId, user, res)) return cleanup()
115 139
116 try { 140 try {
diff --git a/server/tests/api/videos/resumable-upload.ts b/server/tests/api/videos/resumable-upload.ts
index 59970aa94..6b5e0c09d 100644
--- a/server/tests/api/videos/resumable-upload.ts
+++ b/server/tests/api/videos/resumable-upload.ts
@@ -180,6 +180,21 @@ describe('Test resumable upload', function () {
180 await sendChunks({ pathUploadId: uploadId, expectedStatus, contentRangeBuilder, contentLength: size }) 180 await sendChunks({ pathUploadId: uploadId, expectedStatus, contentRangeBuilder, contentLength: size })
181 await checkFileSize(uploadId, 0) 181 await checkFileSize(uploadId, 0)
182 }) 182 })
183
184 it('Should be able to accept 2 PUT requests', async function () {
185 const uploadId = await prepareUpload()
186
187 const result1 = await sendChunks({ pathUploadId: uploadId })
188 const result2 = await sendChunks({ pathUploadId: uploadId })
189
190 expect(result1.body.video.uuid).to.exist
191 expect(result1.body.video.uuid).to.equal(result2.body.video.uuid)
192
193 expect(result1.headers['x-resumable-upload-cached']).to.not.exist
194 expect(result2.headers['x-resumable-upload-cached']).to.equal('true')
195
196 await checkFileSize(uploadId, null)
197 })
183 }) 198 })
184 199
185 after(async function () { 200 after(async function () {
diff --git a/shared/models/server/job.model.ts b/shared/models/server/job.model.ts
index ff96283a4..12e0fcf85 100644
--- a/shared/models/server/job.model.ts
+++ b/shared/models/server/job.model.ts
@@ -138,6 +138,10 @@ export interface ActorKeysPayload {
138 actorId: number 138 actorId: number
139} 139}
140 140
141export interface DeleteResumableUploadMetaFilePayload {
142 filepath: string
143}
144
141export interface MoveObjectStoragePayload { 145export interface MoveObjectStoragePayload {
142 videoUUID: string 146 videoUUID: string
143 isNewVideo: boolean 147 isNewVideo: boolean
diff --git a/support/doc/api/openapi.yaml b/support/doc/api/openapi.yaml
index d6f8c1ae0..ef4e7d04d 100644
--- a/support/doc/api/openapi.yaml
+++ b/support/doc/api/openapi.yaml
@@ -2081,6 +2081,13 @@ paths:
2081 description: video unreadable 2081 description: video unreadable
2082 '429': 2082 '429':
2083 description: too many concurrent requests 2083 description: too many concurrent requests
2084 '503':
2085 description: upload is already being processed
2086 headers:
2087 'Retry-After':
2088 schema:
2089 type: number
2090 example: 300
2084 delete: 2091 delete:
2085 summary: Cancel the resumable upload of a video, deleting any data uploaded so far 2092 summary: Cancel the resumable upload of a video, deleting any data uploaded so far
2086 description: Uses [a resumable protocol](https://github.com/kukhariev/node-uploadx/blob/master/proto.md) to cancel the upload of a video 2093 description: Uses [a resumable protocol](https://github.com/kukhariev/node-uploadx/blob/master/proto.md) to cancel the upload of a video