From 7373507fa830b0f18cb4cd95dfd923b1600e501d Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 16 Nov 2018 10:05:25 +0100 Subject: Improve video upload error handling --- server/middlewares/validators/videos/videos.ts | 2 ++ 1 file changed, 2 insertions(+) (limited to 'server') diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index 656d161d8..bf21bca8c 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -393,6 +393,8 @@ export { function areErrorsInScheduleUpdate (req: express.Request, res: express.Response) { if (req.body.scheduleUpdate) { if (!req.body.scheduleUpdate.updateAt) { + logger.warn('Invalid parameters: scheduleUpdate.updateAt is mandatory.') + res.status(400) .json({ error: 'Schedule update at is mandatory.' }) -- cgit v1.2.3 From 8d1fa36ad22a21a9b0fb6bf51a27d09954220013 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 16 Nov 2018 11:18:13 +0100 Subject: Do not host remote AP objects --- server/controllers/activitypub/client.ts | 12 +++++++++++- server/middlewares/cache.ts | 7 +++++++ server/models/redundancy/video-redundancy.ts | 7 +++++-- server/tests/api/check-params/user-subscriptions.ts | 5 +++++ 4 files changed, 28 insertions(+), 3 deletions(-) (limited to 'server') diff --git a/server/controllers/activitypub/client.ts b/server/controllers/activitypub/client.ts index ffbf1ba19..a342a48d4 100644 --- a/server/controllers/activitypub/client.ts +++ b/server/controllers/activitypub/client.ts @@ -39,6 +39,7 @@ import { import { VideoCaptionModel } from '../../models/video/video-caption' import { videoRedundancyGetValidator } from '../../middlewares/validators/redundancy' import { getServerActor } from '../../helpers/utils' +import { VideoRedundancyModel } from '../../models/redundancy/video-redundancy' const activityPubClientRouter = express.Router() @@ -164,6 +165,8 @@ function getAccountVideoRate (rateType: VideoRateType) { async function videoController (req: express.Request, res: express.Response, next: express.NextFunction) { const video: VideoModel = res.locals.video + if (video.isOwned() === false) return res.redirect(video.url) + // We need captions to render AP object video.VideoCaptions = await VideoCaptionModel.listVideoCaptions(video.id) @@ -180,6 +183,9 @@ async function videoController (req: express.Request, res: express.Response, nex async function videoAnnounceController (req: express.Request, res: express.Response, next: express.NextFunction) { const share = res.locals.videoShare as VideoShareModel + + if (share.Actor.isOwned() === false) return res.redirect(share.url) + const { activity } = await buildAnnounceWithVideoAudience(share.Actor, share, res.locals.video, undefined) return activityPubResponse(activityPubContextify(activity), res) @@ -252,6 +258,8 @@ async function videoChannelFollowingController (req: express.Request, res: expre async function videoCommentController (req: express.Request, res: express.Response, next: express.NextFunction) { const videoComment: VideoCommentModel = res.locals.videoComment + if (videoComment.isOwned() === false) return res.redirect(videoComment.url) + const threadParentComments = await VideoCommentModel.listThreadParentComments(videoComment, undefined) const isPublic = true // Comments are always public const audience = getAudience(videoComment.Account.Actor, isPublic) @@ -267,7 +275,9 @@ async function videoCommentController (req: express.Request, res: express.Respon } async function videoRedundancyController (req: express.Request, res: express.Response) { - const videoRedundancy = res.locals.videoRedundancy + const videoRedundancy: VideoRedundancyModel = res.locals.videoRedundancy + if (videoRedundancy.isOwned() === false) return res.redirect(videoRedundancy.url) + const serverActor = await getServerActor() const audience = getAudience(serverActor) diff --git a/server/middlewares/cache.ts b/server/middlewares/cache.ts index 1e00fc731..8ffe75700 100644 --- a/server/middlewares/cache.ts +++ b/server/middlewares/cache.ts @@ -19,6 +19,7 @@ function cacheRoute (lifetimeArg: string | number) { logger.debug('No cached results for route %s.', req.originalUrl) const sendSave = res.send.bind(res) + const redirectSave = res.redirect.bind(res) res.send = (body) => { if (res.statusCode >= 200 && res.statusCode < 400) { @@ -38,6 +39,12 @@ function cacheRoute (lifetimeArg: string | number) { return sendSave(body) } + res.redirect = url => { + done() + + return redirectSave(url) + } + return next() } diff --git a/server/models/redundancy/video-redundancy.ts b/server/models/redundancy/video-redundancy.ts index 35e0cd3b1..9de4356b4 100644 --- a/server/models/redundancy/video-redundancy.ts +++ b/server/models/redundancy/video-redundancy.ts @@ -117,8 +117,7 @@ export class VideoRedundancyModel extends Model { @BeforeDestroy static async removeFile (instance: VideoRedundancyModel) { - // Not us - if (!instance.strategy) return + if (!instance.isOwned()) return const videoFile = await VideoFileModel.loadWithVideo(instance.videoFileId) @@ -404,6 +403,10 @@ export class VideoRedundancyModel extends Model { })) } + isOwned () { + return !!this.strategy + } + toActivityPubObject (): CacheFileObject { return { id: this.url, diff --git a/server/tests/api/check-params/user-subscriptions.ts b/server/tests/api/check-params/user-subscriptions.ts index 9fba99ac8..6af7ed43b 100644 --- a/server/tests/api/check-params/user-subscriptions.ts +++ b/server/tests/api/check-params/user-subscriptions.ts @@ -15,6 +15,7 @@ import { userLogin } from '../../utils' import { checkBadCountPagination, checkBadSortPagination, checkBadStartPagination } from '../../utils/requests/check-api-params' +import { waitJobs } from '../../utils/server/jobs' describe('Test user subscriptions API validators', function () { const path = '/api/v1/users/me/subscriptions' @@ -141,6 +142,8 @@ describe('Test user subscriptions API validators', function () { }) it('Should succeed with the correct parameters', async function () { + this.timeout(20000) + await makePostBodyRequest({ url: server.url, path, @@ -148,6 +151,8 @@ describe('Test user subscriptions API validators', function () { fields: { uri: 'user1_channel@localhost:9001' }, statusCodeExpected: 204 }) + + await waitJobs([ server ]) }) }) -- cgit v1.2.3 From 8d4273463fb19d503b1aa0a32dc289f292ed614e Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 16 Nov 2018 15:02:48 +0100 Subject: Check follow constraints when getting a video --- server/controllers/api/videos/index.ts | 2 + server/middlewares/oauth.ts | 16 ++ server/middlewares/validators/videos/videos.ts | 52 ++++-- server/models/video/video.ts | 17 ++ server/tests/api/server/follow-constraints.ts | 215 +++++++++++++++++++++++++ server/tests/api/server/index.ts | 1 + 6 files changed, 292 insertions(+), 11 deletions(-) create mode 100644 server/tests/api/server/follow-constraints.ts (limited to 'server') diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts index e654bdd09..89fd0432f 100644 --- a/server/controllers/api/videos/index.ts +++ b/server/controllers/api/videos/index.ts @@ -31,6 +31,7 @@ import { asyncMiddleware, asyncRetryTransactionMiddleware, authenticate, + checkVideoFollowConstraints, commonVideosFiltersValidator, optionalAuthenticate, paginationValidator, @@ -123,6 +124,7 @@ videosRouter.get('/:id/description', videosRouter.get('/:id', optionalAuthenticate, asyncMiddleware(videosGetValidator), + asyncMiddleware(checkVideoFollowConstraints), getVideo ) videosRouter.post('/:id/views', diff --git a/server/middlewares/oauth.ts b/server/middlewares/oauth.ts index 5233b66bd..8c1df2c3e 100644 --- a/server/middlewares/oauth.ts +++ b/server/middlewares/oauth.ts @@ -28,9 +28,24 @@ function authenticate (req: express.Request, res: express.Response, next: expres }) } +function authenticatePromiseIfNeeded (req: express.Request, res: express.Response) { + return new Promise(resolve => { + // Already authenticated? (or tried to) + if (res.locals.oauth && res.locals.oauth.token.User) return resolve() + + if (res.locals.authenticated === false) return res.sendStatus(401) + + authenticate(req, res, () => { + return resolve() + }) + }) +} + function optionalAuthenticate (req: express.Request, res: express.Response, next: express.NextFunction) { if (req.header('authorization')) return authenticate(req, res, next) + res.locals.authenticated = false + return next() } @@ -53,6 +68,7 @@ function token (req: express.Request, res: express.Response, next: express.NextF export { authenticate, + authenticatePromiseIfNeeded, optionalAuthenticate, token } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index bf21bca8c..051a19e16 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -31,8 +31,8 @@ import { } from '../../../helpers/custom-validators/videos' import { getDurationFromVideoFile } from '../../../helpers/ffmpeg-utils' import { logger } from '../../../helpers/logger' -import { CONSTRAINTS_FIELDS } from '../../../initializers' -import { authenticate } from '../../oauth' +import { CONFIG, CONSTRAINTS_FIELDS } from '../../../initializers' +import { authenticatePromiseIfNeeded } from '../../oauth' import { areValidationErrors } from '../utils' import { cleanUpReqFiles } from '../../../helpers/express-utils' import { VideoModel } from '../../../models/video/video' @@ -43,6 +43,7 @@ import { VideoChangeOwnershipModel } from '../../../models/video/video-change-ow import { AccountModel } from '../../../models/account/account' import { VideoFetchType } from '../../../helpers/video' import { isNSFWQueryValid, isNumberArray, isStringArray } from '../../../helpers/custom-validators/search' +import { getServerActor } from '../../../helpers/utils' const videosAddValidator = getCommonVideoAttributes().concat([ body('videofile') @@ -127,6 +128,31 @@ const videosUpdateValidator = getCommonVideoAttributes().concat([ } ]) +async function checkVideoFollowConstraints (req: express.Request, res: express.Response, next: express.NextFunction) { + const video: VideoModel = res.locals.video + + // Anybody can watch local videos + if (video.isOwned() === true) return next() + + // Logged user + if (res.locals.oauth) { + // Users can search or watch remote videos + if (CONFIG.SEARCH.REMOTE_URI.USERS === true) return next() + } + + // Anybody can search or watch remote videos + if (CONFIG.SEARCH.REMOTE_URI.ANONYMOUS === true) return next() + + // Check our instance follows an actor that shared this video + const serverActor = await getServerActor() + if (await VideoModel.checkVideoHasInstanceFollow(video.id, serverActor.id) === true) return next() + + return res.status(403) + .json({ + error: 'Cannot get this video regarding follow constraints.' + }) +} + const videosCustomGetValidator = (fetchType: VideoFetchType) => { return [ param('id').custom(isIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid id'), @@ -141,17 +167,20 @@ const videosCustomGetValidator = (fetchType: VideoFetchType) => { // Video private or blacklisted if (video.privacy === VideoPrivacy.PRIVATE || video.VideoBlacklist) { - return authenticate(req, res, () => { - const user: UserModel = res.locals.oauth.token.User + await authenticatePromiseIfNeeded(req, res) + + const user: UserModel = res.locals.oauth ? res.locals.oauth.token.User : null - // Only the owner or a user that have blacklist rights can see the video - if (video.VideoChannel.Account.userId !== user.id && !user.hasRight(UserRight.MANAGE_VIDEO_BLACKLIST)) { - return res.status(403) - .json({ error: 'Cannot get this private or blacklisted video.' }) - } + // Only the owner or a user that have blacklist rights can see the video + if ( + !user || + (video.VideoChannel.Account.userId !== user.id && !user.hasRight(UserRight.MANAGE_VIDEO_BLACKLIST)) + ) { + return res.status(403) + .json({ error: 'Cannot get this private or blacklisted video.' }) + } - return next() - }) + return next() } // Video is public, anyone can access it @@ -376,6 +405,7 @@ export { videosAddValidator, videosUpdateValidator, videosGetValidator, + checkVideoFollowConstraints, videosCustomGetValidator, videosRemoveValidator, diff --git a/server/models/video/video.ts b/server/models/video/video.ts index 6c183933b..1e68b380c 100644 --- a/server/models/video/video.ts +++ b/server/models/video/video.ts @@ -1253,6 +1253,23 @@ export class VideoModel extends Model { }) } + static checkVideoHasInstanceFollow (videoId: number, followerActorId: number) { + // Instances only share videos + const query = 'SELECT 1 FROM "videoShare" ' + + 'INNER JOIN "actorFollow" ON "actorFollow"."targetActorId" = "videoShare"."actorId" ' + + 'WHERE "actorFollow"."actorId" = $followerActorId AND "videoShare"."videoId" = $videoId ' + + 'LIMIT 1' + + const options = { + type: Sequelize.QueryTypes.SELECT, + bind: { followerActorId, videoId }, + raw: true + } + + return VideoModel.sequelize.query(query, options) + .then(results => results.length === 1) + } + // threshold corresponds to how many video the field should have to be returned static async getRandomFieldSamples (field: 'category' | 'channelId', threshold: number, count: number) { const serverActor = await getServerActor() diff --git a/server/tests/api/server/follow-constraints.ts b/server/tests/api/server/follow-constraints.ts new file mode 100644 index 000000000..3135fc568 --- /dev/null +++ b/server/tests/api/server/follow-constraints.ts @@ -0,0 +1,215 @@ +/* tslint:disable:no-unused-expression */ + +import * as chai from 'chai' +import 'mocha' +import { doubleFollow, getAccountVideos, getVideo, getVideoChannelVideos, getVideoWithToken } from '../../utils' +import { flushAndRunMultipleServers, killallServers, ServerInfo, setAccessTokensToServers, uploadVideo } from '../../utils/index' +import { unfollow } from '../../utils/server/follows' +import { userLogin } from '../../utils/users/login' +import { createUser } from '../../utils/users/users' + +const expect = chai.expect + +describe('Test follow constraints', function () { + let servers: ServerInfo[] = [] + let video1UUID: string + let video2UUID: string + let userAccessToken: string + + before(async function () { + this.timeout(30000) + + servers = await flushAndRunMultipleServers(2) + + // Get the access tokens + await setAccessTokensToServers(servers) + + { + const res = await uploadVideo(servers[ 0 ].url, servers[ 0 ].accessToken, { name: 'video server 1' }) + video1UUID = res.body.video.uuid + } + { + const res = await uploadVideo(servers[ 1 ].url, servers[ 1 ].accessToken, { name: 'video server 2' }) + video2UUID = res.body.video.uuid + } + + const user = { + username: 'user1', + password: 'super_password' + } + await createUser(servers[0].url, servers[0].accessToken, user.username, user.password) + userAccessToken = await userLogin(servers[0], user) + + await doubleFollow(servers[0], servers[1]) + }) + + describe('With a followed instance', function () { + + describe('With an unlogged user', function () { + + it('Should get the local video', async function () { + await getVideo(servers[0].url, video1UUID, 200) + }) + + it('Should get the remote video', async function () { + await getVideo(servers[0].url, video2UUID, 200) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + }) + + describe('With a logged user', function () { + it('Should get the local video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video1UUID, 200) + }) + + it('Should get the remote video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video2UUID, 200) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + }) + }) + + describe('With a non followed instance', function () { + + before(async function () { + this.timeout(30000) + + await unfollow(servers[0].url, servers[0].accessToken, servers[1]) + }) + + describe('With an unlogged user', function () { + + it('Should get the local video', async function () { + await getVideo(servers[0].url, video1UUID, 200) + }) + + it('Should not get the remote video', async function () { + await getVideo(servers[0].url, video2UUID, 403) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should not list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, undefined, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(0) + expect(res.body.data).to.have.lengthOf(0) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should not list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, undefined, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(0) + expect(res.body.data).to.have.lengthOf(0) + }) + }) + + describe('With a logged user', function () { + it('Should get the local video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video1UUID, 200) + }) + + it('Should get the remote video', async function () { + await getVideoWithToken(servers[0].url, userAccessToken, video2UUID, 200) + }) + + it('Should list local account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote account videos', async function () { + const res = await getAccountVideos(servers[0].url, userAccessToken, 'root@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list local channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9001', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + + it('Should list remote channel videos', async function () { + const res = await getVideoChannelVideos(servers[0].url, userAccessToken, 'root_channel@localhost:9002', 0, 5) + + expect(res.body.total).to.equal(1) + expect(res.body.data).to.have.lengthOf(1) + }) + }) + }) + + after(async function () { + killallServers(servers) + }) +}) diff --git a/server/tests/api/server/index.ts b/server/tests/api/server/index.ts index 78ab7e18b..6afcab1f9 100644 --- a/server/tests/api/server/index.ts +++ b/server/tests/api/server/index.ts @@ -1,5 +1,6 @@ import './config' import './email' +import './follow-constraints' import './follows' import './handle-down' import './jobs' -- cgit v1.2.3 From babecc3c09cd4ed06fe643a97fff4bcc31c5a9be Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 16 Nov 2018 15:38:09 +0100 Subject: Fix AP collections pagination --- server/controllers/activitypub/client.ts | 4 ++-- server/helpers/activitypub.ts | 14 +++++++------- server/models/activitypub/actor-follow.ts | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) (limited to 'server') diff --git a/server/controllers/activitypub/client.ts b/server/controllers/activitypub/client.ts index a342a48d4..d9d385460 100644 --- a/server/controllers/activitypub/client.ts +++ b/server/controllers/activitypub/client.ts @@ -298,7 +298,7 @@ async function actorFollowing (req: express.Request, actor: ActorModel) { return ActorFollowModel.listAcceptedFollowingUrlsForApi([ actor.id ], undefined, start, count) } - return activityPubCollectionPagination(CONFIG.WEBSERVER.URL + req.url, handler, req.query.page) + return activityPubCollectionPagination(CONFIG.WEBSERVER.URL + req.path, handler, req.query.page) } async function actorFollowers (req: express.Request, actor: ActorModel) { @@ -306,7 +306,7 @@ async function actorFollowers (req: express.Request, actor: ActorModel) { return ActorFollowModel.listAcceptedFollowerUrlsForApi([ actor.id ], undefined, start, count) } - return activityPubCollectionPagination(CONFIG.WEBSERVER.URL + req.url, handler, req.query.page) + return activityPubCollectionPagination(CONFIG.WEBSERVER.URL + req.path, handler, req.query.page) } function videoRates (req: express.Request, rateType: VideoRateType, video: VideoModel, url: string) { diff --git a/server/helpers/activitypub.ts b/server/helpers/activitypub.ts index 4bf6e387d..bcbd9be59 100644 --- a/server/helpers/activitypub.ts +++ b/server/helpers/activitypub.ts @@ -57,16 +57,16 @@ function activityPubContextify (data: T) { } type ActivityPubCollectionPaginationHandler = (start: number, count: number) => Bluebird> | Promise> -async function activityPubCollectionPagination (url: string, handler: ActivityPubCollectionPaginationHandler, page?: any) { +async function activityPubCollectionPagination (baseUrl: string, handler: ActivityPubCollectionPaginationHandler, page?: any) { if (!page || !validator.isInt(page)) { // We just display the first page URL, we only need the total items const result = await handler(0, 1) return { - id: url, + id: baseUrl, type: 'OrderedCollection', totalItems: result.total, - first: url + '?page=1' + first: baseUrl + '?page=1' } } @@ -81,19 +81,19 @@ async function activityPubCollectionPagination (url: string, handler: ActivityPu // There are more results if (result.total > page * ACTIVITY_PUB.COLLECTION_ITEMS_PER_PAGE) { - next = url + '?page=' + (page + 1) + next = baseUrl + '?page=' + (page + 1) } if (page > 1) { - prev = url + '?page=' + (page - 1) + prev = baseUrl + '?page=' + (page - 1) } return { - id: url + '?page=' + page, + id: baseUrl + '?page=' + page, type: 'OrderedCollectionPage', prev, next, - partOf: url, + partOf: baseUrl, orderedItems: result.data, totalItems: result.total } diff --git a/server/models/activitypub/actor-follow.ts b/server/models/activitypub/actor-follow.ts index 3373355ef..0a6935083 100644 --- a/server/models/activitypub/actor-follow.ts +++ b/server/models/activitypub/actor-follow.ts @@ -509,12 +509,12 @@ export class ActorFollowModel extends Model { tasks.push(ActorFollowModel.sequelize.query(query, options)) } - const [ followers, [ { total } ] ] = await Promise.all(tasks) + const [ followers, [ dataTotal ] ] = await Promise.all(tasks) const urls: string[] = followers.map(f => f.url) return { data: urls, - total: parseInt(total, 10) + total: dataTotal ? parseInt(dataTotal.total, 10) : 0 } } -- cgit v1.2.3 From 58d515e32fe1d0133435b3a5e550c6ff24906fff Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 16 Nov 2018 16:48:17 +0100 Subject: Fix images size when downloading them --- server/helpers/requests.ts | 12 +++++++++++- server/lib/activitypub/actor.ts | 9 +++------ server/lib/activitypub/videos.ts | 10 +++------- server/lib/job-queue/handlers/video-import.ts | 10 +++++----- server/tests/api/redundancy/redundancy.ts | 5 +++-- 5 files changed, 25 insertions(+), 21 deletions(-) (limited to 'server') diff --git a/server/helpers/requests.ts b/server/helpers/requests.ts index 51facc9e0..805930a9f 100644 --- a/server/helpers/requests.ts +++ b/server/helpers/requests.ts @@ -2,6 +2,7 @@ import * as Bluebird from 'bluebird' import { createWriteStream } from 'fs-extra' import * as request from 'request' import { ACTIVITY_PUB } from '../initializers' +import { processImage } from './image-utils' function doRequest ( requestOptions: request.CoreOptions & request.UriOptions & { activityPub?: boolean } @@ -27,9 +28,18 @@ function doRequestAndSaveToFile (requestOptions: request.CoreOptions & request.U }) } +async function downloadImage (url: string, destPath: string, size: { width: number, height: number }) { + const tmpPath = destPath + '.tmp' + + await doRequestAndSaveToFile({ method: 'GET', uri: url }, tmpPath) + + await processImage({ path: tmpPath }, destPath, size) +} + // --------------------------------------------------------------------------- export { doRequest, - doRequestAndSaveToFile + doRequestAndSaveToFile, + downloadImage } diff --git a/server/lib/activitypub/actor.ts b/server/lib/activitypub/actor.ts index b16a00669..218dbc6a7 100644 --- a/server/lib/activitypub/actor.ts +++ b/server/lib/activitypub/actor.ts @@ -11,9 +11,9 @@ import { isActivityPubUrlValid } from '../../helpers/custom-validators/activityp import { retryTransactionWrapper, updateInstanceWithAnother } from '../../helpers/database-utils' import { logger } from '../../helpers/logger' import { createPrivateAndPublicKeys } from '../../helpers/peertube-crypto' -import { doRequest, doRequestAndSaveToFile } from '../../helpers/requests' +import { doRequest, doRequestAndSaveToFile, downloadImage } from '../../helpers/requests' import { getUrlFromWebfinger } from '../../helpers/webfinger' -import { CONFIG, IMAGE_MIMETYPE_EXT, sequelizeTypescript } from '../../initializers' +import { AVATARS_SIZE, CONFIG, IMAGE_MIMETYPE_EXT, PREVIEWS_SIZE, sequelizeTypescript } from '../../initializers' import { AccountModel } from '../../models/account/account' import { ActorModel } from '../../models/activitypub/actor' import { AvatarModel } from '../../models/avatar/avatar' @@ -180,10 +180,7 @@ async function fetchAvatarIfExists (actorJSON: ActivityPubActor) { const avatarName = uuidv4() + extension const destPath = join(CONFIG.STORAGE.AVATARS_DIR, avatarName) - await doRequestAndSaveToFile({ - method: 'GET', - uri: actorJSON.icon.url - }, destPath) + await downloadImage(actorJSON.icon.url, destPath, AVATARS_SIZE) return avatarName } diff --git a/server/lib/activitypub/videos.ts b/server/lib/activitypub/videos.ts index 5bd03c8c6..80de92f24 100644 --- a/server/lib/activitypub/videos.ts +++ b/server/lib/activitypub/videos.ts @@ -10,8 +10,8 @@ import { sanitizeAndCheckVideoTorrentObject } from '../../helpers/custom-validat import { isVideoFileInfoHashValid } from '../../helpers/custom-validators/videos' import { resetSequelizeInstance, retryTransactionWrapper } from '../../helpers/database-utils' import { logger } from '../../helpers/logger' -import { doRequest, doRequestAndSaveToFile } from '../../helpers/requests' -import { ACTIVITY_PUB, CONFIG, REMOTE_SCHEME, sequelizeTypescript, VIDEO_MIMETYPE_EXT } from '../../initializers' +import { doRequest, downloadImage } from '../../helpers/requests' +import { ACTIVITY_PUB, CONFIG, REMOTE_SCHEME, sequelizeTypescript, THUMBNAILS_SIZE, VIDEO_MIMETYPE_EXT } from '../../initializers' import { ActorModel } from '../../models/activitypub/actor' import { TagModel } from '../../models/video/tag' import { VideoModel } from '../../models/video/video' @@ -97,11 +97,7 @@ function generateThumbnailFromUrl (video: VideoModel, icon: ActivityIconObject) const thumbnailName = video.getThumbnailName() const thumbnailPath = join(CONFIG.STORAGE.THUMBNAILS_DIR, thumbnailName) - const options = { - method: 'GET', - uri: icon.url - } - return doRequestAndSaveToFile(options, thumbnailPath) + return downloadImage(icon.url, thumbnailPath, THUMBNAILS_SIZE) } function getOrCreateVideoChannelFromVideoObject (videoObject: VideoTorrentObject) { diff --git a/server/lib/job-queue/handlers/video-import.ts b/server/lib/job-queue/handlers/video-import.ts index e3f2a276c..4de901c0c 100644 --- a/server/lib/job-queue/handlers/video-import.ts +++ b/server/lib/job-queue/handlers/video-import.ts @@ -6,8 +6,8 @@ import { VideoImportState } from '../../../../shared/models/videos' import { getDurationFromVideoFile, getVideoFileFPS, getVideoFileResolution } from '../../../helpers/ffmpeg-utils' import { extname, join } from 'path' import { VideoFileModel } from '../../../models/video/video-file' -import { CONFIG, sequelizeTypescript, VIDEO_IMPORT_TIMEOUT } from '../../../initializers' -import { doRequestAndSaveToFile } from '../../../helpers/requests' +import { CONFIG, PREVIEWS_SIZE, sequelizeTypescript, THUMBNAILS_SIZE, VIDEO_IMPORT_TIMEOUT } from '../../../initializers' +import { doRequestAndSaveToFile, downloadImage } from '../../../helpers/requests' import { VideoState } from '../../../../shared' import { JobQueue } from '../index' import { federateVideoIfNeeded } from '../../activitypub' @@ -133,7 +133,7 @@ async function processFile (downloader: () => Promise, videoImport: Vide videoId: videoImport.videoId } videoFile = new VideoFileModel(videoFileData) - // Import if the import fails, to clean files + // To clean files if the import fails videoImport.Video.VideoFiles = [ videoFile ] // Move file @@ -145,7 +145,7 @@ async function processFile (downloader: () => Promise, videoImport: Vide if (options.downloadThumbnail) { if (options.thumbnailUrl) { const destThumbnailPath = join(CONFIG.STORAGE.THUMBNAILS_DIR, videoImport.Video.getThumbnailName()) - await doRequestAndSaveToFile({ method: 'GET', uri: options.thumbnailUrl }, destThumbnailPath) + await downloadImage(options.thumbnailUrl, destThumbnailPath, THUMBNAILS_SIZE) } else { await videoImport.Video.createThumbnail(videoFile) } @@ -157,7 +157,7 @@ async function processFile (downloader: () => Promise, videoImport: Vide if (options.downloadPreview) { if (options.thumbnailUrl) { const destPreviewPath = join(CONFIG.STORAGE.PREVIEWS_DIR, videoImport.Video.getPreviewName()) - await doRequestAndSaveToFile({ method: 'GET', uri: options.thumbnailUrl }, destPreviewPath) + await downloadImage(options.thumbnailUrl, destPreviewPath, PREVIEWS_SIZE) } else { await videoImport.Video.createPreview(videoFile) } diff --git a/server/tests/api/redundancy/redundancy.ts b/server/tests/api/redundancy/redundancy.ts index 47f4e59fc..a8a2f305f 100644 --- a/server/tests/api/redundancy/redundancy.ts +++ b/server/tests/api/redundancy/redundancy.ts @@ -17,7 +17,7 @@ import { viewVideo, wait, waitUntilLog, - checkVideoFilesWereRemoved, removeVideo + checkVideoFilesWereRemoved, removeVideo, getVideoWithToken } from '../../utils' import { waitJobs } from '../../utils/server/jobs' import * as magnetUtil from 'magnet-uri' @@ -93,7 +93,8 @@ async function check1WebSeed (strategy: VideoRedundancyStrategy, videoUUID?: str for (const server of servers) { { - const res = await getVideo(server.url, videoUUID) + // With token to avoid issues with video follow constraints + const res = await getVideoWithToken(server.url, server.accessToken, videoUUID) const video: VideoDetails = res.body for (const f of video.files) { -- cgit v1.2.3