Fix 404 AP status codes

7 files changed, 118 insertions, 85 deletions

diff --git a/server/helpers/requests.ts b/server/helpers/requests.ts
index aee8f6673..5eb69486d 100644
--- a/server/helpers/requests.ts
+++ b/server/helpers/requests.ts
@@ -1,5 +1,5 @@
 import { createWriteStream, remove } from 'fs-extra'
-import got, { CancelableRequest, Options as GotOptions } from 'got'
+import got, { CancelableRequest, Options as GotOptions, RequestError } from 'got'
 import { join } from 'path'
 import { CONFIG } from '../initializers/config'
 import { ACTIVITY_PUB, PEERTUBE_VERSION, WEBSERVER } from '../initializers/constants'
@@ -7,6 +7,11 @@ import { pipelinePromise } from './core-utils'
 import { processImage } from './image-utils'
 import { logger } from './logger'
 
+export interface PeerTubeRequestError extends Error {
+  statusCode?: number
+  responseBody?: any
+}
+
 const httpSignature = require('http-signature')
 
 type PeerTubeRequestOptions = {
@@ -180,14 +185,15 @@ function buildGotOptions (options: PeerTubeRequestOptions) {
   }
 }
 
-function buildRequestError (error: any) {
-  const newError = new Error(error.message)
+function buildRequestError (error: RequestError) {
+  const newError: PeerTubeRequestError = new Error(error.message)
   newError.name = error.name
   newError.stack = error.stack
 
-  if (error.response?.body) {
-    error.responseBody = error.response.body
+  if (error.response) {
+    newError.responseBody = error.response.body
+    newError.statusCode = error.response.statusCode
   }
 
-  return error
+  return newError
 }
diff --git a/server/lib/activitypub/actor.ts b/server/lib/activitypub/actor.ts
index 52b6c1f56..3c9a7ba02 100644
--- a/server/lib/activitypub/actor.ts
+++ b/server/lib/activitypub/actor.ts
@@ -14,7 +14,7 @@ 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 { doJSONRequest } from '../../helpers/requests'
+import { doJSONRequest, PeerTubeRequestError } from '../../helpers/requests'
 import { getUrlFromWebfinger } from '../../helpers/webfinger'
 import { MIMETYPES, WEBSERVER } from '../../initializers/constants'
 import { sequelizeTypescript } from '../../initializers/database'
@@ -279,16 +279,7 @@ async function refreshActorIfNeeded <T extends MActorFull | MActorAccountChannel
       actorUrl = actor.url
     }
 
-    const { result, statusCode } = await fetchRemoteActor(actorUrl)
-
-    if (statusCode === HttpStatusCode.NOT_FOUND_404) {
-      logger.info('Deleting actor %s because there is a 404 in refresh actor.', actor.url)
-      actor.Account
-        ? await actor.Account.destroy()
-        : await actor.VideoChannel.destroy()
-
-      return { actor: undefined, refreshed: false }
-    }
+    const { result } = await fetchRemoteActor(actorUrl)
 
     if (result === undefined) {
       logger.warn('Cannot fetch remote actor in refresh actor.')
@@ -328,6 +319,15 @@ async function refreshActorIfNeeded <T extends MActorFull | MActorAccountChannel
       return { refreshed: true, actor }
     })
   } catch (err) {
+    if ((err as PeerTubeRequestError).statusCode === HttpStatusCode.NOT_FOUND_404) {
+      logger.info('Deleting actor %s because there is a 404 in refresh actor.', actor.url)
+      actor.Account
+        ? await actor.Account.destroy()
+        : await actor.VideoChannel.destroy()
+
+      return { actor: undefined, refreshed: false }
+    }
+
     logger.warn('Cannot refresh actor %s.', actor.url, { err })
     return { actor, refreshed: false }
   }
diff --git a/server/lib/activitypub/playlist.ts b/server/lib/activitypub/playlist.ts
index 795be60d7..7166c68a6 100644
--- a/server/lib/activitypub/playlist.ts
+++ b/server/lib/activitypub/playlist.ts
@@ -7,7 +7,7 @@ import { checkUrlsSameHost } from '../../helpers/activitypub'
 import { isPlaylistElementObjectValid, isPlaylistObjectValid } from '../../helpers/custom-validators/activitypub/playlist'
 import { isArray } from '../../helpers/custom-validators/misc'
 import { logger } from '../../helpers/logger'
