aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorChocobozzz <me@florianbigard.com>2021-11-10 09:42:37 +0100
committerChocobozzz <me@florianbigard.com>2021-11-10 09:43:33 +0100
commit020d3d3d79338148873cfd78ba59856f63260f2f (patch)
tree343218cafb3573d7285c4c4616d629f398009913
parent1868ff3db93a8f2e4ceb91aa3600b633ca00b4f7 (diff)
downloadPeerTube-020d3d3d79338148873cfd78ba59856f63260f2f.tar.gz
PeerTube-020d3d3d79338148873cfd78ba59856f63260f2f.tar.zst
PeerTube-020d3d3d79338148873cfd78ba59856f63260f2f.zip
Remove resumable cache after upload success
-rw-r--r--server/controllers/api/videos/upload.ts26
-rw-r--r--server/lib/redis.ts4
-rw-r--r--server/middlewares/validators/videos/videos.ts19
-rw-r--r--server/tests/api/videos/resumable-upload.ts79
-rw-r--r--shared/extra-utils/videos/videos-command.ts14
5 files changed, 124 insertions, 18 deletions
diff --git a/server/controllers/api/videos/upload.ts b/server/controllers/api/videos/upload.ts
index 02aadd426..3e9979330 100644
--- a/server/controllers/api/videos/upload.ts
+++ b/server/controllers/api/videos/upload.ts
@@ -19,7 +19,7 @@ import { VideoPathManager } from '@server/lib/video-path-manager'
19import { buildNextVideoState } from '@server/lib/video-state' 19import { buildNextVideoState } from '@server/lib/video-state'
20import { openapiOperationDoc } from '@server/middlewares/doc' 20import { openapiOperationDoc } from '@server/middlewares/doc'
21import { MVideo, MVideoFile, MVideoFullLight } from '@server/types/models' 21import { MVideo, MVideoFile, MVideoFullLight } from '@server/types/models'
22import { uploadx } from '@uploadx/core' 22import { Uploadx } from '@uploadx/core'
23import { VideoCreate, VideoState } from '../../../../shared' 23import { VideoCreate, VideoState } from '../../../../shared'
24import { HttpStatusCode } from '../../../../shared/models' 24import { HttpStatusCode } from '../../../../shared/models'
25import { auditLoggerFactory, getAuditIdFromRes, VideoAuditView } from '../../../helpers/audit-logger' 25import { auditLoggerFactory, getAuditIdFromRes, VideoAuditView } from '../../../helpers/audit-logger'
@@ -41,6 +41,7 @@ import {
41 authenticate, 41 authenticate,
42 videosAddLegacyValidator, 42 videosAddLegacyValidator,
43 videosAddResumableInitValidator, 43 videosAddResumableInitValidator,
44 videosResumableUploadIdValidator,
44 videosAddResumableValidator 45 videosAddResumableValidator
45} from '../../../middlewares' 46} from '../../../middlewares'
46import { ScheduleVideoUpdateModel } from '../../../models/video/schedule-video-update' 47import { ScheduleVideoUpdateModel } from '../../../models/video/schedule-video-update'
@@ -50,7 +51,9 @@ import { VideoFileModel } from '../../../models/video/video-file'
50const lTags = loggerTagsFactory('api', 'video') 51const lTags = loggerTagsFactory('api', 'video')
51const auditLogger = auditLoggerFactory('videos') 52const auditLogger = auditLoggerFactory('videos')
52const uploadRouter = express.Router() 53const uploadRouter = express.Router()
53const uploadxMiddleware = uploadx.upload({ directory: getResumableUploadPath() }) 54
55const uploadx = new Uploadx({ directory: getResumableUploadPath() })
56uploadx.getUserId = (_, res: express.Response) => res.locals.oauth?.token.user.id
54 57
55const reqVideoFileAdd = createReqFiles( 58const reqVideoFileAdd = createReqFiles(
56 [ 'videofile', 'thumbnailfile', 'previewfile' ], 59 [ 'videofile', 'thumbnailfile', 'previewfile' ],
@@ -84,18 +87,21 @@ uploadRouter.post('/upload-resumable',
84 authenticate, 87 authenticate,
85 reqVideoFileAddResumable, 88 reqVideoFileAddResumable,
86 asyncMiddleware(videosAddResumableInitValidator), 89 asyncMiddleware(videosAddResumableInitValidator),
87 uploadxMiddleware 90 uploadx.upload
88) 91)
89 92
90uploadRouter.delete('/upload-resumable', 93uploadRouter.delete('/upload-resumable',
91 authenticate, 94 authenticate,
92 uploadxMiddleware 95 videosResumableUploadIdValidator,
96 asyncMiddleware(deleteUploadResumableCache),
97 uploadx.upload
93) 98)
94 99
95uploadRouter.put('/upload-resumable', 100uploadRouter.put('/upload-resumable',
96 openapiOperationDoc({ operationId: 'uploadResumable' }), 101 openapiOperationDoc({ operationId: 'uploadResumable' }),
97 authenticate, 102 authenticate,
98 uploadxMiddleware, // uploadx doesn't next() before the file upload completes 103 videosResumableUploadIdValidator,
104 uploadx.upload, // uploadx doesn't next() before the file upload completes
99 asyncMiddleware(videosAddResumableValidator), 105 asyncMiddleware(videosAddResumableValidator),
100 asyncMiddleware(addVideoResumable) 106 asyncMiddleware(addVideoResumable)
101) 107)
@@ -108,7 +114,7 @@ export {
108 114
109// --------------------------------------------------------------------------- 115// ---------------------------------------------------------------------------
110 116
111export async function addVideoLegacy (req: express.Request, res: express.Response) { 117async function addVideoLegacy (req: express.Request, res: express.Response) {
112 // Uploading the video could be long 118 // Uploading the video could be long
113 // Set timeout to 10 minutes, as Express's default is 2 minutes 119 // Set timeout to 10 minutes, as Express's default is 2 minutes
114 req.setTimeout(1000 * 60 * 10, () => { 120 req.setTimeout(1000 * 60 * 10, () => {
@@ -128,7 +134,7 @@ export async function addVideoLegacy (req: express.Request, res: express.Respons
128 return res.json(response) 134 return res.json(response)
129} 135}
130 136
131export async function addVideoResumable (req: express.Request, res: express.Response) { 137async function addVideoResumable (req: express.Request, res: express.Response) {
132 const videoPhysicalFile = res.locals.videoFileResumable 138 const videoPhysicalFile = res.locals.videoFileResumable
133 const videoInfo = videoPhysicalFile.metadata 139 const videoInfo = videoPhysicalFile.metadata
134 const files = { previewfile: videoInfo.previewfile } 140 const files = { previewfile: videoInfo.previewfile }
@@ -291,3 +297,9 @@ function createTorrentFederate (video: MVideoFullLight, videoFile: MVideoFile) {
291 }) 297 })
292 .catch(err => logger.error('Cannot federate or notify video creation %s', video.url, { err, ...lTags(video.uuid) })) 298 .catch(err => logger.error('Cannot federate or notify video creation %s', video.url, { err, ...lTags(video.uuid) }))
293} 299}
300
301async function deleteUploadResumableCache (req: express.Request, res: express.Response, next: express.NextFunction) {
302 await Redis.Instance.deleteUploadSession(req.query.upload_id)
303
304 return next()
305}
diff --git a/server/lib/redis.ts b/server/lib/redis.ts
index 76b7868e8..8aec4b793 100644
--- a/server/lib/redis.ts
+++ b/server/lib/redis.ts
@@ -271,6 +271,10 @@ class Redis {
271 : '' 271 : ''
272 } 272 }
273 273
274 deleteUploadSession (uploadId: string) {
275 return this.deleteKey('resumable-upload-' + uploadId)
276 }
277
274 /* ************ Keys generation ************ */ 278 /* ************ Keys generation ************ */
275 279
276 generateCachedRouteKey (req: express.Request) { 280 generateCachedRouteKey (req: express.Request) {
diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts
index 5f1234379..53643635c 100644
--- a/server/middlewares/validators/videos/videos.ts
+++ b/server/middlewares/validators/videos/videos.ts
@@ -103,6 +103,22 @@ const videosAddLegacyValidator = getCommonVideoEditAttributes().concat([
103 } 103 }
104]) 104])
105 105
106const videosResumableUploadIdValidator = [
107 (req: express.Request, res: express.Response, next: express.NextFunction) => {
108 const user = res.locals.oauth.token.User
109 const uploadId = req.query.upload_id
110
111 if (uploadId.startsWith(user.id + '-') !== true) {
112 return res.fail({
113 status: HttpStatusCode.FORBIDDEN_403,
114 message: 'You cannot send chunks in another user upload'
115 })
116 }
117
118 return next()
119 }
120]
121
106/** 122/**
107 * Gets called after the last PUT request 123 * Gets called after the last PUT request
108 */ 124 */
@@ -110,7 +126,7 @@ const videosAddResumableValidator = [
110 async (req: express.Request, res: express.Response, next: express.NextFunction) => { 126 async (req: express.Request, res: express.Response, next: express.NextFunction) => {
111 const user = res.locals.oauth.token.User 127 const user = res.locals.oauth.token.User
112 const body: express.CustomUploadXFile<express.UploadXFileMetadata> = req.body 128 const body: express.CustomUploadXFile<express.UploadXFileMetadata> = req.body
113 const file = { ...body, duration: undefined, path: getResumableUploadPath(body.id), filename: body.metadata.filename } 129 const file = { ...body, duration: undefined, path: getResumableUploadPath(body.name), filename: body.metadata.filename }
114 const cleanup = () => deleteFileAndCatch(file.path) 130 const cleanup = () => deleteFileAndCatch(file.path)
115 131
116 const uploadId = req.query.upload_id 132 const uploadId = req.query.upload_id
@@ -552,6 +568,7 @@ export {
552 videosAddLegacyValidator, 568 videosAddLegacyValidator,
553 videosAddResumableValidator, 569 videosAddResumableValidator,
554 videosAddResumableInitValidator, 570 videosAddResumableInitValidator,
571 videosResumableUploadIdValidator,
555 572
556 videosUpdateValidator, 573 videosUpdateValidator,
557 videosGetValidator, 574 videosGetValidator,
diff --git a/server/tests/api/videos/resumable-upload.ts b/server/tests/api/videos/resumable-upload.ts
index 6b5e0c09d..1ba7cdbcc 100644
--- a/server/tests/api/videos/resumable-upload.ts
+++ b/server/tests/api/videos/resumable-upload.ts
@@ -22,6 +22,8 @@ describe('Test resumable upload', function () {
22 const defaultFixture = 'video_short.mp4' 22 const defaultFixture = 'video_short.mp4'
23 let server: PeerTubeServer 23 let server: PeerTubeServer
24 let rootId: number 24 let rootId: number
25 let userAccessToken: string
26 let userChannelId: number
25 27
26 async function buildSize (fixture: string, size?: number) { 28 async function buildSize (fixture: string, size?: number) {
27 if (size !== undefined) return size 29 if (size !== undefined) return size
@@ -30,24 +32,33 @@ describe('Test resumable upload', function () {
30 return (await stat(baseFixture)).size 32 return (await stat(baseFixture)).size
31 } 33 }
32 34
33 async function prepareUpload (sizeArg?: number) { 35 async function prepareUpload (options: {
34 const size = await buildSize(defaultFixture, sizeArg) 36 channelId?: number
37 token?: string
38 size?: number
39 originalName?: string
40 lastModified?: number
41 } = {}) {
42 const { token, originalName, lastModified } = options
43
44 const size = await buildSize(defaultFixture, options.size)
35 45
36 const attributes = { 46 const attributes = {
37 name: 'video', 47 name: 'video',
38 channelId: server.store.channel.id, 48 channelId: options.channelId ?? server.store.channel.id,
39 privacy: VideoPrivacy.PUBLIC, 49 privacy: VideoPrivacy.PUBLIC,
40 fixture: defaultFixture 50 fixture: defaultFixture
41 } 51 }
42 52
43 const mimetype = 'video/mp4' 53 const mimetype = 'video/mp4'
44 54
45 const res = await server.videos.prepareResumableUpload({ attributes, size, mimetype }) 55 const res = await server.videos.prepareResumableUpload({ token, attributes, size, mimetype, originalName, lastModified })
46 56
47 return res.header['location'].split('?')[1] 57 return res.header['location'].split('?')[1]
48 } 58 }
49 59
50 async function sendChunks (options: { 60 async function sendChunks (options: {
61 token?: string
51 pathUploadId: string 62 pathUploadId: string
52 size?: number 63 size?: number
53 expectedStatus?: HttpStatusCode 64 expectedStatus?: HttpStatusCode
@@ -55,12 +66,13 @@ describe('Test resumable upload', function () {
55 contentRange?: string 66 contentRange?: string
56 contentRangeBuilder?: (start: number, chunk: any) => string 67 contentRangeBuilder?: (start: number, chunk: any) => string
57 }) { 68 }) {
58 const { pathUploadId, expectedStatus, contentLength, contentRangeBuilder } = options 69 const { token, pathUploadId, expectedStatus, contentLength, contentRangeBuilder } = options
59 70
60 const size = await buildSize(defaultFixture, options.size) 71 const size = await buildSize(defaultFixture, options.size)
61 const absoluteFilePath = buildAbsoluteFixturePath(defaultFixture) 72 const absoluteFilePath = buildAbsoluteFixturePath(defaultFixture)
62 73
63 return server.videos.sendResumableChunks({ 74 return server.videos.sendResumableChunks({
75 token,
64 pathUploadId, 76 pathUploadId,
65 videoFilePath: absoluteFilePath, 77 videoFilePath: absoluteFilePath,
66 size, 78 size,
@@ -105,6 +117,12 @@ describe('Test resumable upload', function () {
105 const body = await server.users.getMyInfo() 117 const body = await server.users.getMyInfo()
106 rootId = body.id 118 rootId = body.id
107 119
120 {
121 userAccessToken = await server.users.generateUserAndToken('user1')
122 const { videoChannels } = await server.users.getMyInfo({ token: userAccessToken })
123 userChannelId = videoChannels[0].id
124 }
125
108 await server.users.update({ userId: rootId, videoQuota: 10_000_000 }) 126 await server.users.update({ userId: rootId, videoQuota: 10_000_000 })
109 }) 127 })
110 128
@@ -147,14 +165,14 @@ describe('Test resumable upload', function () {
147 }) 165 })
148 166
149 it('Should not accept more chunks than expected', async function () { 167 it('Should not accept more chunks than expected', async function () {
150 const uploadId = await prepareUpload(100) 168 const uploadId = await prepareUpload({ size: 100 })
151 169
152 await sendChunks({ pathUploadId: uploadId, expectedStatus: HttpStatusCode.CONFLICT_409 }) 170 await sendChunks({ pathUploadId: uploadId, expectedStatus: HttpStatusCode.CONFLICT_409 })
153 await checkFileSize(uploadId, 0) 171 await checkFileSize(uploadId, 0)
154 }) 172 })
155 173
156 it('Should not accept more chunks than expected with an invalid content length/content range', async function () { 174 it('Should not accept more chunks than expected with an invalid content length/content range', async function () {
157 const uploadId = await prepareUpload(1500) 175 const uploadId = await prepareUpload({ size: 1500 })
158 176
159 // Content length check seems to have changed in v16 177 // Content length check seems to have changed in v16
160 if (process.version.startsWith('v16')) { 178 if (process.version.startsWith('v16')) {
@@ -167,7 +185,7 @@ describe('Test resumable upload', function () {
167 }) 185 })
168 186
169 it('Should not accept more chunks than expected with an invalid content length', async function () { 187 it('Should not accept more chunks than expected with an invalid content length', async function () {
170 const uploadId = await prepareUpload(500) 188 const uploadId = await prepareUpload({ size: 500 })
171 189
172 const size = 1000 190 const size = 1000
173 191
@@ -195,6 +213,51 @@ describe('Test resumable upload', function () {
195 213
196 await checkFileSize(uploadId, null) 214 await checkFileSize(uploadId, null)
197 }) 215 })
216
217 it('Should not have the same upload id with 2 different users', async function () {
218 const originalName = 'toto.mp4'
219 const lastModified = new Date().getTime()
220
221 const uploadId1 = await prepareUpload({ originalName, lastModified, token: server.accessToken })
222 const uploadId2 = await prepareUpload({ originalName, lastModified, channelId: userChannelId, token: userAccessToken })
223
224 expect(uploadId1).to.not.equal(uploadId2)
225 })
226
227 it('Should have the same upload id with the same user', async function () {
228 const originalName = 'toto.mp4'
229 const lastModified = new Date().getTime()
230
231 const uploadId1 = await prepareUpload({ originalName, lastModified })
232 const uploadId2 = await prepareUpload({ originalName, lastModified })
233
234 expect(uploadId1).to.equal(uploadId2)
235 })
236
237 it('Should not cache a request with 2 different users', async function () {
238 const originalName = 'toto.mp4'
239 const lastModified = new Date().getTime()
240
241 const uploadId = await prepareUpload({ originalName, lastModified, token: server.accessToken })
242
243 await sendChunks({ pathUploadId: uploadId, token: server.accessToken })
244 await sendChunks({ pathUploadId: uploadId, token: userAccessToken, expectedStatus: HttpStatusCode.FORBIDDEN_403 })
245 })
246
247 it('Should not cache a request after a delete', async function () {
248 const originalName = 'toto.mp4'
249 const lastModified = new Date().getTime()
250 const uploadId1 = await prepareUpload({ originalName, lastModified, token: server.accessToken })
251
252 await sendChunks({ pathUploadId: uploadId1 })
253 await server.videos.endResumableUpload({ pathUploadId: uploadId1 })
254
255 const uploadId2 = await prepareUpload({ originalName, lastModified, token: server.accessToken })
256 expect(uploadId1).to.equal(uploadId2)
257
258 const result2 = await sendChunks({ pathUploadId: uploadId1 })
259 expect(result2.headers['x-resumable-upload-cached']).to.not.exist
260 })
198 }) 261 })
199 262
200 after(async function () { 263 after(async function () {
diff --git a/shared/extra-utils/videos/videos-command.ts b/shared/extra-utils/videos/videos-command.ts
index 68241f062..167fae22d 100644
--- a/shared/extra-utils/videos/videos-command.ts
+++ b/shared/extra-utils/videos/videos-command.ts
@@ -469,8 +469,11 @@ export class VideosCommand extends AbstractCommand {
469 attributes: VideoEdit 469 attributes: VideoEdit
470 size: number 470 size: number
471 mimetype: string 471 mimetype: string
472
473 originalName?: string
474 lastModified?: number
472 }) { 475 }) {
473 const { attributes, size, mimetype } = options 476 const { attributes, originalName, lastModified, size, mimetype } = options
474 477
475 const path = '/api/v1/videos/upload-resumable' 478 const path = '/api/v1/videos/upload-resumable'
476 479
@@ -482,7 +485,14 @@ export class VideosCommand extends AbstractCommand {
482 'X-Upload-Content-Type': mimetype, 485 'X-Upload-Content-Type': mimetype,
483 'X-Upload-Content-Length': size.toString() 486 'X-Upload-Content-Length': size.toString()
484 }, 487 },
485 fields: { filename: attributes.fixture, ...this.buildUploadFields(options.attributes) }, 488 fields: {
489 filename: attributes.fixture,
490 originalName,
491 lastModified,
492
493 ...this.buildUploadFields(options.attributes)
494 },
495
486 // Fixture will be sent later 496 // Fixture will be sent later
487 attaches: this.buildUploadAttaches(omit(options.attributes, 'fixture')), 497 attaches: this.buildUploadAttaches(omit(options.attributes, 'fixture')),
488 implicitToken: true, 498 implicitToken: true,