-import { doJSONRequest } from '../../helpers/requests'
+import { doJSONRequest, PeerTubeRequestError } from '../../helpers/requests'
 import { ACTIVITY_PUB, CRAWL_REQUEST_CONCURRENCY } from '../../initializers/constants'
 import { sequelizeTypescript } from '../../initializers/database'
 import { VideoPlaylistModel } from '../../models/video/video-playlist'
@@ -116,13 +116,7 @@ async function refreshVideoPlaylistIfNeeded (videoPlaylist: MVideoPlaylistOwner)
   if (!videoPlaylist.isOutdated()) return videoPlaylist
 
   try {
-    const { statusCode, playlistObject } = await fetchRemoteVideoPlaylist(videoPlaylist.url)
-    if (statusCode === HttpStatusCode.NOT_FOUND_404) {
-      logger.info('Cannot refresh remote video playlist %s: it does not exist anymore. Deleting it.', videoPlaylist.url)
-
-      await videoPlaylist.destroy()
-      return undefined
-    }
+    const { playlistObject } = await fetchRemoteVideoPlaylist(videoPlaylist.url)
 
     if (playlistObject === undefined) {
       logger.warn('Cannot refresh remote playlist %s: invalid body.', videoPlaylist.url)
@@ -136,6 +130,13 @@ async function refreshVideoPlaylistIfNeeded (videoPlaylist: MVideoPlaylistOwner)
 
     return videoPlaylist
   } catch (err) {
+    if ((err as PeerTubeRequestError).statusCode === HttpStatusCode.NOT_FOUND_404) {
+      logger.info('Cannot refresh remote video playlist %s: it does not exist anymore. Deleting it.', videoPlaylist.url)
+
+      await videoPlaylist.destroy()
+      return undefined
+    }
+
     logger.warn('Cannot refresh video playlist %s.', videoPlaylist.url, { err })
 
     await videoPlaylist.setAsRefreshed()
diff --git a/server/lib/activitypub/videos.ts b/server/lib/activitypub/videos.ts
index a5f58dd01..d484edd36 100644
--- a/server/lib/activitypub/videos.ts
+++ b/server/lib/activitypub/videos.ts
@@ -30,7 +30,7 @@ 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 { doJSONRequest } from '../../helpers/requests'
+import { doJSONRequest, PeerTubeRequestError } from '../../helpers/requests'
 import { fetchVideoByUrl, getExtFromMimetype, VideoFetchByUrlType } from '../../helpers/video'
 import {
   ACTIVITY_PUB,
@@ -523,14 +523,7 @@ async function refreshVideoIfNeeded (options: {
     : await VideoModel.loadByUrlAndPopulateAccount(options.video.url)
 
   try {
-    const { statusCode, videoObject } = await fetchRemoteVideo(video.url)
-    if (statusCode === HttpStatusCode.NOT_FOUND_404) {
-      logger.info('Cannot refresh remote video %s: video does not exist anymore. Deleting it.', video.url)
-
-      // Video does not exist anymore
-      await video.destroy()
-      return undefined
-    }
+    const { videoObject } = await fetchRemoteVideo(video.url)
 
     if (videoObject === undefined) {
       logger.warn('Cannot refresh remote video %s: invalid body.', video.url)
@@ -554,6 +547,14 @@ async function refreshVideoIfNeeded (options: {
 
     return video
   } catch (err) {
+    if ((err as PeerTubeRequestError).statusCode === HttpStatusCode.NOT_FOUND_404) {
+      logger.info('Cannot refresh remote video %s: video does not exist anymore. Deleting it.', video.url)
+
+      // Video does not exist anymore
+      await video.destroy()
+      return undefined
+    }
+
     logger.warn('Cannot refresh video %s.', options.video.url, { err })
 
     ActorFollowScoreCache.Instance.addBadServerId(video.VideoChannel.Actor.serverId)
diff --git a/server/lib/job-queue/handlers/activitypub-cleaner.ts b/server/lib/job-queue/handlers/activitypub-cleaner.ts
index 9dcc778fa..1caca1dcc 100644
--- a/server/lib/job-queue/handlers/activitypub-cleaner.ts
+++ b/server/lib/job-queue/handlers/activitypub-cleaner.ts
@@ -7,7 +7,7 @@ import {
   isLikeActivityValid
 } from '@server/helpers/custom-validators/activitypub/activity'
 import { sanitizeAndCheckVideoCommentObject } from '@server/helpers/custom-validators/activitypub/video-comments'
-import { doJSONRequest } from '@server/helpers/requests'
+import { doJSONRequest, PeerTubeRequestError } from '@server/helpers/requests'
 import { AP_CLEANER_CONCURRENCY } from '@server/initializers/constants'
 import { VideoModel } from '@server/models/video/video'
 import { VideoCommentModel } from '@server/models/video/video-comment'
@@ -81,39 +81,44 @@ async function updateObjectIfNeeded <T> (
   updater: (url: string, newUrl: string) => Promise<T>,
   deleter: (url: string) => Promise<T>
 ): Promise<{ data: T, status: 'deleted' | 'updated' } | null> {
-  const { statusCode, body } = await doJSONRequest<any>(url, { activityPub: true })
-
-  // Does not exist anymore, remove entry
-  if (statusCode === HttpStatusCode.NOT_FOUND_404) {
+  const on404OrTombstone = async () => {
     logger.info('Removing remote AP object %s.', url)
     const data = await deleter(url)
 
-    return { status: 'deleted', data }
+    return { status: 'deleted' as 'deleted', data }
   }
 
-  // If not same id, check same host and update
-  if (!body || !body.id || !bodyValidator(body)) throw new Error(`Body or body id of ${url} is invalid`)
+  try {
+    const { body } = await doJSONRequest<any>(url, { activityPub: true })
 
-  if (body.type === 'Tombstone') {
-    logger.info('Removing remote AP object %s.', url)
-    const data = await deleter(url)
+    // If not same id, check same host and update
+    if (!body || !body.id || !bodyValidator(body)) throw new Error(`Body or body id of ${url} is invalid`)
 
-    return { status: 'deleted', data }
-  }
+    if (body.type === 'Tombstone') {
+      return on404OrTombstone()
+    }
 
-  const newUrl = body.id
-  if (newUrl !== url) {
-    if (checkUrlsSameHost(newUrl, url) !== true) {
-      throw new Error(`New url ${newUrl} has not the same host than old url ${url}`)
+    const newUrl = body.id
+    if (newUrl !== url) {
+      if (checkUrlsSameHost(newUrl, url) !== true) {
+        throw new Error(`New url ${newUrl} has not the same host than old url ${url}`)
+      }
+
+      logger.info('Updating remote AP object %s.', url)
+      const data = await updater(url, newUrl)
+
+      return { status: 'updated', data }
     }
 
-    logger.info('Updating remote AP object %s.', url)
-    const data = await updater(url, newUrl)
+    return null
+  } catch (err) {
+    // Does not exist anymore, remove entry
+    if ((err as PeerTubeRequestError).statusCode === HttpStatusCode.NOT_FOUND_404) {
+      return on404OrTombstone()
+    }
 
-    return { status: 'updated', data }
+    throw err
   }
-
-  return null
 }
 
 function rateOptionsFactory () {
diff --git a/server/tests/api/activitypub/security.ts b/server/tests/api/activitypub/security.ts
index 26b4545ac..9745052a3 100644
--- a/server/tests/api/activitypub/security.ts
+++ b/server/tests/api/activitypub/security.ts
@@ -79,9 +79,12 @@ describe('Test ActivityPub security', function () {
         Digest: buildDigest({ hello: 'coucou' })
       }
 
-      const { statusCode } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
-
-      expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      try {
+        await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
+        expect(true, 'Did not throw').to.be.false
+      } catch (err) {
+        expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      }
     })
 
     it('Should fail with an invalid date', async function () {
@@ -89,9 +92,12 @@ describe('Test ActivityPub security', function () {
       const headers = buildGlobalHeaders(body)
       headers['date'] = 'Wed, 21 Oct 2015 07:28:00 GMT'
 
-      const { statusCode } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
-
-      expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      try {
+        await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
+        expect(true, 'Did not throw').to.be.false
+      } catch (err) {
+        expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      }
     })
 
     it('Should fail with bad keys', async function () {
@@ -101,9 +107,12 @@ describe('Test ActivityPub security', function () {
       const body = activityPubContextify(getAnnounceWithoutContext(servers[1]))
       const headers = buildGlobalHeaders(body)
 
-      const { statusCode } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
-
-      expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      try {
+        await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
+        expect(true, 'Did not throw').to.be.false
+      } catch (err) {
+        expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      }
     })
 
     it('Should reject requests without appropriate signed headers', async function () {
@@ -123,8 +132,12 @@ describe('Test ActivityPub security', function () {
       for (const badHeaders of badHeadersMatrix) {
         signatureOptions.headers = badHeaders
 
-        const { statusCode } = await makePOSTAPRequest(url, body, signatureOptions, headers)
-        expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+        try {
+          await makePOSTAPRequest(url, body, signatureOptions, headers)
+          expect(true, 'Did not throw').to.be.false
+        } catch (err) {
+          expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+        }
       }
     })
 
@@ -133,7 +146,6 @@ describe('Test ActivityPub security', function () {
       const headers = buildGlobalHeaders(body)
 
       const { statusCode } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
-
       expect(statusCode).to.equal(HttpStatusCode.NO_CONTENT_204)
     })
 
@@ -150,9 +162,12 @@ describe('Test ActivityPub security', function () {
       const body = activityPubContextify(getAnnounceWithoutContext(servers[1]))
       const headers = buildGlobalHeaders(body)
 
-      const { statusCode } = await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
-
-      expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      try {
+        await makePOSTAPRequest(url, body, baseHttpSignature(), headers)
+        expect(true, 'Did not throw').to.be.false
+      } catch (err) {
+        expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      }
     })
   })
 
@@ -183,9 +198,12 @@ describe('Test ActivityPub security', function () {
 
       const headers = buildGlobalHeaders(signedBody)
 
-      const { statusCode } = await makePOSTAPRequest(url, signedBody, baseHttpSignature(), headers)
-
-      expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      try {
+        await makePOSTAPRequest(url, signedBody, baseHttpSignature(), headers)
+        expect(true, 'Did not throw').to.be.false
+      } catch (err) {
+        expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      }
     })
 
     it('Should fail with an altered body', async function () {
@@ -204,9 +222,12 @@ describe('Test ActivityPub security', function () {
 
       const headers = buildGlobalHeaders(signedBody)
 
-      const { statusCode } = await makePOSTAPRequest(url, signedBody, baseHttpSignature(), headers)
-
-      expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      try {
+        await makePOSTAPRequest(url, signedBody, baseHttpSignature(), headers)
+        expect(true, 'Did not throw').to.be.false
+      } catch (err) {
+        expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      }
     })
 
     it('Should succeed with a valid signature', async function () {
@@ -221,7 +242,6 @@ describe('Test ActivityPub security', function () {
       const headers = buildGlobalHeaders(signedBody)
 
       const { statusCode } = await makePOSTAPRequest(url, signedBody, baseHttpSignature(), headers)
-
       expect(statusCode).to.equal(HttpStatusCode.NO_CONTENT_204)
     })
 
@@ -243,9 +263,12 @@ describe('Test ActivityPub security', function () {
 
       const headers = buildGlobalHeaders(signedBody)
 
-      const { statusCode } = await makePOSTAPRequest(url, signedBody, baseHttpSignature(), headers)
-
-      expect(statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      try {
+        await makePOSTAPRequest(url, signedBody, baseHttpSignature(), headers)
+        expect(true, 'Did not throw').to.be.false
+      } catch (err) {
+        expect(err.statusCode).to.equal(HttpStatusCode.FORBIDDEN_403)
+      }
     })
   })
 
diff --git a/server/tools/peertube-import-videos.ts b/server/tools/peertube-import-videos.ts
index 9be0834ba..915995031 100644
--- a/server/tools/peertube-import-videos.ts
+++ b/server/tools/peertube-import-videos.ts
@@ -202,10 +202,7 @@ async function uploadVideoOnPeerTube (parameters: {
   if (videoInfo.thumbnail) {
     thumbnailfile = join(cwd, sha256(videoInfo.thumbnail) + '.jpg')
 
-    await doRequestAndSaveToFile({
-      method: 'GET',
-      uri: videoInfo.thumbnail
-    }, thumbnailfile)
+    await doRequestAndSaveToFile(videoInfo.thumbnail, thumbnailfile)
   }
 
   const originallyPublishedAt = buildOriginallyPublishedAt(videoInfo)